[docs] Fix a few scroll issues on mobile #7900

oliviertassinari · 2023-02-10T23:46:12Z

Fix for SEO https://search.google.com/search-console/mobile-usability?resource_id=sc-domain:mui.com&utm_source=wnc_10030322&utm_medium=gamma&utm_campaign=wnc_10030322&utm_content=msg_100058679&hl=en

Fix part of #5652

mui-bot · 2023-02-10T23:51:16Z

	Messages
📖	Netlify deploy preview: https://deploy-preview-7900--material-ui-x.netlify.app/

These are the results for the performance tests:

Test case	Unit	Min	Max	Median	Mean	σ
Filter 100k rows	ms	653.9	1,139	733.5	856.56	167.321
Sort 100k rows	ms	649.9	1,193.4	649.9	920.24	178.457
Select 100k rows	ms	281.6	400.7	304.4	330.4	46.188
Deselect 100k rows	ms	162.4	326.2	210.9	233.44	60.453

Generated by 🚫 dangerJS against 3fa54d9

oliviertassinari · 2023-02-10T23:47:18Z

docs/src/modules/components/LocalizationTable.js

@@ -78,6 +78,7 @@ function LocalisationTable(props) {
 <MarkdownElement
 sx={{
 width: '100%',
+ px: [2, 0],


Without

https://next.mui.com/x/react-date-pickers/localization/#supported-locales

docs/data/date-pickers/fields/LifeCycleDateRangeField.tsx

oliviertassinari · 2023-02-10T23:50:01Z

docs/data/date-pickers/custom-layout/AddComponent.tsx

@@ -65,6 +65,7 @@ function CustomLayout(props: PickersLayoutProps<Dayjs | null, Dayjs, DateView>)
 <PickersLayoutRoot
 ownerState={props}
 sx={{
+ overflow: 'auto',


Without https://next.mui.com/x/react-date-pickers/custom-layout/#dom-customization

packages/x-date-pickers/src/internals/hooks/useStaticPicker/useStaticPicker.tsx

packages/x-date-pickers-pro/src/internal/hooks/useStaticRangePicker/useStaticRangePicker.tsx

oliviertassinari · 2023-02-11T00:00:22Z

packages/x-date-pickers/src/internals/demo/DemoContainer.tsx

@@ -105,7 +105,9 @@ export function DemoContainer(props: DemoGridProps) {

 let direction: StackProps['direction'];
 let spacing: StackProps['spacing'];
- let sx: StackProps['sx'];
+ let sx: StackProps['sx'] = {
+ overflow: 'auto',


without https://next.mui.com/x/react-date-pickers/date-range-calendar/

LukasTy

Really nice improvements! 💯
Left a few improvement comments as well as a suggestion to limit the PR impact to only docs scope. 🤔

docs/data/date-pickers/date-range-calendar/CustomDateRangePickerDay.tsx

docs/data/date-pickers/fields/LifeCycleDateRangeField.tsx

packages/x-date-pickers-pro/src/internal/hooks/useStaticRangePicker/useStaticRangePicker.tsx

LukasTy

Nice result! 👍
I do agree that those simple demos with only one element can benefit from increased simplicity by removing the redundant DemoContainer wrapper.
If we ever add certain styling logic, which would be needed for them—it's not a rocket science to re-introduce it. 👌

If we are removing it from demos having one example, could we also remove DemoContainer from https://deploy-preview-7900--material-ui-x.netlify.app/x/react-date-pickers/date-picker/#basic-usage ?

What's your take on it @flaviendelangle? After all, it's "your baby". 🙈

docs/data/date-pickers/date-time-field/BasicDateTimeField.tsx

docs/data/date-pickers/fields/LifeCycleDateRangeField.tsx

flaviendelangle · 2023-02-15T14:30:22Z

I think that DemoContainer makes a lot of sense when we need to add a label above each demo. For the case where there are 2 - 3 demos in an example without a label, I would personally be more inclined to use Stack but why not DemoContainer.

It also handles the responsive rules when several items can be placed next to one another.
I would not be in favor of removing it when we have several items, even if they have no label at the GridItem level.
Removing it when there is only one item, why not, I can double check if I don't see any regression.

LukasTy · 2023-02-15T14:33:48Z

It also handles the responsive rules when several items can be placed next to one another. I would not be in favor of removing it when we have several items, even if they have no label at the GridItem level. Removing it when there is only one item, why not, I can double check if I don't see any regression.

I've looked through the changes and the only regression is with the DateTime(Field|Picker).
I do also agree that it's only sensible to drop it from demos having a single component and this is what has been done by Olivier. 😉

oliviertassinari · 2023-02-15T15:40:28Z

For the SEO fix, I will fix it directly with the demo container in the main repo, we should have had overflow: auto there in the first place.

Actually, we didn't add overflow: auto on purpose in the main docs demo wrapper because we want each demo to be self contained. So, it's not an option, I have added back what I had.

flaviendelangle · 2023-02-15T16:09:53Z

I've looked through the changes and the only regression is with the DateTime(Field|Picker).

Nice 👌
I'm ok with the change then

oliviertassinari · 2023-02-15T16:54:40Z

Ok, I think that I'm good now. I have pushed forward with the DemoContainer approach. In HEAD, there are many cases that have the wrong dimensions, check:

But I get why, it's hard to get it right! You have to constantly check each change on many different pages. I could improve a bit the situation, but I couldn't make it work everywhere. I felt that the best way to get something reliably great is to take the DemoContainer abstraction and go one level lower, with hard coding more width values in each demo (we do need an abstraction, e.g. for the overflow but not for the width, there are too many different cases possible for the width:

Long date format (e.g. date time with second range vs. date range), it's the same component but the length needed is way different
Long outline label

LukasTy

Looking great! 👍
Left a few comments and a question to Flavien.

docs/data/date-pickers/base-concepts/ComponentExplorerNoSnap.tsx

LukasTy · 2023-02-16T09:27:16Z

packages/x-date-pickers/src/internals/demo/DemoContainer.tsx

+ alignItems="stretch"
+ spacing={spacing}
+ sx={sx}
+ className={textFieldClasses.root}


Was there something that required adding this class here? 🤔
The final result is a bit strange, where we end up with nested MuiTextField-root classes. 🙈

And example of issue is https://next.mui.com/x/react-date-pickers/time-range-field/#uncontrolled-vs-controlled that doesn't receive the right min width because there is a DemoItem wrapper, but I have reverted. We don't have to fix this in this PR.

oliviertassinari · 2023-02-19T00:17:13Z

docs/data/date-pickers/time-picker/TimePickerViews.tsx

@@ -9,6 +9,7 @@ export default function TimePickerViews() {
 <LocalizationProvider dateAdapter={AdapterDayjs}>
 <DemoContainer
 components={['MobileTimePicker', 'MobileTimePicker', 'MobileTimePicker']}
+ sx={{ minWidth: 210 }}


Related to this fix mui/material-ui#36246, the label label={'"hours", "minutes" and "seconds"'} overflows.

LukasTy

LGTM! 💯
Thank you for all the effort! 🙏
I've looked through Argos changes and didn't find any regressions there, so I approved the changes.
There is still the issue of static landscape pickers having overflow: hidden that prevents the content from being interacted with on narrow viewports, but that is probably better resolved by a separate PR looking into feasibility of changing the overflow:hidden to possibly overflow: auto. 👍

oliviertassinari · 2023-02-20T13:42:41Z

Great, it turned out to be more work than I thought, but it was a great forcing function for me to read all the pages of the docs.

oliviertassinari added bug 🐛 Something doesn't work docs Improvements or additions to the documentation labels Feb 10, 2023

oliviertassinari commented Feb 11, 2023

View reviewed changes

LukasTy reviewed Feb 14, 2023

View reviewed changes

LukasTy added the mobile Targets mobile platform label Feb 14, 2023

This comment was marked as outdated.

Sign in to view

LukasTy reviewed Feb 15, 2023

View reviewed changes

docs/data/date-pickers/date-time-field/BasicDateTimeField.tsx Outdated Show resolved Hide resolved

docs/data/date-pickers/fields/LifeCycleDateRangeField.tsx Outdated Show resolved Hide resolved

oliviertassinari force-pushed the fix-static-overflow branch from d305bd4 to e46eb36 Compare February 15, 2023 15:18

oliviertassinari changed the title ~~[docs] Fix scroll on mobile~~ [docs] Fix a few scroll issues on mobile Feb 15, 2023

oliviertassinari force-pushed the fix-static-overflow branch from e46eb36 to ca9a7b2 Compare February 15, 2023 15:38

oliviertassinari force-pushed the fix-static-overflow branch 3 times, most recently from 5f78e0e to 102d953 Compare February 15, 2023 15:49

oliviertassinari added 5 commits February 15, 2023 17:05

[docs] Fix a few scroll issues on mobile

93421a6

fix blank line

7243a02

missing dots

ec6e469

more fixes

a9eebbb

fix bug

5e8a5a6

oliviertassinari force-pushed the fix-static-overflow branch from 68e8f0e to 5e8a5a6 Compare February 15, 2023 16:05

improve demo

36dc98f

oliviertassinari force-pushed the fix-static-overflow branch from ad4a7e8 to ded61af Compare February 15, 2023 16:46

simplify

801b397

oliviertassinari force-pushed the fix-static-overflow branch from ded61af to 801b397 Compare February 15, 2023 16:50

LukasTy reviewed Feb 16, 2023

View reviewed changes

oliviertassinari added 4 commits February 19, 2023 00:25

simpler

7035d45

reproduce docs demo container

86432fa

fix textfield label getting cut

7b69063

keep it simple

3fa54d9

oliviertassinari commented Feb 19, 2023

View reviewed changes

LukasTy approved these changes Feb 20, 2023

View reviewed changes

oliviertassinari merged commit b7c172a into mui:next Feb 20, 2023

oliviertassinari deleted the fix-static-overflow branch February 20, 2023 13:43

oliviertassinari mentioned this pull request Sep 24, 2023

[pickers] Unexpected token 'export' error when using @mui/x-date-pickers/internals/demo/DemoContainer #10454

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[docs] Fix a few scroll issues on mobile #7900

[docs] Fix a few scroll issues on mobile #7900

oliviertassinari commented Feb 10, 2023 •

edited

Loading

mui-bot commented Feb 10, 2023 •

edited

Loading

oliviertassinari Feb 10, 2023

oliviertassinari Feb 10, 2023

oliviertassinari Feb 11, 2023

LukasTy left a comment

This comment was marked as outdated.

LukasTy left a comment

flaviendelangle commented Feb 15, 2023

LukasTy commented Feb 15, 2023

oliviertassinari commented Feb 15, 2023

flaviendelangle commented Feb 15, 2023

oliviertassinari commented Feb 15, 2023 •

edited

Loading

LukasTy left a comment

LukasTy Feb 16, 2023

oliviertassinari Feb 19, 2023 •

edited

Loading

oliviertassinari Feb 19, 2023 •

edited

Loading

LukasTy left a comment

oliviertassinari commented Feb 20, 2023

[docs] Fix a few scroll issues on mobile #7900

[docs] Fix a few scroll issues on mobile #7900

Conversation

oliviertassinari commented Feb 10, 2023 • edited Loading

mui-bot commented Feb 10, 2023 • edited Loading

oliviertassinari Feb 10, 2023

Choose a reason for hiding this comment

oliviertassinari Feb 10, 2023

Choose a reason for hiding this comment

oliviertassinari Feb 11, 2023

Choose a reason for hiding this comment

LukasTy left a comment

Choose a reason for hiding this comment

This comment was marked as outdated.

LukasTy left a comment

Choose a reason for hiding this comment

flaviendelangle commented Feb 15, 2023

LukasTy commented Feb 15, 2023

oliviertassinari commented Feb 15, 2023

flaviendelangle commented Feb 15, 2023

oliviertassinari commented Feb 15, 2023 • edited Loading

LukasTy left a comment

Choose a reason for hiding this comment

LukasTy Feb 16, 2023

Choose a reason for hiding this comment

oliviertassinari Feb 19, 2023 • edited Loading

Choose a reason for hiding this comment

oliviertassinari Feb 19, 2023 • edited Loading

Choose a reason for hiding this comment

LukasTy left a comment

Choose a reason for hiding this comment

oliviertassinari commented Feb 20, 2023

oliviertassinari commented Feb 10, 2023 •

edited

Loading

mui-bot commented Feb 10, 2023 •

edited

Loading

oliviertassinari commented Feb 15, 2023 •

edited

Loading

oliviertassinari Feb 19, 2023 •

edited

Loading

oliviertassinari Feb 19, 2023 •

edited

Loading