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] Reference shared code from the Core monorepo #326

Merged
merged 35 commits into from
Apr 23, 2024

Conversation

michaldudak
Copy link
Member

@michaldudak michaldudak commented Apr 17, 2024

This is a more complete version of #158.
I removed all the common components and left only the ones specific to the Base UI docs site.

@mnajdova you may need to do a similar exercise for the Pigment CSS docs.

Preview: https://deploy-preview-326--base-ui.netlify.app/base-ui/getting-started/

@michaldudak michaldudak added the website Related to the marketing pages label Apr 17, 2024
@michaldudak michaldudak marked this pull request as draft April 17, 2024 10:12
@michaldudak michaldudak marked this pull request as ready for review April 17, 2024 12:08
docs/package.json Outdated Show resolved Hide resolved

export default function Page() {
return <MarkdownDocs {...pageProps} />;
return <MarkdownDocs {...pageProps} disableAd />;
Copy link
Member Author

Choose a reason for hiding this comment

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

Added disableAd for consistency with other pages

Copy link
Member Author

Choose a reason for hiding this comment

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

Copied from the X repo

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Apr 20, 2024
@oliviertassinari oliviertassinari added the docs Improvements or additions to the documentation label Apr 21, 2024
@oliviertassinari oliviertassinari changed the title [website] Reference shared code from the Core monorepo [docs] Reference shared code from the Core monorepo Apr 21, 2024
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.

Nice, great to see this PR

Comment on lines +5 to +8
# Fix links to other MUI products in deploy previews
/base-ui/* /base-ui/:splat 200
/* https://mui.com/:splat 301

Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we should add this:

  1. It's not clear to me that it will work if we redirection /base-ui/ URLs
  2. If it does work, then it sounds like something all repositories should have, e.g. MUI X
  3. But then https://www.notion.so/mui-org/docs-infra-Change-docs-domain-0ce329e7ebc34aba8fbf4f3c5e13eb5d. it might quickly not have value with Base UI in a different URL location.

So maybe to keep this PR a bit more focused (adoption of docs infra != improving docs infra):

Suggested change
# Fix links to other MUI products in deploy previews
/base-ui/* /base-ui/:splat 200
/* https://mui.com/:splat 301

Copy link
Member Author

Choose a reason for hiding this comment

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

I did this to avoid reverting #321. Since we now reference routes from the monorepo, we can't control them.
This will be less important when we publish the site under mui.com, but for now, as deployment previews are the only public docs we have, IMO, it's better to have the links working.
Besides, this way we can try out it if there are any issues with this approach.

Copy link
Member

Choose a reason for hiding this comment

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

Interesting PR (#321), in MUI X/Toolapd the read on this problem was that it was ok. A 404 is clearer than a URL change to figure what MUI page is hosted where.

Now, having these links working helps ensure that once the preview is deployed to production, ahrefs weekly crawl won't find 404 links, so fair enough.

If https://github.com/mui/mui-x/blob/41fb8d93c52eb97557e41c7784a22ac901e06846/docs/public/_redirects#L40 like redirections work, 👍 to propagate this to all the docs-infra consumers.

Copy link
Member Author

Choose a reason for hiding this comment

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

It seems to work. I added a test redirect in d6e1c2c and the site redirects me as expected: https://662678144d579d0008951e2b--base-ui.netlify.app/base-ui/test-redirect/

Copy link
Member

Choose a reason for hiding this comment

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

Nice. We will see how this behaves once integrated in mui.com 👍.

I wonder how we will plan the mui.com/base-ui/ to base-ui.mui.com/ migration.

@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 Apr 22, 2024
Copy link
Member

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

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

Nice cleanup, I left just one question. Maybe someone from @mui/docs-infra could check it too.

headers: PropTypes.object.isRequired,
}).isRequired,
};
export { default } from 'docs/src/modules/components/ComponentLinkHeader';
Copy link
Member

Choose a reason for hiding this comment

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

Why are this and ComponentPageTabs not exported from @mui/doc?

Copy link
Member Author

Choose a reason for hiding this comment

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

They must be present locally as they are included in markdown files (with the {"component": ... syntax)

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Apr 22, 2024
@michaldudak michaldudak requested a review from a team April 22, 2024 18:38
@michaldudak michaldudak merged commit d40161b into mui:master Apr 23, 2024
16 checks passed
@michaldudak michaldudak deleted the docs-cleanup branch April 23, 2024 19:59
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 website Related to the marketing pages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants