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

Values of space keys when referenced in variants are not populated in CSS #1027

Closed
mirabilio opened this issue Jun 25, 2020 · 4 comments
Closed

Comments

@mirabilio
Copy link

mirabilio commented Jun 25, 2020

Describe the bug

Create an array assigned to a space key for responsive values (called "lg" in this issue). Use that key in a variant for a property. Theme-ui does not populate those values in the CSS as expected. Using [...theme.space.lg] does work, however.

To Reproduce

  1. Use this theme:
theme.space = [
  "0", 
  "0.25rem",
  "0.5rem", 
  "0.75rem",
  "1rem",
  // ...
];
theme.space.sm = [1, 2, 3, 4, 5]; // step 1
theme.space.md = [2, 4, 6, 8, 10]; // step 2
theme.space.lg = [3, 6, 9, 12, 15]; // step 3
theme.space.xlg = [4, 8, 12, 16, 19]; // step 4

theme.people = {
  mx: "lg", // margin-right and margin-left are not present in CSS. However, [...theme.space.lg], DOES work. 
}
  1. Render a component a la:
const People = ({people}) => {
  // [...]
  return (
    <Grid sx={{ variant: "people" }}>
      {peopleList}
    </Grid>
  )
}

Expected behavior
margin-right and margin-left values should be populated in People component's CSS like so:

@media screen and (min-width: 1366px)
.css-6857ss-People {
    grid-auto-rows: min-content 1fr;
    grid-template-columns: 1fr 2fr;
    margin-left: 12rem;
    margin-right: 12rem;
    grid-gap: 1.25rem;
}
@media screen and (min-width: 1024px)
.css-6857ss-People {
    grid-auto-rows: min-content 1fr auto;
    grid-template-columns: 1fr;
    margin-left: 6rem;
    margin-right: 6rem;
    grid-gap: 1rem;
}
// [...]

The above CSS results when using theme.people definition:

theme.people = {
  mx: [...theme.space.lg]
}

Actual behavior
margin-left and margin-right are missing:

@media screen and (min-width: 1366px)
.css-149upde-People {
    grid-auto-rows: min-content 1fr;
    grid-template-columns: 1fr 2fr;
    grid-gap: 1.25rem;
}
@media screen and (min-width: 1024px)
.css-149upde-People {
    grid-auto-rows: min-content 1fr auto;
    grid-template-columns: 1fr;
    grid-gap: 1rem;
}
// [...]
@hasparus
Copy link
Member

I don't think you can alias a responsive tuple.
ATM They're not values you can keep in the theme.
It's just a syntax for input, i.e. JSX pragma transforms arrays in the style object.

You don't have to shallow copy it, theme.space.lg would also work.

{
  space: {
    sm: [1, 2, 3, 4, 5]
  }
}

This is treated as a nested scale. You can do marginRight: 'space.sm.2', similarly to colors.blue.900

This would be a pretty awesome feature, especially since we can do marginRight: t => t.space.sm, and it will work, so it's a bit inconsistent right now.

@jxnblk
Copy link
Member

jxnblk commented Jun 26, 2020

I think this is a slight difference in understanding how variants and scales work. Scales (e.g. space) can be nested however you like, but they do NOT support responsive array values like the sx prop or variants do. In order to use responsive arrays as values, they MUST be in either a variant or the sx prop, if that makes sense

@mirabilio
Copy link
Author

Thank you @jxnblk for the clarification! What is the recommended best practice for achieving reusability of responsive scale arrays with override ability?

@jxnblk
Copy link
Member

jxnblk commented Jun 30, 2020

Overriding responsive values from a variant doesn't work as well as it should at the moment, but it's something we'd like to address for v1, see #832

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

No branches or pull requests

3 participants