Skip to content

Conversation

Janpot
Copy link
Member

@Janpot Janpot commented Sep 12, 2024

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

mui-bot commented Sep 12, 2024

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
Member

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

@Janpot
Copy link
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
Member

oliviertassinari commented Oct 6, 2024

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/.

Copy link
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?

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

@Janpot
Copy link
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
19 checks passed
@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
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
Development

Successfully merging this pull request may close these issues.

4 participants