-
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
Migrate site editor feature preferences to interface package #34191
Migrate site editor feature preferences to interface package #34191
Conversation
Size Change: +250 B (0%) Total Size: 1.04 MB
ℹ️ View Unchanged
|
4449137
to
aa4b281
Compare
edit: now ready for review. |
aa4b281
to
31ecd50
Compare
79b95c1
to
bd42d9a
Compare
@youknowriad should have a good overview of how it should work to pass down all the preferences to the block editor. If it's something different then feel free to ignore my comment 😅 |
Feels like "interface" is becoming a package that contain everything that need to be shared across screens. Kind of like a "utils" package. The questions I have is:
|
Yeah, it's difficult to know where to draw the line on that. I did consider a separate package, but interface seems like a good fit as it already keeps some similar data in its store for controlling active/visible parts of the editor. There are a few grey areas as part of the overall work, I'm wondering where things like the block manager could live - #31965 (comment).
At the moment each screen has individual preferences. With this change preferences could be shared across editors like proposed in #24370, but I think there needs to be more progress on that issue to understand exactly what approach should be taken. |
As long as the preferences are "interface preferences" (top toolbar, spotlight), and as long as we want to share these preferences between editors for a unified block editor interface experience, moving them to the interface package is good. All the block editors will need though the possibility to extend the preferences inherited from the interface package with their own special preferences, and also to disable default interface preferences they don't use. |
Even if the decision is to make a 'preferences' package, it still might be worth going ahead with this. The site editor is the only editor that hasn't had this migration to interface, so going ahead will at least mean there isn't an odd one out situation. A future migration would be easier. This PR also deprecates the site editor's preferences APIs, which would be good to do earlier so they can be removed. |
It all depends on how the UI evolves and what user experience we want to have. We definitely want to have a unified way to store all preferences. The question is where it should be bundled is hard to answer because we still didn't fully define what role One question we should answer soon is whether preferences should be saved per screen (post, site, widgets, ...) as it happens today, or should we also let to store some preferences globally like for the top toolbar or full screen mode. |
Superceded by #39158 |
Description
Partly addresses #31965. Related to #31965, #34135 and #34154.
Migrates site editor feature preferences over to the interface package.
As they've never hit core, I've deprecated the
toggleFeature
action andisFeatureActive
selector and will remove them from a future version.The way editor settings work in the site editor is slightly different to other editors, I guess it has been augmented for the theme.json.
Typically in other editors the settings are merged with the feature preferences using
useMemo
in the 'Editor' component before being passed into theEditorProvider
, but the site editor uses a memoized selector to combine the values. This wouldn't work with this refactor, because the values are now in a different store.The options:
useMemo
in the Editor component. This is what I've done in this PR, but it's open to feedback. The downside is that thefocusMode
andfixedToolbar
properties are no longer available when using the selector.useEditorSettings
hook that combines settings with preferences. This might be the way to go. There's some prior art in the site editor with auseSetting
hook -gutenberg/packages/edit-site/src/components/editor/utils.js
Lines 101 to 112 in 914f543
How has this been tested?
Types of changes
New feature (non-breaking change which adds functionality)
Checklist:
*.native.js
files for terms that need renaming or removal).