mui / mui-x

MUI X: Build complex and data-rich applications using a growing list of advanced React components, like the Data Grid, Date and Time Pickers, Charts, and more!
https://mui.com/x/
3.82k stars 1.14k forks source link

[docs] Revise and split up "Overview" page into "Introduction" section #4692

Closed samuelsycamore closed 1 year ago

samuelsycamore commented 2 years ago

Doc preview: https://deploy-preview-4692--material-ui-x.netlify.app/

This PR revises and splits up the content on the Overview page.

Follow up to: Add and/or revise "Overview" pages for each MUI product

The new Introduction section features 5 pages:

I rewrote the content on the Overview page to match the formatting of the Base Overview page.

mui-bot commented 2 years ago

These are the results for the performance tests:

Test case Unit Min Max Median Mean σ
Filter 100k rows ms 248.6 509.7 377.9 368.54 102.379
Sort 100k rows ms 558.1 1,083.7 558.1 825.1 199.768
Select 100k rows ms 154.5 283.1 192.9 198.02 45.162
Deselect 100k rows ms 108.5 220.5 170.3 167.76 43.958

Generated by :no_entry_sign: dangerJS against 1b259aa850c6504be36163fee71df906dcbe4577

oliviertassinari commented 2 years ago

We would be breaking https://github.com/mui/mui-store/blob/6353c17aa61c29a13e063382b65f75a9a592f695/lambda/keymailer/index.js#L7 a second time, I think that it's time to use the /r/foo bar 302 pattern as customers might go back to their older emails and follow the link.

oliviertassinari commented 2 years ago

Started in: https://github.com/mui/mui-x/pull/4711/files#diff-7a0cbac74402caf0e99ba338a9e7c42684580a1f85e149110c3c2ca6e38055afR7.

cherniavskii commented 2 years ago

Try switching to master and run git pull, then go back to this branch, run git merge master and commit.

@m4theushw @samuelsycamore I would wait with this until https://github.com/mui/material-ui/pull/32552 is merged, otherwise docs:dev won't work

cherniavskii commented 2 years ago

@samuelsycamore

I'm not sure why, but the doc switcher widget at the top of the nav bar isn't showing up on the new pages I created. Maybe someone can help me figure out how to make it come back?

Ok, it needs a change in the core repo right here: https://github.com/mui/material-ui/blob/f4818344868ffeaff31e613be90edaf82d3c801e/docs/src/modules/components/AppNavDrawer.js#L656

diff --git a/docs/src/modules/components/AppNavDrawer.js b/docs/src/modules/components/AppNavDrawer.js
index 2e5f8225f05..07c1b4fec9f 100644
--- a/docs/src/modules/components/AppNavDrawer.js
+++ b/docs/src/modules/components/AppNavDrawer.js
@@ -653,7 +653,7 @@ export default function AppNavDrawer(props) {
               ])}
             />
           )}
