-
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
Migrate standalone widgets editor to use new preferences package #39084
Conversation
@@ -155,8 +155,7 @@ $z-layers: ( | |||
".components-popover.customize-widgets-more-menu__content": 99998, | |||
".components-popover.edit-post-more-menu__content": 99998, | |||
".components-popover.edit-site-more-menu__content": 99998, | |||
".components-popover.edit-widgets-more-menu__content": 99998, | |||
".components-popover.preferences-more-menu__content": 99998, |
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.
Seems like I accidentally added this in #38873. The more menu is staying in the interface package, so the 'preferences' part here is wrong.
It shouldn't have caused any bugs as nothing should be using this name. As I migrate each editor, the individual z-indices for each editor can be removed in favor of just the interface
one.
@@ -31,5 +31,5 @@ | |||
} | |||
|
|||
.components-popover.interface-more-menu-dropdown__content { | |||
z-index: z-index(".components-popover.edit-widgets-more-menu__content"); | |||
z-index: z-index(".components-popover.interface-more-menu__content"); |
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.
This was wrong for some reason, so I'm changing it here to match the change in the _z-index.scss file.
Size Change: +27 B (0%) Total Size: 1.15 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.
Excellent stuff. Tested works as described and the code is very straightforward.
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.
Can we now update the widgets store to use register
instead of registerStore
?
We can, though it looks like it didn't need to use |
c502142
to
167c6b7
Compare
When I Anyway, testing this and looking at the code now 😀 |
Yeah, they would fail, I already had to wrestle them once or twice. What changes are you seeing? |
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.
Code looks good and this tests well! I like the unit tests included—convinces me that the migration code is good.
|
||
// Once we build a more generic persistence plugin that works across types of stores | ||
// we'd be able to replace this with a register call. | ||
registerStore( STORE_NAME, storeConfig ); |
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.
Just for my own clarity: Is there something in this PR that lets us remove this that I'm not understanding? Or is it a "while we're here" 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.
It's a 'while we're here' change. It should have been done previously when the edit-widgets store stopped using the persistence plugin (in #33774).
Thanks for reviewing.
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.
Ahhh gotcha.
I did yet another |
Next one up is the customize widget editor, which is addressed in #39112. |
Description
Addresses some of #31965, follows #38873
Migrates the standalone widgets editor to use the new preferences package (introduced in #38873). Previously it was using
interface
, but the plan is now to deprecate the preferences APIs in interface in favor of this new package.The main goal here is that nothing is different from a user perspective.
Testing Instructions
trunk
open the widgets editor and dismiss the welcome guideScreenshots
Types of changes
What types of changes does your code introduce?
Checklist:
*.native.js
files for terms that need renaming or removal).