[Pickers] Fix showTodayButton not returning the current time
#24650
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There must be some React magic i don't completly understand, but wrapping the "now" in a React.useRef have the effect of now updating the value in the components
eps1lon
previously requested changes
Jan 27, 2021
There was a problem hiding this comment.
It's not clear to me whether we want these semantics everywhere.
As a first step, please add a test so that we can check if it makes sense to create two different implementations.
useNow might be used as default value of sorts and that shouldn't change over time. That might be implied by how it's used though.
oliviertassinari
added
the
component: pickers
oliviertassinari
added
bug 🐛
oliviertassinari
changed the title
showTodayButton not returning the current time
oliviertassinari
dismissed
eps1lon’s stale review
issue solved and test case added
eps1lon
reviewed
Jan 27, 2021
|
|
||
| expect(onCloseMock.callCount).to.equal(1); | ||
| expect(handleChange.callCount).to.equal(1); | ||
| expect(adapterToUse.getDiff(handleChange.args[0][0], start)).to.equal(10); |
|
@anthonyraymond Thanks for raising the issue to our attention, it's half of the job done. Sebastian was definitely right about the useNow semantic for default values, we needed to keep a ref. |
natac13
pushed a commit
to natac13/material-ui
that referenced
this pull request
Jan 30, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
When clicking on the "Today" button in MobileDate[Time]Picker, the returned value is always the same => the moment at which the component was first mounted.
How to reproduce
Example: https://codesandbox.io/s/62sdn
Expected behaviour
The "Now" value should represent the current time when the user click the button, and not the time when the component was first mounted.
Describe the changes
Somehow, removing the useReft fix the issue, i suspect some hook cache magic being used somewhere in React, but i don't know enough about the Hook implementation so...
Here is a sandbox using the fix proposed in the PR: https://codesandbox.io/s/funny-liskov-8pdp7