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

Varya: Add new line-height control support #44

Merged
merged 61 commits into from
Apr 8, 2020

Conversation

allancole
Copy link
Collaborator

@allancole allancole commented Apr 2, 2020

While this works, and addresses both the frontend and backend pretty well. I’m not sure it’s the best approach.

Demo: https://cloudup.com/cyNcpgnlaSp

ToDo

  • Audit line-height variable overrides to make sure it works consistently for gutenberg and doesn’t negatively impact non-gutenberg variables.
  • Consider how child-themes might be impacted by this change.
  • Look into setting a default line-height for headings and paragraphs. The editor shows a 1.5 line-height by default despite what the CSS actually has as the default before changing the option.

jffng and others added 27 commits March 30, 2020 16:31
…t-grid block

- Update variables to include `none`
- Add variable for margin offset
- Separate gutter, background, and padding block options
… tweak [data] margins in editors to prevent unnecessary vertical margins.
…u hover styles for simpler color declarations on hover.
Copy link
Contributor

@jffng jffng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that I'm not sure this the best approach, as it will likely have unwanted side effects for any styles tied to Varya's --global--line-height variables.

For now, I think trying to scope the changes to the individual block level is best. For example, could we do something like:

p {
  line-height: var(--wp--typography--line-height, --global--line-height-body);
}

Once WordPress/gutenberg#21346 and WordPress/gutenberg#21428 land, we can revisit how to integrate the global styles variables.

I think we should consider supplying a theme.json file as well, so we can override the default values of key global styles variables. cc @kjellr

@allancole
Copy link
Collaborator Author

For now, I think trying to scope the changes to the individual block level is best. For example, could we do something like:

p {
   line-height: var(--wp--typography--line-height, --global--line-height-body);
}

Yeah, this feels like a better approach. I’ll give it a shot 👍

@allancole
Copy link
Collaborator Author

@jffng I rebased this against master and tried your suggestion above—works like a charm. LGTM, if you agree :-)

@allancole allancole requested a review from jffng April 8, 2020 18:09
@allancole
Copy link
Collaborator Author

Look into setting a default line-height for headings and paragraphs. The editor shows a 1.5 line-height by default despite what the CSS actually has as the default before changing the option.

I don’t think there’s a solution for this yet in Gutenberg. but I think we can merge this without it.

Copy link
Contributor

@jffng jffng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 🚢

I don’t think there’s a solution for this yet in Gutenberg. but I think we can merge this without it.

Right. Once opt-in support is added for FSE and global styles, we can supply a theme.json file to set the defaults.

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

Successfully merging this pull request may close these issues.

3 participants