search

WordPress / twentytwentytwo

Twenty Twenty-Two, the default WordPress theme that will launch with WordPress 5.9.
406 stars 91 forks source link

Consider using spacing.large value in place of hard-coded 8rem styles in patterns #317

Closed richtabor closed 2 years ago

richtabor commented 2 years ago

Is your feature request related to a problem? Please describe.

This is related to #309 — which I've submitted a PR for, to connect the small spacing value to the theme.json entry. In that PR, I'm using the spacing.small theme.json value, instead of hard-coded "small" spacing in patterns.

We're also using 8rem throughout the patterns — as a hard-coded value for large spacing.

Describe the solution you'd like Let's consider adapting those values to be connected to theme.json? We don't have a matching theme.json settings.custom.spacing entry for 8rem exactly, but we do have: "large": "clamp(4rem, 10vw, 8rem)"

We should test if we can use settings.custom.spacing.large as var(--wp--custom--spacing--large, 8rem) in place of the hard-coded 8rem values within the patterns, to achieve the same effect as that PR I submitted for the small spacing values.

Screenshot:

CleanShot 2022-01-03 at 12 06 07@2x
kjellr commented 2 years ago

I think that should work fine. I believe I used 8rem in there before I created that var(--wp--custom--spacing--large) variable, and never got around to updating the existing patterns.

richtabor commented 2 years ago

@kjellr Do you want me to update #316 with this change too? Or keep them separate?

kjellr commented 2 years ago

I think it's fine to do them in the same PR. 👍