Transform Styles: Update selector so that styles work when custom fields panel is active #62121
Conversation
This file contains 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
andrewserong
added
[Type] Bug
|
Size Change: +12 B (0%) Total Size: 1.74 MB
ℹ️ View Unchanged
|
|
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: @gyurmey2. 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. |
andrewserong
commented
May 30, 2024
| @@ -54,6 +54,6 @@ test.describe( 'Styles', () => { | |||
| await paddingControl.fill( '2' ); | |||
|
|
|||
| // Check the padding value | |||
| await expect( block ).toHaveCSS( 'padding-left', '35.4644px' ); | |||
| await expect( block ).toHaveCSS( 'padding-left', '35.4932px' ); | |||
There was a problem hiding this comment.
For some reason this line was failing:
So I've updated this to the new expected value to see if it gets the tests passing. Separately, I wonder if this check should be simplified to ignore the fraction, as this effectively averages out to around 36px anyway 🤔
There was a problem hiding this comment.
Oh that's going to be a fun test to maintain 🤦
There was a problem hiding this comment.
I think it would be better to use a custom value - e.g. set it to 5px, and then the expected padding-left should be 5px. I can look at updating this as part of the tests I'm working on.
There was a problem hiding this comment.
Looks like it was just updated to a hard-coded value in #62111 so I've reverted the change here.
There was a problem hiding this comment.
Thanks for the PR! I'm thinking we might need to check for both :root :where(body) and :where(body), because there's at least one instance of the latter (the rule that sets the margin to 0) that will otherwise be ignored.
aaronrobertshaw
approved these changes
May 30, 2024
There was a problem hiding this comment.
Thanks for fixing this up @andrewserong 🚀
After a false start due to a local theme issue, this is testing well for me.
LGTM 🚢
|
Thanks for the reviews, folks! @tellthemachines I've updated the rule to chain the existing rule and the new one (with the new one running first). Does that look okay to do? The output in the post editor seems to look okay to me now: 
|
|
The tweak for the margin rule works for me 👍 
|
tellthemachines
approved these changes
May 30, 2024
andrewserong
force-pushed
the
fix/missing-styles-when-custom-fields-panel-is-active
branch
from
dd9e141
to
e7302e4
Compare
andrewserong
deleted the
fix/missing-styles-when-custom-fields-panel-is-active
branch
|
Thank you, @andrewserong! |
carstingaxion
pushed a commit
to carstingaxion/gutenberg
that referenced
this pull request
Jun 4, 2024
…lds panel is active (WordPress#62121) * Transform Styles: Update selector so that styles work when custom fields panel is active * Try to fix snapshot * Chain new and old selectors to catch margin rule Unlinked contributors: gyurmey2. Co-authored-by: andrewserong <andrewserong@git.wordpress.org> Co-authored-by: tellthemachines <isabel_brison@git.wordpress.org> Co-authored-by: aaronrobertshaw <aaronrobertshaw@git.wordpress.org> Co-authored-by: talldan <talldanwp@git.wordpress.org> Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>
patil-vipul
pushed a commit
to patil-vipul/gutenberg
that referenced
this pull request
Jun 17, 2024
…lds panel is active (WordPress#62121) * Transform Styles: Update selector so that styles work when custom fields panel is active * Try to fix snapshot * Chain new and old selectors to catch margin rule Unlinked contributors: gyurmey2. Co-authored-by: andrewserong <andrewserong@git.wordpress.org> Co-authored-by: tellthemachines <isabel_brison@git.wordpress.org> Co-authored-by: aaronrobertshaw <aaronrobertshaw@git.wordpress.org> Co-authored-by: talldan <talldanwp@git.wordpress.org> Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What?
Fixes #62091
This re-implements the fix from #61602.
Update the selector in
transformStyleto match the new selector for global styles rules introduced in #61638 (the rules are now prefixed with:root).Why?
When the styles were prefixed with the addition of
:root, the transform intransformStylesthat allows the styles to work in the non-iframed editor no longer matched against the global styles rules. This PR should restore the behaviour so that the editor looks correct in both iframed and non-iframed modes.How?
:rootto the selector of thetransformStylelogic that matches against rules intended for the body elementTesting Instructions
trunknote that the global styles rules are not applied in the editor. With this PR applied the styles should be restored.Screenshots or screencast