-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Unify the preferences modal UI between post and site editor #57639
Conversation
98cf2da
to
4df2961
Compare
This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress. If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged. If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack. Thank you! ❤️ View changed files❔ lib/client-assets.php |
697aa18
to
368b459
Compare
export { default as PreferencesModal } from './preferences-modal'; | ||
export { default as PreferencesModalTabs } from './preferences-modal-tabs'; | ||
export { default as PreferencesModalSection } from './preferences-modal-section'; | ||
export { default as ___unstablePreferencesModalBaseOption } from './preferences-modal-base-option'; |
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'm guessing these components were only here because we wanted them private. I've moved them to the preferences package as private APIs instead.
368b459
to
388fa1f
Compare
Size Change: -3.18 kB (0%) Total Size: 1.69 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.
Do we also need to update legacy storage data migration?
gutenberg/packages/preferences-persistence/src/migrations/legacy-local-storage-data/index.js
Lines 58 to 62 in 1451d8a
data = moveIndividualPreference( | |
data, | |
{ from: 'core/edit-post', to: 'core/edit-post' }, | |
'editorMode' | |
); |
gutenberg/packages/preferences-persistence/src/migrations/legacy-local-storage-data/index.js
Lines 79 to 83 in 1451d8a
data = moveIndividualPreference( | |
data, | |
{ from: 'core/edit-site', to: 'core/edit-site' }, | |
'editorMode' | |
); |
Edited: Sorry, this comment was a mistake, so please ignore it.
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'd appreciate some more testing here, but this LGTM, thanks!
|
||
// Keep a reference of the `allowedBlockTypes` from the server to handle use cases | ||
// where we need to differentiate if a block is disabled by the user or some plugin. | ||
defaultAllowedBlockTypes: settings.allowedBlockTypes, |
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 nice we don't need this anymore.
The PR's pretty big, so I've gone through and manually tested preferences between post and site editors. ✅ All preferences have effect and persist between editors 🎉 |
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.
The code looks good to me and ran great in my manual testing. Since this overhauls quite a lot of lines 🤞🏻 for automated tests to cover what we miss reading.
One thing that the PR description was not clear for me is that this PR makes the prefference settings apply universally across editors. Maybe this is worth a 👍🏻 from @WordPress/gutenberg-design ?
This was actually done in previous PRs iteratively, this PR is the "final" step making the UI common. |
I don't mind that chagne. I see arguments for both cases, but especially with some of the unification done for page editing, I do eventually see these growing towards something we think of more as a single editor, rather than two. That said, things like top toolbar and similarly are intentionally put as top items in the options menu, to make it easy to flow in and out of those. |
83f5009
to
ed02581
Compare
ed02581
to
0bfca08
Compare
I just ran into this problem tonight and came here to make an issue. Now I don't need to. Y'all rock! Thanks for your work in resolving this. |
@youknowriad the only change to PHP files here is in gutenberg/lib/client-assets.php Lines 3 to 4 in 1ea6c22
If you can confirm I will update the docs and the PHP Sync Tracking Issue. |
@getdave that's not entirely true. client-assets.php do in general require changes to WP Core. In this particular instance we need to define a new preferences styles handle in the script loader (which I'll be doing as part of the package update PR) |
I was in two minds about it and then I read
...in the file comment. Maybe we should reword to
|
Related #52632
What?
This PR continues the work on the great unification between post and site editors. This PR brings the "blocks manager" to the site editor as well, allowing hiding/showing blocks in the site editor as well similar to the post editor.
In addition to that, this PR unifies the "preferences modal" UI and just uses the same component for both. There are still some extra preferences that are specific to the post editor but these are injected using a special
extraSections
prop.Testing instructions