Bugfix: remove Paragraph hardcoded style

[flo-sch]

• fix(components): remove hardcoded Paragraph style
  • remove hardcoded sx props (such style is easy to opt-in with the theme / variants, but harder to opt-out)

Fixes #1476

Ping @atanasster since you commented on the original PR, just for awareness and potential feedback?

[vercel]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/systemui/theme-ui/87h1MaW4hYxQYqA7WVHMgDrQcsia
✅ Preview: https://theme-ui-git-fork-flo-sch-bugfix-remove-paragraph-e01b06.vercel.app

[lachlanjc]

Let's absolutely axe the max width, but I think we should keep the margin: 0 we've currently got. Otherwise, as the source comment indicates, you can use a Theme UI component & end up with non-theme spacing from the user agent styles.

[flo-sch]

Yeah, also uncertain about the margin...
I'm struggling overall with SoC here, is theme-ui intention to do CSS-reset, or should that be the role of a preset // something else than theme-ui?

In the issue, part of the feedback from @aaronadamsCA is:

I think the margin reset probably belongs in a base theme

But theme-ui does not have a "base theme", that's its goal actually (not to have one, allowing anyone to provide such base style for their UI)
At the same time, it is trivial to opt in AND out of that margin with the default paragraph variant 🤷‍♂️

[hasparus]

you can use a Theme UI component & end up with non-theme spacing from the user agent styles.

Only if you change the variant to one you added, innit? Good to merge from my perspective if I'm not mistaken here.

[lachlanjc]

I'm okay to go along with removing the margin, but I don't think it's worth breaking people's designs with this change, especially because I would see it as a bug in Theme UI that components have default margins that are not coming from my theme

[flo-sch]

Only if you change the variant to one you added, innit?

Not necessarily I think, if you do not inherit a preset setting margin: 0 on that default variant, and we remove it from the component itself (as this PR currently does), then "Paragraph" rendering a "p" by default would inherit the browser's margin

[flo-sch]

Alright, then let's re-add the margin reset, which can easily be opted out by setting whichever margin value on the text.paragraph default variant :)

[hasparus]

And it's merged! Thanks a lot @flo-sch!

[vanbujm]

I have been having trouble overriding the margin style.

Can we change:

    <Box
      ref={ref}
      as="p"
      variant="paragraph"
      {...props}
      sx={{
        // reset margin by default: avoid relying on user-agent margins (not aware of theme-ui space scale)
        margin: 0,
        ...sx
      }}
      ...

To:

    <Box
      ref={ref}
      as="p"
      variant="paragraph"
      sx={{
        // reset margin by default: avoid relying on user-agent margins (not aware of theme-ui space scale)
        margin: 0,
        ...sx
      }}
      {...props}
      ...

So that you can use margin props (m, my, mx, etc) to override margin?

[lachlanjc]

Seems fine to me @vanbujm! PR welcome

[vanbujm]

Seems fine to me @vanbujm! PR welcome

PR is up.