Skip to content

[docs] Fix layout shift example page #4350

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Dec 30, 2024
Merged

[docs] Fix layout shift example page #4350

merged 3 commits into from
Dec 30, 2024

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Nov 1, 2024 •
edited
Loading

@oliviertassinari oliviertassinari added docs Improvements or additions to the documentation. type: bug It doesn't behave as expected. labels Nov 1, 2024
oliviertassinari
oliviertassinari commented Nov 1, 2024
View reviewed changes
@bharatkashyap
Copy link
Collaborator

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

Janpot
Janpot reviewed Nov 3, 2024
View reviewed changes
@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 oliviertassinari mentioned this pull request Dec 26, 2024
Merged
@oliviertassinari
Copy link
Member Author

oliviertassinari commented Dec 26, 2024 •
edited
Loading

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

  • Docs files aren't types checked in the CI

Janpot
Janpot approved these changes Dec 26, 2024
View reviewed changes
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 oliviertassinari mentioned this pull request Dec 27, 2024
Open
@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.

bharatkashyap
bharatkashyap approved these changes Dec 27, 2024
View reviewed changes
Copy link
Collaborator

@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
@oliviertassinari oliviertassinari mentioned this pull request Jan 29, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Janpot Janpot Janpot approved these changes

@bharatkashyap bharatkashyap bharatkashyap approved these changes

Assignees
No one assigned
Labels
docs Improvements or additions to the documentation. type: bug It doesn't behave as expected.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@oliviertassinari @bharatkashyap @Janpot