-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[pickers] Go to the default view when opening a picker #7484
Conversation
These are the results for the performance tests:
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
expect(handleViewChange.callCount).to.equal(1); | ||
|
||
// Dismiss the picker | ||
userEvent.keyPress(document.activeElement!, { key: 'Escape' }); | ||
|
||
openPicker({ type: 'date', variant: 'desktop' }); | ||
expect(handleViewChange.callCount).to.equal(2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not have a full understanding of how the current view is managed. But this test raised a question:
Would it makes sense to switch back to the default view when we close the view. Such that when reopening, the current view will not be set and so we avoid getting two renderings:
- one for opening
- a second one for updating the
view
state
I'm wondering if removing the early return if(!open)
in hook you modified would do the job
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it makes sense to switch back to the default view when we close the view.
That's an interesting question indeed
The view would probably not be reset if the user is controlling the opening state and closing it from the outside.
Not sure if this is problematic.
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
f71d0ef
to
42c904c
Compare
42c904c
to
55d4c3b
Compare
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Part of #7440
We are moving back to v5 behavior on this