-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
[Layout]: Fix align controls for hybrid themes #47961
Conversation
Size Change: +84 B (0%) Total Size: 1.33 MB
ℹ️ View Unchanged
|
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 can confirm this fixes regression for hybrid themes. Tested Group, Cover, and Image blocks ✅
- I don't see any regressions for block themes ✅
- The code is straightforward and looks good to me 👍
Flaky tests detected in 609564e. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4143098635
|
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 for addressing this!
I think the isBlockBasedTheme
check makes sense as long as the definition of hybrid theme is a theme with PHP templates and a theme.json
file, but we should be aware that this will fail if the theme includes an index.html
file.
It should be ok for this change to go in though, considering that __unstableIsBlockBasedTheme
is already in core and this will fix the issue for many (perhaps most?) hybrid themes.
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.
+1 thanks for the straightforward fix!
✅ Confirmed issue on trunk that a classic theme with a theme.json
file and opted in to add_theme_support( 'align-wide' );
results in no wide / full width controls being available
✅ With this PR applied, the wide / full width controls are restored
✅ In a block-based theme, children of flow
layout still correctly do not show wide/full alignments as those children are already full width to the container
LGTM! ✨
I just cherry-picked this PR to the wp/6.2 branch to get it included in the next release: 1ef68f9 |
What?
Fixes: #47857
From the issue:
Step-by-step reproduction instructions
Tested with WordPress 6.2 beta1 nightly without Gutenberg plugin.