-
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
Include cascading properties in Navigation deprecation #36432
Conversation
Size Change: -15 B (0%) Total Size: 1.09 MB
ℹ️ View Unchanged
|
Code looks good to me, but I would think it might need a code sanity check beyond what I can offer. Thanks for working on this. |
3d3d4e2
to
68912b0
Compare
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.
It works as expected, and the code looks good to me. Thanks, @tellthemachines.
* Include cascading properties in nav deprecation * Update fixtures
@@ -15,7 +15,7 @@ | |||
}, | |||
"layout": { | |||
"type": "flex", | |||
"justifyContent": "left", | |||
"setCascadingProperties": "true", |
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.
What does this property on a "layout" object mean? Sounds very weird to me?
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 see it's coming from #36292 I'll check there.
Description
The Navigation block migration that handles the change to layout isn't adding
setCascadingProperties:"true"
to the attributes, so migrated navigations are not getting the correct justification properties set.This PR adds that attribute and cleans up the code a little.
How has this been tested?
I think the easiest way to test this is switching to code view in the block editor, pasting in some deprecated nav markup such as
<!-- wp:navigation {"itemsJustification":"right","isResponsive":true} -->
and saving. The saved content should show the updated attributes, includingsetCascadingProperties
.Moving out of code view and adding a few items to the nav, the layout should reflect the justification/orientation attributes set.
Screenshots
Types of changes
Bug fix (non-breaking change which fixes an issue)
Checklist:
*.native.js
files for terms that need renaming or removal).