-
Notifications
You must be signed in to change notification settings - Fork 4.7k
Add Line Indent support. #73114
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
Merged
Merged
Add Line Indent support. #73114
Changes from all commits
Commits
Show all changes
24 commits
Select commit
Hold shift + click to select a range
1ea311f
Add Line Indent support.
mtias 3c27f5a
Fix linting errors in theme.json class
aaronrobertshaw dc05a6a
Add since comments detailing addition of new support and elements
aaronrobertshaw 5879f57
Update global settings and styles docs
aaronrobertshaw d2a0f21
Tweak intial position for range control slider
aaronrobertshaw e9e13cb
Make text indent show by default as expected in global styles panel
aaronrobertshaw 8dd95c3
Remove __experimental prefix from block support key
aaronrobertshaw 5555bb9
Tweak organisation and layout of TypographyPanel
aaronrobertshaw 43fa012
Update theme.json schema
aaronrobertshaw 9cc4ad6
Add change backlog
aaronrobertshaw 7c27855
Update docs
aaronrobertshaw 052b246
Try reverting moving text element to under elements
aaronrobertshaw c133900
Trial adding paragraph element
aaronrobertshaw 691473b
Fix linting errors
aaronrobertshaw 627a579
Fix incorrect unit test data structure
aaronrobertshaw 7e206f0
Revert "Fix incorrect unit test data structure"
aaronrobertshaw ca915f8
Revert "Fix linting errors"
aaronrobertshaw 32725df
Revert "Trial adding paragraph element"
aaronrobertshaw bfcd6d6
Restrict line indent support to blocks only
aaronrobertshaw a663651
Update block.json schema and paragraph selectors
aaronrobertshaw 2ab7730
Remove redundant handling of unsupported textIndent
aaronrobertshaw 364cc33
Clean up handling of paragraph block vs text element textIndent style…
aaronrobertshaw dad26d4
Fix linting errors
aaronrobertshaw 34b7954
Improve text indent help text for block inspector
aaronrobertshaw File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| https://github.com/WordPress/wordpress-develop/pull/10573 | ||
|
|
||
| * https://github.com/WordPress/gutenberg/pull/73114 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no idea why this PR is causing the e2e fail.
The fail occurs in the
should apply custom colors and font sizes in a variationtest, and the fontSize value wasn't coming through to the control.Here's what my agent is telling me:
So, doing a deep merge in the variation seems to fix it:
I suppose because there are nested objects that need to be merged there, e.g.,
base.styles.elements.textwithvariation.styles.elements.textThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh wait, now it doesn't pass again. Sorry for the red herring. I swear it passed on this change!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay it's because I had another suggested fix applied, one that does, I hope, work:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I came to the same conclusion, i.e. the move of
textelement styles fromstyles.typographyetc tostyles.elements.text.typographywas the culprit.What I can't answer just yet was why that chance was made in this PR.
The fallback approach should work but I'm interested to know if we really need to move the styles at all and increase complexity here as a result.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah it would be nice to know why that was the choice. there must be a architectural reason.
i can't get much out of the pr description.
cc @mtias if you can provide more info
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've started exploring some of the suggested options here. Starting with a
paragraphselement. It might be easier to discuss with a working PoC.It's rough and incomplete at this stage but a draft has been added via: 70e7e07
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aaronrobertshaw if we are looking at a paragraph element, I think it may be an indication that we should just use the paragraph block instead. That may be more straightforward since "text" is meant to apply to things like lists, etc, which are not applicable here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @mtias for the extra insight.
My thinking on the paragraph element was allowing this line indent style to apply across block types. Having played with this some, I'm not convinced there's a lot of benefit to it. Any third-party blocks wanting to allow this could probably be using paragraph blocks as inner blocks anyway.
If we lean towards just using the paragraph block, this does become much simpler. When adopting this new block support a custom selector can be set via the Block Selectors API for the line indent support only.
I'll get this updated to revert the paragraph element changes and lean more on the paragraph block only.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good! We just need to handle the contiguous paragraph case elegantly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've pushed the promised updates. The paragraph block uses the
p + pselector for thetextIndentblock support feature only.So for global styles this will work almost the same as the first approach in this PR. As it stands now, defining text indent on the paragraph block will result in styles using the
:root :where(p + p)selector.