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

[YearPicker] Scroll to the current year even with autoFocus=false #6089

Merged
merged 13 commits into from
Sep 14, 2022

Conversation

alexfauquette
Copy link
Member

Fix #4601

@mui-bot
Copy link

mui-bot commented Sep 8, 2022

These are the results for the performance tests:

Test case Unit Min Max Median Mean σ
Filter 100k rows ms 491.1 1,016.5 561.4 735.28 189.119
Sort 100k rows ms 613.9 1,210.7 1,210.7 942.48 201.779
Select 100k rows ms 219.3 308.6 300.5 279.42 33.206
Deselect 100k rows ms 135.2 373.8 252.4 241.26 84.824

Generated by 🚫 dangerJS against 86e7798

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Sep 9, 2022
@github-actions
Copy link

github-actions bot commented Sep 9, 2022

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Revert "[core] Remove Storybook (mui#6040)"

This reverts commit e15560c.
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Sep 9, 2022
Unify RFC template using core version
@alexfauquette alexfauquette changed the base branch from master to next September 9, 2022 12:15
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Sep 9, 2022
@github-actions
Copy link

github-actions bot commented Sep 9, 2022

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Sep 9, 2022
@alexfauquette alexfauquette changed the title [YearPicker] scroll to the current year even without autoFocus=true [YearPicker] scroll to the current year even with autoFocus=false Sep 12, 2022
Copy link
Member

@flaviendelangle flaviendelangle left a comment

Choose a reason for hiding this comment

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

Is there a way to achieve the same behavior without adding a new div ?

And what do you think about hiding the content of the div before the scroll is completed ?
Because in it's current shape, we have an ugly flickering.

@alexfauquette
Copy link
Member Author

It was a bit more complicated, but yes I managed to do the same by customizing the existing div

I do not see the flickering. (light mode the PR deploy, dark mode the current doc)

year.scroll.mp4

@flaviendelangle
Copy link
Member

flaviendelangle commented Sep 12, 2022

I do not see the flickering. (light mode the PR deploy, dark mode the current doc)

Use openTo="year" to see it
It's when we go to the year view on mount. I think the problem is that it does not have the ref on the 1st render.

@alexfauquette
Copy link
Member Author

For the blink, it's not a problem with the ref that is responsible. The autofocus has the same behavior

Screencast.from.13.09.22.11.55.06.mp4

You would like to add a


About avoiding to add a new div, I realized I ended up using display: contents which is breaking the accessibility.

We have the following structure:

<CalendarRoot> // a display flex column
	<CalendarHeader /> // fix height
	<TransitionGroup> // extends as much as possible (height: 100%)
		<div>
			<RenderedView/>

The problem is that with height: 100% the div height matches it's content, not its parent. And so I cannot put the overflow to this element.

I can not put the ref on the TransitionGroup (exact name is CalendarPickerViewTransitionContainer) because if reduceAnimations=true it simply render its children

The current solution I implemented is to manage the scroll in the YearPicker because it's the only component with a scroll bar.

@flaviendelangle
Copy link
Member

For the blink, it's not a problem with the ref that is responsible. The autofocus has the same behavior

OK, it's not very sexy, but if the autoFocus has it I guess we can release it like that

The current solution I implemented is to manage the scroll in the YearPicker because it's the only component with a scroll bar.

Makes sense to me

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Sep 14, 2022
@github-actions
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Sep 14, 2022
@alexfauquette alexfauquette changed the title [YearPicker] scroll to the current year even with autoFocus=false [YearPicker] Scroll to the current year even with autoFocus=false Sep 14, 2022
@alexfauquette alexfauquette merged commit 31038b4 into mui:next Sep 14, 2022
@alexfauquette alexfauquette deleted the autoScroll-years branch September 14, 2022 13:10
@oliviertassinari oliviertassinari added design This is about UI or UX design, please involve a designer and removed design: ux labels Aug 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design This is about UI or UX design, please involve a designer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[StaticDatePicker] The year view on StaticDatePicker does not auto scroll to current year
5 participants