-          {asPathWithoutLang.startsWith('/x/advanced-components') && (
+          {asPathWithoutLang.startsWith('/x/getting-started') && (
             <ProductIdentifier name="Advanced components" metadata="MUI X" />
           )}

I can open a PR addressing this or feel free to open one if you want :) After merging it, @mui/monorepo commit should be updated in this PR:

https://github.com/mui/mui-x/blob/cfc9a6f824fd45ce6e8a59e214f9ae22a8c58853/yarn.lock#L2602-L2604

github-actions[bot] commented 2 years ago

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

flaviendelangle commented 2 years ago

@cherniavskii for the monorepo PR to fix the link, we can avoid temporarily breaking stuff by just handling both links starting now and remove the legacy one later

-          {asPathWithoutLang.startsWith('/x/advanced-components') && (
+          {(asPathWithoutLang.startsWith('/x/advanced-components')  || asPathWithoutLang.startsWith('/x/getting-started')) && (
cherniavskii commented 2 years ago

I've opened https://github.com/mui/material-ui/pull/32657 to address missing product identifier component

oliviertassinari commented 2 years ago

The new separation of the content looks great, I think that we are heading in the right direction. A few notes:

  1. Something is off with the auto menu expansion: https://deploy-preview-4692--material-ui-x.netlify.app/x/getting-started/support/. We should have:
Screenshot 2022-05-07 at 22 04 11

It's increasingly disorienting without.

  1. This comment https://github.com/mui/mui-x/pull/4692#issuecomment-1112680762 is taken care of. See https://github.com/mui/mui-store/pull/160. We now need to rebase this PR and update the redirection to point to the right page.
samuelsycamore commented 2 years ago
  1. Something is off with the auto menu expansion

Do you have any idea what could be causing this? I see what you're describing, but as far as I can tell it's only happening in the deploy preview. I can't reproduce that behavior locally—it works as expected.

In the deploy preview it appears to happen with all of the pages under Getting started, with the exception of Overview.

cherniavskii commented 2 years ago

@samuelsycamore

Do you have any idea what could be causing this? I see what you're describing, but as far as I can tell it's only happening in the deploy preview. I can't reproduce that behavior locally—it works as expected.

It's an old deploy preview, the one with the fix failed - should be fixed after resolving merge conflicts and triggering new redeploy

samuelsycamore commented 2 years ago

Thanks @cherniavskii, that's a relief—I was stumped! 😅

samuelsycamore commented 2 years ago

After merging it, @mui/monorepo commit should be updated in this PR:

https://github.com/mui/mui-x/blob/cfc9a6f824fd45ce6e8a59e214f9ae22a8c58853/yarn.lock#L2602-L2604

I'm getting an error regarding the monorepo now after merging the master into this branch. Would this address it?

Module not found: Can't resolve '../../node_modules/@mui/monorepo/docs/src/modules/utils/CodeCopy'

cherniavskii commented 2 years ago

@samuelsycamore I'll take a look. Would you mind if I commit to this branch with a fix?

samuelsycamore commented 2 years ago

@cherniavskii please do! 🙌 This is turning out to be more complex than I expected. I appreciate your help.

cherniavskii commented 2 years ago

@samuelsycamore I don't see any issues (tried both docs:dev and docs:build). Did you install dependencies after merge (yarn install)?

samuelsycamore commented 2 years ago

@cherniavskii that fixed the errors I was seeing locally. But it's still failing to deploy here.

cherniavskii commented 2 years ago

@samuelsycamore looks like an issue with Netlify. Plus an argos issue :D I'm gonna push an empty commit to trigger it again - maybe that would help

cherniavskii commented 2 years ago

Ok, I had to go through all deployments for this PR and cancel all stalled ones 🙃 But at least this deployment started

cherniavskii commented 2 years ago

To fix 404 when going to https://deploy-preview-4692--material-ui-x.netlify.app/ we need this change:

index 51892abcc..0b731011d 100644
--- a/docs/public/_redirects
+++ b/docs/public/_redirects
@@ -1,4 +1,4 @@
-/ /x/advanced-components/
+/ /x/getting-started/overview/

 # For links that we can't edit later on, e.g. hosted in the code published on npm or sent by email
 # should all be prefixed with x-
samuelsycamore commented 2 years ago

Sheesh, thank you for doing that @cherniavskii !

github-actions[bot] commented 2 years ago

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

github-actions[bot] commented 1 year ago

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

samuelsycamore commented 1 year ago

I have added an Installation page that covers the Data Grid and the Date Pickers. This Introduction section just feels incomplete without it, and the Getting started pages have a lot of other stuff going on. I also noticed that the Date Pickers don't have complete installation instructions that I could find anywhere. I'd like to update the Getting started pages for both packages next, but that can wait for a new PR.

I have also made copy revisions throughout, most notably on the Licensing page.

samuelsycamore commented 1 year ago

Broken links and redirects should be resolved now. I'll double check once the preview redeploys ✅ LGTM. In the meantime, I'll create a PR in the Core repo to update any broken links on that side so it will be ready when this goes up.

samuelsycamore commented 1 year ago

Thanks for the edits @mbrookes! I think this content is looking super sharp now. 👍

oliviertassinari commented 1 year ago

I have tried to polish the content as much as I could. I love it! So much better vs. HEAD. I also like how it reinforces what MUI X is about vs. Material-UI, same as in https://mui.com/material-ui/discover-more/roadmap/#quarterly-roadmap.

mbrookes commented 1 year ago

Merging in 3, 2, 1...