Skip to content

[docs] Remove HighlightedCode max-width#43731

Merged
Janpot merged 6 commits intomui:masterfrom
Janpot:remove-max-width
Oct 16, 2024
Merged

[docs] Remove HighlightedCode max-width#43731
Janpot merged 6 commits intomui:masterfrom
Janpot:remove-max-width

Conversation

@Janpot
Copy link
Copy Markdown
Member

@Janpot Janpot commented Sep 12, 2024 •
edited
Loading

This messes up a bit the copy button positioning: https://app.argos-ci.com/mui/material-ui/builds/31842/107814670

Not sure why it's there. Removing to see if I can find something breaking. I can't find an instance of this component where this CSS can have an impact 🤔

@mui-bot
Copy link
Copy Markdown

mui-bot commented Sep 12, 2024 •
edited
Loading

Netlify deploy preview

https://deploy-preview-43731--material-ui.netlify.app/

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against 5e4c4a5

@Janpot Janpot added docs Improvements or additions to the documentation. type: enhancement It’s an improvement, but we can’t make up our mind whether it's a bug fix or a new feature. labels Sep 12, 2024
@oliviertassinari
Copy link
Copy Markdown
Member

oliviertassinari commented Sep 13, 2024

This was needed to fix mobile overflow. I think to check that the initial cases are not broken.

@Janpot
Copy link
Copy Markdown
Member Author

Janpot commented Sep 23, 2024

This was needed to fix mobile overflow. I think to check that the initial cases are not broken.

Do you remember which use-case this was fixing? I've been looking around at a bunch of code blocks and I can't figure out what this is trying to achieve. Seems to have initially been added in #27488.

Opening a separate ticket for some other issues I noticed.

@Janpot Janpot marked this pull request as ready for review September 23, 2024 07:25
@Janpot Janpot requested a review from a team September 23, 2024 07:26
@oliviertassinari
Copy link
Copy Markdown
Member

oliviertassinari commented Oct 6, 2024 •
edited
Loading

The origin seems to be #20108. A reproduction of the bug: https://v4.mui.com/blog/march-2019-update/.

Ok, it seems fine: https://deploy-preview-43731--material-ui.netlify.app/blog/march-2019-update/.

alexfauquette
alexfauquette approved these changes Oct 14, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@alexfauquette alexfauquette left a comment

Choose a reason for hiding this comment

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

I did not found any issue introduced by this PR.

What about doing the same modification to the MarkdownElement.tsx file?

material-ui/packages/mui-docs/src/MarkdownElement/MarkdownElement.tsx

Lines 35 to 39 in bfe28ad

maxWidth: 'calc(100vw - 32px)',
maxHeight: '400px',
[lightTheme.breakpoints.up('md')]: {
maxWidth: 'calc(100vw - 32px - 16px)',
},

@Janpot
Copy link
Copy Markdown
Member Author

Janpot commented Oct 14, 2024

What about doing the same modification to the MarkdownElement.tsx file?

At least in screenshots that seems to fix a few more things: https://app.argos-ci.com/mui/material-ui/builds/32870/113602375

Can't find any regressions on pages that use it

@Janpot Janpot merged commit e3ec0c7 into mui:master Oct 16, 2024
@Janpot Janpot deleted the remove-max-width branch October 16, 2024 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alexfauquette alexfauquette alexfauquette approved these changes

Assignees

No one assigned

Labels

docs Improvements or additions to the documentation. type: enhancement It’s an improvement, but we can’t make up our mind whether it's a bug fix or a new feature.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Janpot @mui-bot @oliviertassinari @alexfauquette