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

fix: sx variant responsive #1273

Merged
merged 3 commits into from
Nov 23, 2020
Merged

Conversation

atanasster
Copy link
Collaborator

issue: #1030

when a variant was used as an sx prop, its values were merged after applying responsive mods. This PR moves the variant props merging before applying responsive.

The original issue: https://codesandbox.io/s/nameless-sun-hspem-hspem?file=/src/App.js
After the fix (slightly modified): https://theme-ui-responsive-variants.netlify.app

I have seen some other similar issues related to responsive if you guys can check if they are being addressed with the PR @lachlanjc @hasparus

@atanasster atanasster changed the title fix: sx variant responsve fix: sx variant responsive Nov 21, 2020
@lachlanjc
Copy link
Member

Amazing! Does this fix #720 as well?

@atanasster
Copy link
Collaborator Author

atanasster commented Nov 21, 2020

@lachlanjc -- I am not sure. This fixes when the variant is part of the sx prop

<Box sx={{ variant:'xx', p:[1, 5] }} /> - this is affected (fixed)

<Box variant="xx" sx={{ p:[1, 5] }} /> - this is NOT affected (istm that it was working fine before)

@lachlanjc
Copy link
Member

Got it. Do we close #1027 with this?

@hasparus hasparus merged commit ecb50a1 into system-ui:master Nov 23, 2020
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 this pull request may close these issues.

None yet

3 participants