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.

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

[DateInput] Selected value stays selected when canClearSelection=false with timePrecision enabled #1792

Merged
merged 4 commits into from
Nov 7, 2017

Conversation

cmslewis
Copy link
Contributor

@cmslewis cmslewis commented Nov 6, 2017

Fixes #1737

Checklist

  • Include tests

Changes proposed in this pull request:

BEFORE:

  1. Make a DateInput with canClearSelection=false and timePrecision enabled.
  2. Select a date.
  3. Click the selected date again.
  4. onChange is erroneously emitted with null, suggesting that the date has just been deselected, which shouldn't be possible.

AFTER:
DateInput no longer emits onChange with null when clicking selected date with canClearSelection=false with timePrecision enabled.

Reviewers should focus on:

Nothing in particular. Easy fix.

Screenshot

Here's a working example with the following props:

<DateInput
    canClearSelection={false}
    value={new Date(2017, Months.NOVEMBER, 6)}
    onChange={console.log}
    timePrecision={TimePickerPrecision.SECOND}
/>

2017-11-06 09 19 56

// the current month.
// this change handler was triggered by a change in month, day, or (if
// enabled) time. for UX purposes, we want to close the popover only if
// the user explicitly clicked a day within the current month.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wrapped to 80 chars. @adidahiya is there a tslint rule or prettier rule that can enforce a different max-line-length for comments? Wrapping them at 120 chars makes them kind of hard to read.

@blueprint-bot
Copy link

Add uncontrolled test too

Preview: documentation
Coverage: core | datetime

@cmslewis cmslewis merged commit e7ffe56 into master Nov 7, 2017
@cmslewis cmslewis deleted the cl/1737-di-canClearSelection branch November 7, 2017 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants