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

[docs] Group picker pages #6369

Merged
merged 9 commits into from
Nov 17, 2022
Merged

Conversation

flaviendelangle
Copy link
Member

@flaviendelangle flaviendelangle commented Oct 3, 2022

Part of #5565
Doc preview: https://deploy-preview-6369--material-ui-x.netlify.app/x/react-date-pickers/date-picker/

Follow up

  • Create one page per "Calendar" component

@flaviendelangle flaviendelangle added docs Improvements or additions to the documentation component: pickers This is the name of the generic UI component, not the React module! labels Oct 3, 2022
@flaviendelangle flaviendelangle self-assigned this Oct 3, 2022
@mui-bot
Copy link

mui-bot commented Oct 3, 2022

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

These are the results for the performance tests:

Test case Unit Min Max Median Mean σ
Filter 100k rows ms 631.3 1,069.8 639.7 788.88 180.138
Sort 100k rows ms 669.7 1,005.5 737.8 837.76 132.147
Select 100k rows ms 269.7 350.7 276 291.46 30.175
Deselect 100k rows ms 189.5 218.8 195.1 199.52 10.211

Generated by 🚫 dangerJS against 7a32bc8

@samuelsycamore
Copy link
Member

I went for Picker components as the title of the group instead of Pickers to be more explicit.

Why not simply "Components" for this section? This would match the pattern of the Core docs.

Also, given our recent discussions about formatting component names, I would expect these to be capitalized like proper nouns: Date Picker, Date Range Picker, etc.

@flaviendelangle
Copy link
Member Author

Why not simply "Components" for this section? This would match the pattern of the Core docs.

