[docs] Prepare scripts + E2E tests for migration #3515
Conversation
…o docs/migration-preparation
siriwatknp
force-pushed
the
docs/migration-preparation
branch
from
df7910a to
f7d819d
Compare
github-actions
bot
added
the
PR: out-of-date
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
…o docs/migration-preparation
github-actions
bot
removed
the
PR: out-of-date
docs/data/data-grid/pages.ts
Outdated
| subheader: '/x/react-data-grid/overview', | ||
| title: 'Overview', | ||
| }, | ||
| { pathname: '/x/react-data-grid/demo' }, |
There was a problem hiding this comment.
Not sure about the react- prefix here
Could you explain why you added it instead of just /x/data-grid/* ?
There was a problem hiding this comment.
I believe that this benefits the SEO proposed by @oliviertassinari in https://www.notion.so/mui-org/Documentation-per-product-decision-time-d9347d7d37b641129d2fea12d68555d2#67b6d390ec7c4977b5cb4d942ba2ee6b
There was a problem hiding this comment.
But why react- on components and mui- on the apis ?
Just to have benefits for the SEO for both words ?
There was a problem hiding this comment.
For mui-, I think it relates to the package or the folder in the repo:
/material/api/mui-material/button/material/api/mui-lab/button(heremui-lablives in material)
But this is an interesting question for x packages 🤔, mui- does not make sense here. @mui-org/x @oliviertassinari any thought?
There was a problem hiding this comment.
For API pages it should be /x/api/x-data-grid/data-grid and /x/api/x-data-grid-pro/data-grid-pro, if we want to have the package name in the URL.
There was a problem hiding this comment.
I have polished https://www.notion.so/mui-org/Documentation-per-product-decision-time-d9347d7d37b641129d2fea12d68555d2#67b6d390ec7c4977b5cb4d942ba2ee6b to be more in sync with the current direction. It covers why react-.
Regarding why mui- we have never really talked about it so far. For me, the idea was to match the code imports import Button from '@mui/material/Button. To be honest, there, If it was only about me, I think that I would have done a 1:1 mapping as close as possible with code, so https://mui.com/api/mui-material/button/ (which is different from the current direction). But skipping mui- or adding the product prefix makes sense too. I think that our only concern should be: How clear the API URL is? (not its length).
There was a problem hiding this comment.
In my opinion, the product prefix is already explicit enough and as a user, I don't really care about the package or folder (in the URL). So I am good if we keep the URL the same and just add the product prefix.
/material/api/button/x/api/data-grid/data-grid/x/api/data-grid/grid-api
There was a problem hiding this comment.
@m4theushw @flaviendelangle is this format looks good to you (similar to existing URLs, just prepend with the prefix x)?
- /x/api/data-grid/data-grid
- /x/api/data-grid/grid-api
There was a problem hiding this comment.
The URL seems clean to me that way yes 👍
There was a problem hiding this comment.
Would it be simpler by removing the /data-grid/ prefix? So using the previous example:
- /x/api/data-grid/
- /x/api/data-grid-overview!/
It would match what we do in the other "spaces".
| @@ -195,36 +196,6 @@ Tip: you can access the documentation \`theme\` object directly in the console. | |||
| ); | |||
| } | |||
|
|
|||
| // DO NOT REPLACE THIS FUNCTION WITH THE ONE FROM THE MONOREPO | |||
There was a problem hiding this comment.
Just in case, why was this comment here and did you check that we could replace it now ?
There was a problem hiding this comment.
did you check that we could replace it now
These are what I checked:
- the e2e-test make sure the page does not crash
- the footer of the docs works as before (can link to the next page)
There was a problem hiding this comment.
I suggest you try the migration steps I mentioned in the PR description.
github-actions
bot
removed
the
PR: out-of-date
|
@m4theushw @flaviendelangle Can you review again? I think this is good to be merged. Here is the result once we run the migration (I create a migration demo in this PR): |
…o docs/migration-preparation
This PR is a part of the first phase described in mui/material-ui#30091
Summary
data-grid-current.spec.tsanddata-grid-new.spec.ts(will be tested in the migration phase)yarn docs:migrate:pagesTry it out
yarnyarn docs:migrate:pages && yarn docs:api(the script will setenable_product_scope: truein the feature toggle fileyarn docs:devyarn test:e2e-website:devNext step
Open another PR and run
yarn docs:migrate:pages && yarn docs:api.