Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Breaking] Make DateRangePicker key down handlers match SingleDatePicker's #800

Merged
merged 3 commits into from
Oct 27, 2017
Merged

[Breaking] Make DateRangePicker key down handlers match SingleDatePicker's #800

merged 3 commits into from
Oct 27, 2017

Conversation

erin-doyle
Copy link
Collaborator

Some minor clean up of function name inconsistencies between SingleDatePicker and DateRangePicker. SingleDatePicker has onKeyDownArrowDown and onKeyDownQuestionMark and that's what DateInput has, so it seemed like DateRangePicker should match.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 86.76% when pulling b1a3cfd on erin-doyle:date_range_function_cleanup into ef98109 on airbnb:master.

@ljharb ljharb added the semver-major: breaking change A non-backwards-compatible change; anything that breaks code - including adding a peerDep. label Oct 25, 2017
ljharb
ljharb previously approved these changes Oct 25, 2017
View reviewed changes
Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

Do docs and examples need to be updated as well?

majapw
majapw previously approved these changes Oct 25, 2017
View reviewed changes
Copy link
Collaborator

@majapw majapw left a comment

Choose a reason for hiding this comment

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

LGTM as well!

@erin-doyle erin-doyle dismissed stale reviews from majapw and ljharb via ca9bb33 October 26, 2017 11:58
@coveralls
Copy link

Coverage Status

Coverage remained the same at 86.76% when pulling ca9bb33 on erin-doyle:date_range_function_cleanup into ef98109 on airbnb:master.

Clean up call to onClose which from SingleDatePicker should only send…
f3d2c8a 
… date as there is no startDate and endDate.
@erin-doyle
Copy link
Collaborator Author

I checked the README and I don't see any impacts to it.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 86.76% when pulling f3d2c8a on erin-doyle:date_range_function_cleanup into ef98109 on airbnb:master.

majapw
majapw approved these changes Oct 27, 2017
View reviewed changes
Copy link
Collaborator

@majapw majapw left a comment

Choose a reason for hiding this comment

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

Do we want to do a minor cut with the existing changes rn or do we want to lump this all into a v15?

@ljharb
Copy link
Member

ljharb commented Oct 27, 2017

I think it'd be a good idea to do a release before merging in breaking changes

@majapw
Copy link
Collaborator

majapw commented Oct 27, 2017

v14.1.0 is out, so we can merge this in.

@majapw majapw merged commit 9ae7efa into react-dates:master Oct 27, 2017
@erin-doyle erin-doyle deleted the date_range_function_cleanup branch October 27, 2017 01:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@majapw majapw majapw approved these changes

@ljharb ljharb ljharb left review comments

Assignees
No one assigned
Labels
semver-major: breaking change A non-backwards-compatible change; anything that breaks code - including adding a peerDep.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@erin-doyle @coveralls @ljharb @majapw