Migrate standalone widgets editor to use new preferences package#39084
Merged
Conversation
talldan
added
[Type] Enhancement
talldan
requested review from
adamziel,
ajitbohra,
draganescu,
kevin940726,
nerrad,
noisysocks,
ntwb and
tellthemachines
as code owners
This was referenced Feb 25, 2022
talldan
commented
Feb 25, 2022
| ".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, |
Contributor
Author
There was a problem hiding this comment.
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.
talldan
commented
Feb 25, 2022
|
|
||
| .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"); |
Contributor
Author
There was a problem hiding this comment.
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
|
draganescu
approved these changes
Feb 25, 2022
Contributor
draganescu
left a comment
draganescu
left a comment
There was a problem hiding this comment.
Excellent stuff. Tested works as described and the code is very straightforward.
youknowriad
reviewed
Feb 26, 2022
Contributor
youknowriad
left a comment
youknowriad
left a comment
There was a problem hiding this comment.
Can we now update the widgets store to use register instead of registerStore ?
Contributor
Author
We can, though it looks like it didn't need to use |
talldan
force-pushed
the
migrate/edit-widgets-to-use-new-preferences
branch
from
c502142 to
167c6b7
Compare
Member
|
When I Anyway, testing this and looking at the code now 😀 |
Contributor
Author
Yeah, they would fail, I already had to wrestle them once or twice. What changes are you seeing? |
noisysocks
approved these changes
Feb 28, 2022
Member
noisysocks
left a comment
noisysocks
left a comment
There was a problem hiding this comment.
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 ); |
Member
There was a problem hiding this comment.
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?
Contributor
Author
There was a problem hiding this comment.
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.
Member
|
I did yet another |
8 tasks
Contributor
Author
|
Next one up is the customize widget editor, which is addressed in #39112. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
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
trunkopen the widgets editor and dismiss the welcome guideScreenshots
Types of changes
What types of changes does your code introduce?
Checklist:
*.native.jsfiles for terms that need renaming or removal).