Skip to content

Conversation

@dtaylor113
Copy link
Member

Description

Added two-way binding on Date property and added the ability for user to define a callback function when date is changed via the picker.

Fixes: #730

image
image

PR Checklist

  • Screenshots are attached (if there are visual changes in the UI)

@dtaylor113
Copy link
Member Author

CC @wywywywy

@nicolethoen
Copy link
Contributor

LGTM

nicolethoen
nicolethoen previously approved these changes Jul 3, 2018
Copy link
Member

@jeff-phillips-18 jeff-phillips-18 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be unit tests added to verify the callback is called.

Do we need the two-way binding if we have the callback?

@dtaylor113
Copy link
Member Author

Do we need the two-way binding if we have the callback?

Probably not. I had thought it was an ask in #730 but re-reading the issue maybe not. Would two way binding help app devs if they had a $watch on the var.?

@jeff-phillips-18
Copy link
Member

The callback is better than using $watch. My only thought was it would be a way to reject the selected Date and not update if it were invalid. I'm OK with either way.

@dtaylor113 dtaylor113 force-pushed the pfbootstrapdatepicker branch from b1c4181 to 1ebfaf6 Compare July 5, 2018 15:11
@dtaylor113 dtaylor113 changed the title fix(pfBootstrapDatepicker): added two way binding and callback function fix(pfBootstrapDatepicker): added callback function when date changes Jul 5, 2018
@dtaylor113
Copy link
Member Author

Hi @jeff-phillips-18, removed two way binding and added unit test PTAL -thanks

@jeff-phillips-18 jeff-phillips-18 merged commit 6596116 into patternfly:master Jul 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants