Skip to content
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.

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

Remove sx props from Paragraph component #1476

Closed
aaronadamsCA opened this issue Feb 4, 2021 · 6 comments · Fixed by #1560
Closed

Remove sx props from Paragraph component #1476

aaronadamsCA opened this issue Feb 4, 2021 · 6 comments · Fixed by #1560

Comments

@aaronadamsCA
Copy link
Contributor

sx={{
// reset margin by default: avoid relying on user-agent margins (not aware of theme-ui space scale)
margin: 0,
// set a max-width: avoid full-page paragraphs
'@media screen and (min-width: 36em)': {
maxWidth: '48rem',
},
...sx,
}}

While I appreciate the intent, this makes it really tough to use Paragraph except for one very specific use case. It takes a specificity hack to defeat it using the theme:

export const theme = {
  text: {
    paragraph: {
      ':not(_)': {
        my: 3,
        maxWidth: 'unset'
      }
    }
  }
}

I think the margin reset probably belongs in a base theme, and the maxWidth is an interesting idea that could be mentioned in the documentation or included in a paragraph variant in one of the themes.

@hasparus
Copy link
Member

hasparus commented Feb 14, 2021

Hi @aaronadamsCA 👋 Thanks for the issue!

cc @lachlanjc @flo-sch

@flo-sch
Copy link
Collaborator

flo-sch commented Feb 14, 2021

Alright, now that you mention it, I agree that such opinionated style should rather be set in a theme.

It is fairly easy to set it globally in the "text.paragraph" variant, and much harder to unset indeed.

(I had similar issues recently with Checkbox, Radio and Switch, ended up building custom components instead)

Should we move that to the base preset perhaps? And/or some others, I do not know all of them, so just mention which ones?
(And remove the hardcoded breakpoint perhaps?)

@emil-wearealldigital
Copy link

emil-wearealldigital commented Feb 21, 2021

Currently margins are not working (<Paragraph my={ 10 }>). It only works with sx - <Paragraph sx={{ my: 10 }}>

@flo-sch
Copy link
Collaborator

flo-sch commented Feb 21, 2021

I am pretty sure sx is the way forward for any kind of style

@hasparus any chance you know what was/is the intent and status behind those "shortcut" props? I am also curious (they are still mentioned in some examples of the doc I think)

@hasparus
Copy link
Member

@flo-sch No idea TBH, mate. I remember jxnblk's comment he wanted to move towards sx everywhere, but I'm not sure if I can find it.

My opinion on this is:

  • You want shorthand style props? Build your own components on top of Theme UI Components (which most likely is sth you'd do anyway when your UI gets complex). Or use Chakra for a ready-made set of UI primitives.
  • Want a high-level layer on top of your className prop similar to style prop but allowing all legal CSS (Emotion) or even fancier shenanigans (That's Theme UI).

@emil-wearealldigital
Copy link

I found something in the documentation... - https://dev.theme-ui.com/components/#style-props

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants