Skip to content

[docs] Polish the pickers demo#26094

Merged
oliviertassinari merged 1 commit into
mui:nextfrom
oliviertassinari:docs-pickers-remove-jss
May 11, 2021
Merged

[docs] Polish the pickers demo#26094
oliviertassinari merged 1 commit into
mui:nextfrom
oliviertassinari:docs-pickers-remove-jss

Conversation

@oliviertassinari

Copy link
Copy Markdown
Member
  • Remove inline-style
  • Use the default text field variant
  • Fix spacing issues
  • Remove built-in margin from the standalone components
  • Remove the makeStyles that we can (waiting for migration to emotion for the last two)

@oliviertassinari oliviertassinari added docs Improvements or additions to the documentation. scope: pickers Changes related to the date/time pickers. labels May 2, 2021
@mui-pr-bot

mui-pr-bot commented May 2, 2021

Copy link
Copy Markdown

No bundle size changes (experimental)

Generated by 🚫 dangerJS against 8a851dc

return (
<LocalizationProvider dateAdapter={AdapterDateFns}>
<div style={{ width: 300 }}>
<Stack spacing={3}>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please use the div here instead of introducing a new component. The component is not relevant to this demo.

@oliviertassinari oliviertassinari May 3, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

If we agree that each instance of the date picker (same of the other components) needs to have a large enough gap with its neighbors so the developers can easily process the content, then I believe we don't have that many options.

The alternatives are:

  • A root Box with a descendant selector to add the margin. This adds boilerplate, it's what we used to do in the other demos before replacing the with Stack. For instance https://next.material-ui.com/components/text-fields/#BasicTextFields.tsx
  • Use the margin prop of the TextField. This adds margin outside of the demo, once the developer copy and paste it into it's project. The date pickers is already using a lot of space, I expect that most developers will want to remove the margin, and have to figure out how. It's also learning that can't be shared with the other demos like Stack, as specific to the TextField. This prop was introduced before we had the sx, not sure if it's still relevant anymore.

I personally think that updating https://next.material-ui.com/components/autocomplete/ too would help.

cc @mui-org/core for thoughts on this matter. How can we maximize the DX? Keeping the content of the demos short, minimizing imports, and making it easy to map what's displayed on the screen with the associated source.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I pushed in the same direction for the Autocomplete in: d5455a9 (82 additions and 262 deletions) for #26127.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If we agree that each instance of the date picker (same of the other components) needs to have a large enough gap with its neighbors so the developers can easily process the content, then I believe we don't have that many options.

Sure, just don't introduce new components when they're not relevant to the demo.

@oliviertassinari oliviertassinari May 9, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We agree with the intent, the less developers have to learn / information to process / cleanup to do to use an example, the better.

On the terminology, I use "demo" as a group of multiple "examples". Here, we have two examples bundled under the same demo. The distinction is important because developer don't care so much about the demo but one specific example.

If a demo has multiple examples, each must be spaced enough so that developers can mentally isolate and identify them.

As for how this is solved, either we leak into the example to add spacing, something the developers will need to learn (e.g. margin here or sx when not available) and remove, or it's globally handled by a wrapper that developers can ignore when they copy and paste one instance.

So far, we have systematically refactored the demos in the documentation to use a Stack (as simpler than a Box with a descendant selector).

@github-actions github-actions Bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label May 3, 2021
@oliviertassinari
oliviertassinari force-pushed the docs-pickers-remove-jss branch from 4484c2d to 154c138 Compare May 5, 2021 22:57
@github-actions github-actions Bot added PR: out-of-date The pull request has merge conflicts and can't be merged. and removed PR: out-of-date The pull request has merge conflicts and can't be merged. labels May 5, 2021
@oliviertassinari
oliviertassinari force-pushed the docs-pickers-remove-jss branch from 154c138 to 8a851dc Compare May 11, 2021 10:43
@github-actions github-actions Bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label May 11, 2021
@oliviertassinari
oliviertassinari merged commit e43a6cf into mui:next May 11, 2021
@oliviertassinari
oliviertassinari deleted the docs-pickers-remove-jss branch May 11, 2021 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Improvements or additions to the documentation. scope: pickers Changes related to the date/time pickers.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants