Add text align controls to breadcrumbs#76747
Add text align controls to breadcrumbs#76747n2erjo00 wants to merge 1 commit intoWordPress:trunkfrom
Conversation
|
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 Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @jonashellwig. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. 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. |
t-hamano
added
[Type] Enhancement
t-hamano
left a comment
t-hamano
left a comment
There was a problem hiding this comment.
Thanks for the PR!
The textAlign block support is intended to apply the text-align CSS property, so using justify-content instead seems a bit odd to me.
I'm curious why this block uses the flex layout in the first place.
In my testing, just using the default layout works well for me.
diff --git a/packages/block-library/src/breadcrumbs/block.json b/packages/block-library/src/breadcrumbs/block.json
index 597e36c41ae..acbc77e19a9 100644
--- a/packages/block-library/src/breadcrumbs/block.json
+++ b/packages/block-library/src/breadcrumbs/block.json
@@ -68,7 +68,8 @@
"__experimentalLetterSpacing": true,
"__experimentalDefaultControls": {
"fontSize": true
- }
+ },
+ "textAlign": true
},
"interactivity": {
"clientNavigation": true
diff --git a/packages/block-library/src/breadcrumbs/style.scss b/packages/block-library/src/breadcrumbs/style.scss
index 7111f1130bb..081e64e3db5 100644
--- a/packages/block-library/src/breadcrumbs/style.scss
+++ b/packages/block-library/src/breadcrumbs/style.scss
@@ -6,15 +6,12 @@
list-style: none;
margin: 0;
padding: 0;
- display: flex;
- flex-wrap: wrap;
- align-items: center;
}
li {
margin: 0;
padding: 0;
- display: flex;
+ display: inline-flex;
align-items: center;
&:not(:last-child)::after {cc @ntsekouras
|
Putting some my own thoughts here. From technical standpoint it is not text alignment so from that point of view it is maybe bit "hacky" but from user standpoint it is setting text alignment so based on this I decided to use textAlign control (familiar icon and familiar workflow for users) @t-hamano But lets see what @ntsekouras has on his mind and then lets hussle 😸 |
|
Thanks for the PR! I commented on the issue and I think it should be closed. That said, |
3 participants
What?
Closes #76739
This PR add text align controls breadcrumbs block.
Why?
Breadcrumbs benefit from text align support
How?
Added
typography.textAlignsupport block and wrote couple lines of CSS to create the support.Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast
Use of AI Tools