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

[datetime2] fix(DateRangeInput2): input value when controlled #5792

Merged
merged 2 commits into from
Dec 8, 2022

Conversation

tzyl
Copy link
Contributor

@tzyl tzyl commented Dec 8, 2022

Fixes #5791

Checklist

  • Includes tests
  • Update documentation

Changes proposed in this pull request:

DateRangeInput2 input focus handler should read from the controlled value rather than the state value.

When a new date is selected the onChange callback is called which updates the controlled value. On the next render cycle componentDidUpdate syncs the state value with the controlled props value and also refocuses the boundary input at the same time. During this input focus the stale value from the state is read before it has been updated leading to the input display value showing the previous value.

Reviewers should focus on:

The input blur and input change handlers already special case controlled mode could it have been intentional to have this behaviour in the input focus handler for any reason?

Screenshot

I tested this on the docs app locally by changing the example to be controlled:

@@ -120,6 +120,7 @@ export class DateRangeInput2Example extends React.PureComponent<ExampleProps, Da
                     {...spreadProps}
                     {...format}
                     onChange={this.handleRangeChange}
+                    value={range}
                     footerElement={showFooterElement ? exampleFooterElement : undefined}
                     timePickerProps={
                         enableTimePicker
Before After
controlled_before controlled_after

@tzyl tzyl requested a review from adidahiya December 8, 2022 16:52
@adidahiya
Copy link
Contributor

@tzyl it was a bit tricky to add a unit test which failed with the existing behavior... but I've added one that may catch regressions in this behavior in the future.

@adidahiya adidahiya changed the title Fix DateRangeInput2 controlled mode input focus stale value [datetime2] fix(DateRangeInput2) controlled mode input focus stale value Dec 8, 2022
@adidahiya adidahiya changed the title [datetime2] fix(DateRangeInput2) controlled mode input focus stale value [datetime2] fix(DateRangeInput2): stale input value in controlled mode Dec 8, 2022
@blueprint-bot
Copy link

Add unit test

Previews: documentation | landing | table | demo

@adidahiya adidahiya changed the title [datetime2] fix(DateRangeInput2): stale input value in controlled mode [datetime2] fix(DateRangeInput2): input value when controlled Dec 8, 2022
@adidahiya adidahiya merged commit 0b13de2 into develop Dec 8, 2022
@adidahiya adidahiya deleted the tl/fix-date-range-input-2-controlled-focus branch December 8, 2022 20:02
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.

DateRangeInput2 in controlled mode uses previous formatted selected value
3 participants