Add text-indent supports for paragraph block #59496
Conversation
MaggieCabrera
requested review from
tellthemachines,
spacedmonkey,
ajitbohra,
ajlende,
ryanwelcher,
juanmaguitar,
fabiankaegy and
ndiego
as code owners
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress. If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged. If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack. Thank you! ❤️ View changed files❔ lib/block-supports/typography.php ❔ lib/class-wp-theme-json-gutenberg.php ❔ lib/theme.json |
|
Size Change: -459 B (-0.03%) Total Size: 1.75 MB
ℹ️ View Unchanged
|
MaggieCabrera
added
[Type] Feature
| @@ -28,6 +28,7 @@ function gutenberg_register_typography_support( $block_type ) { | |||
| $has_line_height_support = $typography_supports['lineHeight'] ?? false; | |||
| $has_text_columns_support = $typography_supports['textColumns'] ?? false; | |||
| $has_text_decoration_support = $typography_supports['__experimentalTextDecoration'] ?? false; | |||
| $has_text_indent_support = $typography_supports['__experimentalTextIndent'] ?? false; | |||
There was a problem hiding this comment.
Should it still be __experimental?
|
This is working as expected but it has some unfortunate side effects when used with the paragraph block - for example the site-title block ends up being indented. This is an issue with the paragraph block though rather than the implementation of this PR. My only question is whether we want to keep adding these properties as __experimental, which is probably a question for @youknowriad |
Can you explain what's going on with the site title block? Also, I think it's fair if this one starts as experimental at least until it lands in core? |
It gets indented because it uses a |
The lack of classnames on Paragraph blocks is becoming a real nuisance. #47282 has been open for a while; might be time to get it shipped 😅 |
|
For whatever reason I couldn't get this to work despite adding the theme.json properties. But thanks for the screenshot, I can give a little feedback based on this. As a blanket property applied to all paragraphs, it seems like text-indent has limited utility. I could see it being useful as a non-default tool that you could indeed apply on a per-paragraph basis, or yes if you're making a particularly old-school book-style theme, to all paragraphs through a Global Styles default.
I'm still personally very hesitant to add classes to every paragraph by default. The paragraph certainly sits in a unique place, since it's a block but it's largely output as an element instead. But I'd rather suggest taking the path that we did with the Button block here, where a button block exists, but you can also style the button element in Global Styles. The utility of styling just the paragraph element, though, is currently limited, as also noted above. If we want text-indent, can we not just apply it like f.x. letter spacing is applied, as an inline style? Or even like font size is applied, as a class if it's not one of the presets?
How does this affect margin collapsing?
If we do end up in a place where the p element is targeted as part of setting the text-indent on the Paragraph block (or element) in Global Styles, I'd say this text indentation also applying to the site logo is "intended", insofar as it's the CSS behavior. You'd presumably also be able to then unset it for the site logo yourself. If it becomes a problem we could explicitly unset it there by default, I'd prefer this over adding a class to the paragraph block. |
|
I am conscious that we don't want to conflate this PR with the unanswered questions about the paragraph block. Is having a text-indent option valuable at a block level for all blocks? |
|
When I mentioned margin collapsing I'm just thinking of the use cases that the original issue is referring to |
|
As far as next steps, as noted, I'm hesitant to add CSS classes and I don't think a blanket text-indent toggle for all paragraphs on a site is that useful. It can be useful on a per-block basis, and if we have that it shouldn't be a problem to also have a global option, but conversely there should also be a local way to un-indent. Finally there might be a conversation to have around whether there's a user expectation that indent is something that can stack, I.e. can I indent three times in one paragraph, two times in another? This is something TinyMCE does:
I think it's risky to add that, it's easy to confuse with the Quote style, but it doesn't come with good semantics for quotes. |
|
@jasmussen please help me understand
as you said in contexts like say a theme that is like an older book this would work, wouldn't your suggestion to use it on "all paragraphs through a Global Styles default" imply this PR to land? |
If you want to apply that indent on all paragraphs, you should be able to, sure! But without also being able to do it on a per-paragraph basis instead, it isn't clear it's all that useful. There's also the aspect of indentation in traditional editors not being just a boolean, but more like pressing tab, indenting a step each keypress. It's not that I don't appreciate the effort, or that the feature doesn't have merit, it's just that we want to make sure and build it in a way that will actually be used to solve a meaningful problem. |
I understand that editing "inline" is probably better in this case, but you are showing the indenting of a paragraph in that video, not the first line. I don't think that's what we want here, is it? |
|
No, it's the first line, I'm thinking in terms of how do we set the width of the indentation? May be the wrong approach. |
Ok that makes sense to me. I think both this PR and your suggestion have tradeoffs. Having a theme global setting would be ideal but it implies a lot to make it happen. Having it inline brings more control to the user and a theme can still provide patterns that use it, so it's worth exploring too. |
|
@MaggieCabrera I wanted to comment again on this issue as it recently came up in a chat with renewed interest. Despite my initial instincts, there were quite a lot of counter instincts that supported a global text-indentation toggle, exactly as you suggested. It's an alternative to paragraphs having space between them, then the indentation serves as a marker for "new paragraph", in the typographical sense. This is counter to the previous instinct I shared, so show me a sword and I will throw myself on it! In that sense, this text-indentation PR can be put in the same general category as justified text (#48202) and pretty text-wrapping (#55190) in that they are mostly global toggles that you flip once for your whole site. These typography issues are also mostly similar in that they are properties that you should probably set globally, once, but rarely or never on a per-block basis. Justified text is my go-to example here, as it's a requirement in some schools and news-rooms to be applied across everything. So we wouldn't want you to have to remember to select every paragraph in a post, set it to justified, and then rinse and repeat for every post and page on your site. You should set it once. Despite the above, I could still see benefit in a local option to set the initial indentation for just one paragraph. So the longer term, probably for all the three global typographical changes, could be to start with a global toggle, and then carefully consider whether to also enable them locally. And in order to get this shipping, it might be good to just go ahead with the initial instinct: not too many options, just globally, and work out a good UI in global styles for how to toggle this on. Let me know what you think! And thanks for working on this. |
|
Thanks for the reflections @jasmussen, it all makes a lot of sense to me. So, let's narrow it down a bit for this PR:
|
MaggieCabrera
force-pushed
the
add-text-indent-supports
branch
from
7a19556 to
e767118
Compare
|
I think we should target the "text" element not a specific block like paragraph. |
by adding the supports, we are allowing theme devs to add it to any block they want, I suppose that's ok? The "text" element is basically targeting |
What?
Partially addresses #37462
This PR introduces the text-indent support and adds it to the paragraph block. This introduces no new UI, we can do that in a follow-up. Themes can make use of the new setting via theme.json.
Why?
More tools for the theme author's arsenal
How?
Using the style engine, following how text-transform and text-decoration are being implemented
Testing Instructions
Add to your theme:
Insert some paragraphs and check the frontend
Screenshots or screencast
Editor:
Frontend:
Questions: