Paragraph: Add LineHeightControl + Attribute #20775
Conversation
ItsJonQ
added
[Block] Paragraph
ItsJonQ
requested review from
ajitbohra,
chrisvanpatten,
ellatrix,
jorgefilipecosta,
Soean,
talldan and
youknowriad
as code owners
| "customFontSize": { | ||
| "type": "number" | ||
| }, | ||
| "direction": { | ||
| "type": "string", | ||
| "enum": [ "ltr", "rtl" ] | ||
| "enum": [ | ||
| "ltr", |
|
Size Change: +444 B (0%) Total Size: 856 kB
ℹ️ View Unchanged
|
|
I am very much against this going in as a control for individual blocks. In my opinion, this is pure bloat; an option that gives too much flexibility and tries to solve a problem the wrong way. Any line height option should be handled at the global styles level; and I don't mean changing this for every Paragraph block... I mean this should be a setting at the same level as a site wide typography scale. There is no good reason to add this kind of control to individual blocks like this. If a user is needing to change the line height on a single block, then that's a sign that the global styles were not good enough to begin with. The problem should be solved at the source, not with a band-aid like this. Please, do not add this control to the Paragraph block (or any block, really). |
|
I should note that I'm fine with adding a component for this that could be used in global styles; I'm just extremely against this kind of control being provided at the individual block level. |
|
I disagree — most of the patterns being designed cannot be accomplished without control over line-height at the block level. If there is control over font-size, whether exposed to users or not, there should be control over line-height as they go hand in hand.
This is entirely complimentary to global styles handling a typography scale, the theme defining its own defaults, and so on. The block itself needs to understand the attribute, so this is a step in that direction. It should be made easy to disable globally like other customization tools, of course, and we could debate the argument of whether it should be exposed by default or implicit on any defined pattern. |
|
@mtias Why not just make line heights part of the font size presets? We should definitely be encouraging the use of the font size presets rather than custom pixel sizes, and if we already have presets, why not automatically set the appropriate line height? (These font size + line height combos could be defined at the theme level or at the global styles level.) We already add padding when selecting a background color on various blocks, so the precedent for such a thing is already there. The user shouldn't have to be responsible for selecting the right line height to go with a font size. They shouldn't even have to think that much about the font size they want to use. They should be able to pick from a few fine-tuned presets and have it work by default with no extra fiddling around. |
|
I think we are mostly talking about the same thing, in a way :) For visual effect, there will still be cases where you need specific control over it.
This doesn't mean that we cannot absorb most common cases in pairs (this we should do regardless for the main font-sizes) and global styles / theme, as you say, but it does mean we need the concept of its control at the block level somehow existing too. |
|
@mtias I think I get the idea of that example, but even in that situation, such an intentional visual effect is likely to be used in multiple places, and so something like that example would be far more suited to a block style variation for the Paragraph block (and/or Heading block). Intentional stylistic choices like that should be the responsibility of the website designer, and good design reuses parts to create a cohesive whole. Therefore, style variations seem like the proper way to implement this. By adding a line-height control to individual Paragraph blocks, you encourage one-off modifications. You bring to the forefront a "solution" to a problem, but it's the wrong solution in nearly every situation. You give the content creator more power, but also more responsibility... responsibility that what supposed to be handled by the site designer (whether that be the theme author or the person managing the global styles or both). Individual block options should be designed around carefully crafted presets set by the designer, thus keeping the responsibility of design with the designer, not the content editor. I recently read a very interesting blog post that points out how Gutenberg seems to be blurring the lines of responsibility with its increasing amount of style options on individual blocks. But it doesn't have to be that way. We already have the block style variations API, and global styles are going to become a thing. Only weird one-off cases are not handled, and one-off cases should not be encouraged, in my opinion. I think a better solution to this problem would be to provide an easier way to create/register a style variation. We could provide a visual interface for creating style variations so people don't have to write CSS; in that UI, there can be a line height control, as well as a zillion other advanced style options. But in the end, that style variation will be just a class applied to a block. There's no need to add a line height attribute to any block. |
|
Additionally, it follows based on my reasoning that the custom font size option probably shouldn't even exist for individual blocks. That may be a controversial stance, and it's definitely useful right now since global styles are not yet implemented and people can't easily create style variations, but in the long term, the custom font size option is not something that should be exposed for individual blocks. In the long run, custom font sizes should be opt-in, not opt-out. As it is, themes have lost control over font size consistency. At least in the case of font sizes, the custom option is somewhat buried, with the preset options presented first. The same can not be said of this line height control. |
|
I know this is probably going to come with the global styles work, but it'd be neat to see a "make default" option, so if you change the setting you can persist it back to the global configuration. I do strongly feel like this would be a good candidate for the features API proposed in #20623, or at the very least something with the theme supports we currently have ( |
|
I agree that at the very least, themes should be able to disable this feature similar to custom font sizes and custom colors. Also, another thing I wanted to point out: in the aforementioned stylistic flair example posted by @mtias, the person wanting to do that could just as easily have wanted to also use a different font weight or letter spacing. Should we be adding font weight and letter spacing controls to the Paragraph block? I don't think so. |
@ZebulanStanphill this is not necessarily the case. The actual appearance attributes need to make it to the block as attributes, otherwise we cannot ensure true WYSIWYG. Also mobile apps don't understand class names with arbitrary CSS attached, they will need to consume agreed upon attributes. The way global styles would work is by traversing the actual appearance attributes within the editor so that they can be consumed at the block level as well. This is paving the way for that, if seen beyond the mere presence of the line-height control on the block, as it makes the block understand an attribute. The presence of a control by the block is just an artefact. We also need to see how all of these should eventually be grouped in a style panel on the block, including ways to make changes the default (or save style variations) but all these layers require we build the tools, otherwise we are chasing ghosts.
There's a multitude of use cases (one of pages, brochures, eclectic presentations) and we should not force our own preference on what site design should be on all people. Most block libraries have added these controls, but the implementation ends up being generally done through inline styles which hurts the user in the longer term. By taking on that mantle we can ensure consistency and build the right levels of control, which is one of the main reasons to do this in core, to avoid fragmentation and ensure the user stands on solid ground. Site builders will be able to retain full control by not enabling / disabling certain appearance attributes. Once it's all properly codified, that should be an easy configuration set globally and cascading properly to blocks because they understand what to do with the attributes. This is also true if we want global styles to transpire to third-party blocks. It's important to separate the fact getting the controls in place doesn't mean that everything will be exposed to end users. Most of the design tools, including global styles, theme editor, etc, are likely going to be admin level to begin with. The reality remains that global styles cannot work without also building the tools, and starting on individual blocks is more ergonomic than trying to solve everything at once at the global level. It also happens that patterns need more fine grained control over these attributes.
@chrisvanpatten agreed, this was in one of the original block style variations issues as one of the future tasks. Also, let's move the main conversation to the issue: #20339 ! |
|
One thing I want to observe is in regards to As noted in the original post above, this is indeed a magical hook! I've been wondering for a while if this type of pattern might crop up. I think it's a pattern that clearly has its use-cases — I fully switched our codebase from As awesome as hooks are, they do mask away some explicitness in favor of magic. And as great as magic can be, I think it's important to balance alongside readability and clarity. Again, this isn't to say that we should go one way or the other… just wanted to share some musings that I've had recently that this hook brought to mind in particular. I think technically the work is super interesting here, and seems like a great place to trial some exciting new APIs! |
Yes, this is a tricky thing to balance. Explicitness is sound when it comes to the main attributes of a block, but when we start dealing with shared attributes, it becomes unnecessarily verbose and tedious to add support. Adding support for text and background color means adding four new attributes at the moment! That's why I asked to explore the notion of implicit attributes as well: #20308 |
|
First, I am super excited to see this. There is a strong case for line-height to create designs and really adjust to make the perfect layout that people envision in their head. @ZebulanStanphill whilst I understand your point of view on this, I would ask you to consider a few points. If we are to truly give control, I don't think a range hits the right spot. There are many cases where more granular control really is what is needed. For sure we should offer strong defaults, that is where ideas like having a range control for typography scale come in. We do need to offer some tools we don't right now to empower people to create. This doesn't always work per block, sometimes we're going to need to do globally, sometimes per block. However, we do need to open up the tools we offer to provide a powerful kit to create what people really want. It's exciting if we do that, because there are going to be so many opportunities for incredible things to be made. |
Co-Authored-By: Zebulan Stanphill <[email protected]>
youknowriad
force-pushed
the
try/paragraph-line-height-control
branch
from
1aea158 to
777f079
Compare
|
I rebased this branch, for me aside potential test failures (wait for travis), this is ready. |
|
Thank you so much @youknowriad !! 🤗 |
|
Oh boy!! Looks like Travis is green 🙌 Will merge it up! |
|
@ItsJonQ when disabling both font sizes and line height, I still see the Typography panel. Shouldn't this be removed? |
|
Late to the party. While I do realize there's sometimes the need to set custom line-heights, I also do share some of @ZebulanStanphill's concerns. General considerations:
Architectural considerations:
Implementation considerations:
Current user interface shortcomings:
|
Description
This update adds a new
lineHeightattribute to the core Paragraph block. Accompanying it would be a new dedicated (and experimental)LineHeightControl, which is provided by@wordpress/block-editor.How has this been tested?
This has been tested on local Gutenberg on multiple themes, from 2011 -> 2020.
Screenshots
GIF demonstrates the live control edit -> rendering for line heights.
Types of changes
A majority of the updates in this PR hopes to improve the relationship + setup of attributes and controls.
A goal for
LineHeightControlis to be implemented with minimal code changes for a Block's Edit/Save components.LineHeightControlTaking a look at the
Paragraphedit.js, the newLineHeightControlhas been added underInspectorControlwithout requiring any props.The component is able to retrieve the appropriate
attributesandsetAttributeson it's own. This workflow makes it much simpler to add controls to Edit components - especially if the control requires multiple moving parts.useBlockEditProps()This new experimental hook from
@wordpress/block-editor. This magical hook (which leverages other new experimental hooks) is able to retrieve a block'sattributesandsetAttributeswithout requiring anything to be passed in.The new
LineHeightControlrelies on this hook as the getter/setter mechanism for thelineHeightvalue.If folks are 👍 with this approach, I can see it being extremely useful for attributes x controls moving forward 🙌
Unitless
To keep things simple, the
line-heightvalue will be unitless and works as a multiplier to the Paragraph's font-size. This more hands-off approach allows for greater flexibility for font-size x line-height.Control Styles
Some
@emotionpackages were added as a dependency to@wordpress/block-editorto enable CSS-in-JS style rendering forLineHeightControl. These are the same packages used internally within@wordpress/components.UX Interactions
To improve the UX for
LineHeightControls, I implemented some logic to better handle incrementing/decrementing values from the start.For example...
If you're adjusting
line-heightfor the first time, and you pressUP, the value would be1.6, rather than the defaultinputbehaviour of0.1.Alternatively, if you press
DOWNfor the first time, the value would be1.4, rather than0.This is because it uses a smart default of
1.5. That being said,1.5isn't actually applied. There is noline-heightstyle set until the user makes a change.Afterwards, incrementing/decrementing works as normal.
Removing the value would reset it entirely.
During development, I found that the above behaviour adjustment was key. Without it, the initial increment (
UPpress), would cause all of the text to collapse, as it would use aline-heightvalue of0.1.Tests
I would love to write tests for this new control. I attempted to do this, but I wasn't able to successfully mock the new
useBlockEditPropshook, which technically could be replaced withuseState. Jest kept complaining 😅Closing Thoughts
Lots of changes in this PR! I'm open to anything. On the surface, adding a new
line-heightproperty to Paragraph is a very minor thing. The bigger goal though would be to systematize and make it easier for blocks (both core and 3rd party) to set up attributes and render their respective controls.Thank you! 🙏
Checklist:
Partially takes care of:
#20339