Because it is only pickers (components with an input and a modal / popper).
The fields (only an input) would be in a different group (see #5565)

Unless we decide to put everything together but that's a lot of components.

@samuelsycamore
Copy link
Member

samuelsycamore commented Oct 3, 2022

Oh I see, I forgot about the original context for this. What about calling the section "Components" and then adding subheaders inside to organize the different kinds of components (like in the screenshot from your comment here)? I don't think it would be too many, especially when compared to Material UI for example which has 50+ pages in the "Components" section.

In general, I would prefer to try to follow the structure we're already using in the Core docs as best as we can, because I think it's safe to assume that most X users start with Core first, and so we should try to maintain continuity with regards to the overall experience to make the jump from one product to the next as smooth as possible.

@flaviendelangle
Copy link
Member Author

@samuelsycamore another question if we do that: how do we avoid confusion between "Components" and "Custom components" ?

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 10, 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 Oct 10, 2022
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 13, 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 Oct 13, 2022
@samuelsycamore
Copy link
Member

@samuelsycamore another question if we do that: how do we avoid confusion between "Components" and "Custom components" ?

I think the "Custom components" page should be renamed 😛 to make it more clear what's in that doc. I'm actually planning to create docs on this same info for the Core libraries, so I'll add that X page to that project.

As for the main topic, I see your point about going too deep with the groupings. I think the way you have it arranged here works well. 👍

Copy link
Member

@joserodolfofreitas joserodolfofreitas left a comment

Choose a reason for hiding this comment

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

I mean, users will click and know what's inside the menu, but I was wondering if "Picker components" doesn't lead to thinking that it's about the components used to build the pickers.

Another consideration: to establish a clear difference from the group of pickers that are coming soon (based on the new fields), would it make sense to rename this group and the next to something like:

=> Legacy Pickers
=> Next Pickers
=> Date and time fields
=> Custom sub-components

=> Masked Pickers
=> Pickers
=> Date and time fields
=> Custom sub-components

=> Masked Pickers
=> Rich keyboard Pickers
=> Rich keyboard Fields
=> Custom sub-components

@flaviendelangle
Copy link
Member Author

flaviendelangle commented Oct 26, 2022

I would avoid to use Masked Pickers because I think most people won't know what "masked" mean.

I would prefer Legacy Pickers / New Pickers, or event Legacy Pickers (v5) / New Pickers (v6) to be very explicit.

Renaming "Custom components" into "Custom sub-components" is fine for me, it think it makes it clear enough.

@joserodolfofreitas
Copy link
Member

joserodolfofreitas commented Oct 26, 2022

I would avoid to use Masked Pickers because I think most people won't know what "masked" mean.

I would prefer Legacy Pickers / New Pickers, or event Legacy Pickers (v5) / New Pickers (v6) to be very explicit.

Renaming "Custom components" into "Custom sub-components" is fine for me, it think it makes it clear enough.

Right.
One of my reasonings for trying to establish a clear difference based on how they work, is that we still need more community validation to "ditch" the pickers based on the masked approach.

@oliviertassinari
Copy link
Member

oliviertassinari commented Oct 30, 2022

I think that the first thing that developers will search for is the type of field that they need: time, date, date time, and only then they will consider the type of UX they need, standalone, keyboard only, integrated, mobile only.

So, I suspect that reversing the grouping could work better. I mean, this is how we structure the URLs/names already:

  • /x/react-date-pickers/date-field/
  • /x/react-date-pickers/date-picker/

@flaviendelangle
Copy link
Member Author

You could go for the following ?

  • Date

    • Date Field
    • Date Picker
  • Time

    • Time Field
    • Time Picker
  • Date Time

    • Date Time Field
    • Date Time Picker
  • Date Range

    • Date Range Field
    • Date Range Picker
  • Time Range

    • Time Range Field
    • Time Range Picker
  • Date Time Range

    • Date Time Range Field
    • Date Time Range Picker

If so, I feel like it's a lot of folders and in the case I would indeed go with headers like @samuelsycamore proposed.

@oliviertassinari
Copy link
Member

oliviertassinari commented Oct 31, 2022

@flaviendelangle I was wondering about something like this indeed. I like the idea of Sam using the menu item subheaders to avoid too much nesting.

@flaviendelangle
Copy link
Member Author

@oliviertassinari if we do that, where would you put the "Getting started" page of the field ?

@oliviertassinari
Copy link
Member

oliviertassinari commented Oct 31, 2022

@flaviendelangle I guess we could remove what is duplicated content with the other pages, move the content for advanced use cases to a guide, and move the introduction content to an overall pickers introduction page.

But I didn't/can't look close enough to judge whether this could be better. Take more this as an initial confusion of somebody (me) that isn't familiar with the evaluation of the component. I landed on https://deploy-preview-6369--material-ui-x.netlify.app/x/react-date-pickers/getting-started/ and wondered "wait, I'm looking for a date picker, no time, no ranges, where can I find this?" The docs felt a bit overwhelming. I like how https://reactdatepicker.com/ is simple, it shows the demos & code upfront rather than telling (they could tell more though, but after the demos).

@github-actions
Copy link

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

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 31, 2022
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 8, 2022
Comment on lines 239 to 241
{ pathname: '/x/react-date-pickers/validation' },
{ pathname: '/x/react-date-pickers/localization' },
{ pathname: '/x/react-date-pickers/fields', title: 'Keyboard editing' },
Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering if those should not be on top of the components, because by priority of reading, I think it's:

  1. Common usage (validation, localization)
  2. Specific usage (Date picker special props)
  3. Customization (slots)
  4. components API

Copy link
Member

Choose a reason for hiding this comment

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

IMHO—the current order makes a tiny bit more sense.
From DX PoV I'd firstly be looking at which component to (can I) use. And only after that consider localisation and validation. 🤔

The only section to me that sounds and looks off is the Keyboard editing... 🙈

I think that we should either update the page title/contents a bit or use a different name.

Maybe going with Field components would make more sense?
And, IMO, this section could use a newFeature: true prop 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

With the new grouping by type, I feel like we are pushing the user to first find its component page and use it as the entry-point for the rest.

I don't think you come to the doc looking for information about the Localization / Validation before first checking how to use the component you need.

On nuance, the Localization pages will contain very basic topic on how to install and use the adapters after #6625 (which is probably even more early-stage in the user needs), but the needed sections will be linked from the getting started.

Copy link
Member Author

@flaviendelangle flaviendelangle Nov 17, 2022

Choose a reason for hiding this comment

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

The only section to me that sounds and looks off is the Keyboard editing... see_no_evil

I think that we should either update the page title/contents a bit or use a different name.

Maybe going with Field components would make more sense? And, IMO, this section could use a newFeature: true prop thinking

@LukasTy

This page contains two quite different kind of content

  1. Basic topics about what fields are. This could probably be moved to the Getting Started page on a new sections explaining that we have 3 type of components: the Pickers, the Fields and the Calendar / Clock. It could also be in a Components page that would be a summary of the available components if we feel that it's to much for the Getting Started/

  2. Advanced topics (the current Advanced section)? This could be moved in an Advanced folder like on the grid with a Field behaviors page. Or if we don't want to create a folder, directly at the root.

I had to put the current page somewhere but I do agree that it is not great.
I can name it Field components (as the alpha only have power users who can take the time to look what "Field" mean) and work on a better split on a follow up PR.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I agree that we can simply name it Field components for now and re-work the whole structure and content in a follow-up PR leaving this one with single responsibility. 😉

Comment on lines 239 to 241
{ pathname: '/x/react-date-pickers/validation' },
{ pathname: '/x/react-date-pickers/localization' },
{ pathname: '/x/react-date-pickers/fields', title: 'Keyboard editing' },
Copy link
Member

Choose a reason for hiding this comment

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

IMHO—the current order makes a tiny bit more sense.
From DX PoV I'd firstly be looking at which component to (can I) use. And only after that consider localisation and validation. 🤔

The only section to me that sounds and looks off is the Keyboard editing... 🙈

I think that we should either update the page title/contents a bit or use a different name.

Maybe going with Field components would make more sense?
And, IMO, this section could use a newFeature: true prop 🤔

Copy link
Member

@oliviertassinari oliviertassinari left a comment

Choose a reason for hiding this comment

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

It more clearly separates the MIT and Pro features, nice 👍


Off-topic

A few opportunities that I could notice while trying the preview link for how it feels to consume the docs:

  1. The static mode isn't static, it forces the focus: https://deploy-preview-6369--material-ui-x.netlify.app/x/react-date-pickers/date-picker/. It's why the scroll bar is broken on this page when loading it. This happens on 4-5 of the pages. It makes it hard to read the docs.
  2. We could disable the ads on some of the pages, e.g. https://deploy-preview-6369--material-ui-x.netlify.app/x/react-date-pickers/date-range-field/.
  3. The live edit demo https://deploy-preview-6369--material-ui-x.netlify.app/x/react-date-pickers/localization/#date-library-locale is broken.

Screenshot 2022-11-15 at 19 08 12

We need to update the logic in the docs-infra of how we extract imports. This https://github.com/mui/material-ui/blob/04d2a26389c8ca299d5d5ab18da3e95a506f7332/docs/packages/markdown/extractImports.js#L1 is too simple. It should be like https://github.com/mui/material-ui/blob/04d2a26389c8ca299d5d5ab18da3e95a506f7332/docs/src/modules/sandbox/Dependencies.ts#L117.
Now, it uses the v1 of the live edit, the v2 makes the demo work when no edits are done. So, the bug will be less visible. TL:DR updating the @mui/monorepo will fix a large chunk of the issue. cc @bharatkashyap if it's a problem you would enjoy working on.

@flaviendelangle
Copy link
Member Author

@oliviertassinari
Concerning the dayjs error, #6625 should solve the issue
Could you confirm using the preview ?

@flaviendelangle flaviendelangle merged commit 2e54cb9 into mui:next Nov 17, 2022
@flaviendelangle flaviendelangle deleted the doc-group-pickers branch November 17, 2022 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: pickers This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants