Skip to content

Conversation

@oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Feb 3, 2024

@oliviertassinari oliviertassinari added type: bug It doesn't behave as expected. scope: docs-infra Involves the docs-infra product (https://www.notion.so/mui-org/b9f676062eb94747b6768209f7751305). labels Feb 3, 2024
@oliviertassinari oliviertassinari force-pushed the fix-arbitracy-callout-gap branch from b9c8c7b to f9bed90 Compare February 3, 2024 17:37
@mui-bot
Copy link

mui-bot commented Feb 3, 2024

Netlify deploy preview

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

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against b4afd2f

@oliviertassinari oliviertassinari added the type: regression A bug, but worse, it used to behave as expected. label Feb 3, 2024
Copy link
Collaborator

@danilo-leal danilo-leal 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! I just don't understand why this is considered a regression. What was broken with the previous code? Maybe I'm not understanding, or it's worth clarifying in the PR description — not obvious from the screenshot diff. It looks more like an improvement than a bug fix to me.

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Feb 3, 2024

What was broken with the previous code?

@danilo-leal The #38525 PR introduced a custom gap between paragraphs: https://github.com/mui/material-ui/pull/38525/files#diff-77f04bc58bbbb50bc7d3dea9ec533b08d76f110c581114cda8105da367d236a5R283

The CSS no longer relies on the default margin-bottom value. I think it's a regression because:

  • It doesn't look consistent, or correct (too short of a gap, it should be at least larger than the line gap inside the same paragraph).
  • It adds unnecessary complexity, defining the gap for all the typography seems enough.

This PR reverts to how it was before.

@oliviertassinari oliviertassinari force-pushed the fix-arbitracy-callout-gap branch from b649901 to b4afd2f Compare February 3, 2024 21:10
@oliviertassinari oliviertassinari enabled auto-merge (squash) February 3, 2024 21:11
@oliviertassinari oliviertassinari merged commit 7c6c54e into mui:master Feb 3, 2024
@oliviertassinari oliviertassinari deleted the fix-arbitracy-callout-gap branch February 3, 2024 21:40
@oliviertassinari oliviertassinari removed the type: bug It doesn't behave as expected. label Apr 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

scope: docs-infra Involves the docs-infra product (https://www.notion.so/mui-org/b9f676062eb94747b6768209f7751305). type: regression A bug, but worse, it used to behave as expected.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants