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

Allow preset line height variables to be defined in experimental-theme.json #27100

Open
kjellr opened this issue Nov 19, 2020 · 14 comments
Open
Labels
[Feature] Site Editor Related to the overarching Site Editor (formerly "full site editing") [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Type] Enhancement A suggestion for improvement.

Comments

@kjellr
Copy link
Contributor

kjellr commented Nov 19, 2020

It's currently possible (with Gutenberg 9.4) to assign lineHeight to many blocks within experimental-theme.json:

"core/paragraph": {
    "styles": {
        "typography": {
            "lineHeight": value
        }
    }
}

However, it's not yet possible to define the values for that as a typography preset. Instead, it's necessary to write these as custom variables:

"global": {
    "settings": {
        "custom": {
             "line-height": {
                "small": 1.2,
                "medium": 1.4,
                "large": 1.8
            }
        }
    }
}

(That's actually taken directly from the current documentation. 😄 )

Even if we don't make line heights available in the UI yet, we should consider allowing theme authors to register preset sizes in global => settings => presets => typography => lineHeight.

@kjellr kjellr added [Type] Enhancement A suggestion for improvement. [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. [Feature] Full Site Editing Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json labels Nov 19, 2020
@kjellr kjellr changed the title Allow line height variables to be defined in experimental-theme.json Allow preset line height variables to be defined in experimental-theme.json Nov 19, 2020
@oandregal
Copy link
Member

👋 So, a while ago, a shared that we'd need presets for variables other than colors & font-sizes #23111 My thinking at that point was that they'd be coupled with some UI #23177 #23176 but that proved to be complex UX wise. My concern about lacking presets for those things was lessened when the custom CSS properties were added at #25446 From that on, theme authors could create their own presets without a coupled UI component at their convenience ― it was precisely created with line-height and spacing in mind!

So, how this would be different than what we have today with custom?

@oandregal
Copy link
Member

I've evolved my view on this topic: the line I'd draw here is that we should only have presets that Gutenberg uses somehow. The rest falls in theme-land, as every theme author has different approaches to things and each one might want a different kind of presets.

@kjellr
Copy link
Contributor Author

kjellr commented Nov 20, 2020

I see what you mean — the main issue I'm thinking of here is just a cognitive one: When authors are setting global styles rules for font family, font size, font decoration, and font styles. they're doing it in the global => settings => presets => typography section. It's unexpected to move down to the custom section to do the lineHeight.

Once there's more of a clear need for other common variables to be set there, I think that could go away, but it does feel a little weird.

@AhmadRaza9
Copy link

Can we add group or column height in theme.json...?

@justingolden21
Copy link

This feature would be nice.

Currently, we use just two or three line heights in our design, but we don't want the people in marketing to change it to any value; only specific values, similar to the color palette. This change would probably be very useful for a number of projects.

Thank you.

@richtabor
Copy link
Member

I'm thinking it may be more useful to register line heights for font sizes (or perhaps additionally). What if we did something more like what Tailwind does, where we could register line heights for font sizes?

That would also solve the issue where headings with font sizes may have proper line height, but paragraph text with font sizes need manually applied line height to look better.

Here's what Tailwind does:

line-heights

And here's what we could do:

CleanShot 2022-10-13 at 18 46 20@2x

@markhowellsmead
Copy link

styles.typography.lineHeight and e.g. styles.elements.h1.typography.lineHeight are validating for me against https://raw.githubusercontent.com/WordPress/gutenberg/wp/6.1/schemas/json/theme.json. Or is this about a different setting?

@sunyatasattva
Copy link
Contributor

Stumbled upon this issue and this other issue while searching for a way to pair line heights to font sizes preset. Considering @richtabor's comment has a bunch of upvotes, and the issue has been noticed a few times, I'd love if we could unbury this issue from the dead. Perhaps we could start by agreeing on how the API could look like? @richtabor's proposal seems the most sensible to me. What do you folks suggest?

@markhowellsmead it is about a different setting. This proposal is not about styling a global lineHeight or a lineHeight connected to a block, element, or group of elements. It is about pairing line heights with preset font-sizes selectable from the Typography controls of supported blocks.

@richtabor
Copy link
Member

Thoughts on this @MaggieCabrera and @scruffian? I know you've done a good bit in theme.json/global styles.

@MaggieCabrera
Copy link
Contributor

I think your comment is a good idea, but @afercia has a point mentioning that the UI is incomplete if we only show one of the two (font-size/line-height). If we are pairing them off in the code, that needs to be apparent to the user as well.

@richtabor
Copy link
Member

I'm not quite following. Do you mean that if you select a font size that has a corresponding line height, that you'd expect to see the line height control added as well?

In this scenario, I have line-height of 2 assigned to the XL font size:
CleanShot 2023-05-09 at 15 44 52

I don't think we need to do this, but it would be nice if the default value was adapted for the line height control.

From that on, theme authors could create their own presets without a coupled UI component at their convenience ― it was precisely created with line-height and spacing in mind!

Theme authors can't assign a line height to a font size though.

@MaggieCabrera
Copy link
Contributor

I'm not quite following. Do you mean that if you select a font size that has a corresponding line height, that you'd expect to see the line height control added as well?

not now, but I think it would make sense if presets worked in pairs if we implement what you said on your comment.

@richtabor
Copy link
Member

I don't think we'd need to have a UI representing the line height addition, other than perish allowing the default value set in the control (like it does otherwise).

I think this would be a nice improvement.

@JustinSainton
Copy link
Contributor

I'm thinking it may be more useful to register line heights for font sizes (or perhaps additionally). What if we did something more like what Tailwind does, where we could register line heights for font sizes?

That would also solve the issue where headings with font sizes may have proper line height, but paragraph text with font sizes need manually applied line height to look better.

Here's what Tailwind does:

line-heights

And here's what we could do:

CleanShot 2022-10-13 at 18 46 20@2x

Massive 💯 to this approach. As it stands, having consistent line-height applied to font-sizes is far more laborious than it needs to be. Which is to say, not actually that bad (as we do usually just apply it in CSS) - but it doesn't make sense to apply it to each heading block as much as it does to apply it to a fontSize.

@annezazu annezazu added [Feature] Site Editor Related to the overarching Site Editor (formerly "full site editing") and removed [Feature] Full Site Editing labels Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Site Editor Related to the overarching Site Editor (formerly "full site editing") [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

10 participants