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] Fix layout shift example page #4350

Merged
merged 3 commits into from
Dec 30, 2024

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Nov 1, 2024

@oliviertassinari oliviertassinari added docs Improvements or additions to the documentation bug 🐛 Something doesn't work labels Nov 1, 2024
@bharatkashyap
Copy link
Member

I can see this still on the PR preview link:

layoutShift.mov

@oliviertassinari
Copy link
Member Author

layoutShift.mov

@bharatkashyap This is another issue, it's a problem with how docs-infra loads the font.

Here the PR fixes the setState:

Screen.Recording.2024-11-03.at.17.28.05.mov

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 22, 2024
@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 Dec 26, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Dec 26, 2024
@oliviertassinari
Copy link
Member Author

oliviertassinari commented Dec 26, 2024

I have noticed problems with the repository spending a bit of time on this:

  • Docs files aren't types checked in the CI

Copy link
Member

@Janpot Janpot left a comment

Choose a reason for hiding this comment

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

looks good. would maybe suggest to extract ${process.env.SOURCE_CODE_REPO}/tree/master into its own variable and reuse between the conditional and the replace but not super important.

@oliviertassinari
Copy link
Member Author

would maybe suggest to extract ${process.env.SOURCE_CODE_REPO}/tree/master into its own variable

We have something equivalent in https://github.com/mui/material-ui/blob/060c55cb219bfc33633f7e61484884ea6d37bfdf/packages/markdown/parseMarkdown.js#L403 for the markdown. We might need to merge those two logics.

Copy link
Member

@bharatkashyap bharatkashyap left a comment

Choose a reason for hiding this comment

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

Looks good!

@oliviertassinari oliviertassinari merged commit 3c9303c into mui:master Dec 30, 2024
14 checks passed
@oliviertassinari oliviertassinari deleted the fix-layout-shift branch December 30, 2024 00:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work docs Improvements or additions to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants