Skip to content
This repository has been archived by the owner on Jun 18, 2020. It is now read-only.

Varya: Gutenberg's line height control does not work in the editor. #37

Closed
kjellr opened this issue Apr 1, 2020 · 5 comments
Closed
Assignees
Labels
bug Something isn't working

Comments

@kjellr
Copy link
Contributor

kjellr commented Apr 1, 2020

Same issue as reported over in Automattic/themes#1895

Gutenberg 7.9 introduces a --wp--typography--line-height CSS variable when the user adjusts the line height in the sidebar. The specificity of Varya's CSS overrides this new rule, preventing it from kicking into effect.

Screen Shot 2020-04-01 at 11 17 35 AM

In general, how should we handle these new WP-provided variables? Should we just start migrating the names of the Varya variables each time one of these new ones becomes available?

@kjellr kjellr added the bug Something isn't working label Apr 1, 2020
@allancole
Copy link
Collaborator

Should we just start migrating the names of the Varya variables each time one of these new ones becomes available?

Yup, that would be ideal. Gonna do some testing in a PR for this but should be pretty straight forward to add this kind of support.

@allancole
Copy link
Collaborator

allancole commented Apr 2, 2020

I did some digging on this its a little tough to follow how it’s intended to work.

There appears to only be one variable --wp--typography--line-height and its doubly used for both paragraphs and headings in kind of an awkward way. This makes it tricky to connect the line-heights that exist in the Varya system since we’re using 2 separate variables for headings and paragraphs.

The CSS-variable is applied to elements inline in the editor and the frontend. Its unclear how the CSS-variables are useful, since the result is an overriding inline style anyway.

image

Not sure what the advantage is for this implementation. It’s essentially an inline style, and it seems to be locked away from any of the responsive or scoping techniques available with non-inline CSS-variables.

It also feels like the new line-height option is a block-specific setting (meaning not global), but the way its implemented can have unintended global effects on the frontend. May need to do some extra styling to make sure that non-gutenberg areas are not impacted.

There’s a few different ways to approach adding this support:

  1. Refactor the line-height system to work the same way it does in core (using a single line-height variable). I’d like to know the why behind using just one variable before pursuing this direction.
  2. Strip out the Varya line-heights variables and rely solely on the ones that come from core / Gutenberg. If we do this, we’d need a way to set default line-heights from the theme for the editor and the front end—not sure if thats possible yet.
  3. We can push for separating the heading and line-height css-variables in Gutenberg so that they’re easier to understand and override from themes or plugins in general.

@jffng
Copy link
Contributor

jffng commented Apr 2, 2020

Thanks for digging and laying that out @allancole! My early thoughts:

  1. Refactor the line-height system to work the same way it does in core (using a single line-height variable).

we could refactor so that either:

a) --wp--typography--line-height is only used on the paragraph block, and we override it for everything else using Varya. This could make sense because (from what I can tell) the line height UI is only exposed on the paragraph block.

or

b) tie --global--line-height-body to --wp-typography--line-height. I don't like this approach because I could see us refactoring every time a small change is made to how the line-height and variables work in core, and this seems to be heavily in flux.

  1. Strip out the Varya line-heights variables and rely solely on the ones that come from core.

This doesn't seem like a good solution because it doesn't give us adequate design control right now. Also since "we’d need a way to set default line-heights for the editor and the front end—not sure if thats possible yet."

I think this is only possible via an experimental-theme.json in the theme and FSE experiment enabled.

  1. We can push for separating the heading and line-height css-variables in Gutenberg so that they’re easier to understand and override from themes in general. We could also suggest a non-inline style method for headings which would make this all easier to understand as well.

I also think this is important: we need to advocate for more sensible design control via the right CSS variables. But that evolution is going to take way longer than our expectations for shipping this theme.

@jffng
Copy link
Contributor

jffng commented Apr 2, 2020

Related Gutenberg PRs:

@jffng
Copy link
Contributor

jffng commented Apr 13, 2020

Solved by #44

@jffng jffng closed this as completed Apr 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants