Migrate site editor feature preferences to interface package#34191
Closed
talldan wants to merge 6 commits into
Closed
Migrate site editor feature preferences to interface package#34191talldan wants to merge 6 commits into
talldan wants to merge 6 commits into
Conversation
talldan
added
[Type] Enhancement
18 tasks
talldan
changed the title
|
Size Change: +250 B (0%) Total Size: 1.04 MB
ℹ️ View Unchanged
|
talldan
force-pushed
the
migrate/site-editor-feature-preferences-to-interface
branch
from
4449137 to
aa4b281
Compare
Contributor
Author
|
edit: now ready for review. |
talldan
force-pushed
the
migrate/site-editor-feature-preferences-to-interface
branch
from
aa4b281 to
31ecd50
Compare
talldan
force-pushed
the
migrate/site-editor-feature-preferences-to-interface
branch
from
79b95c1 to
bd42d9a
Compare
7 tasks
Member
@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 😅 |
Contributor
|
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:
|
Contributor
Author
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. |
Contributor
|
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. |
Contributor
Author
|
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. |
Member
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. |
8 tasks
Contributor
Author
|
Superceded by #39158 |
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
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
toggleFeatureaction andisFeatureActiveselector 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
useMemoin 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:
useMemoin the Editor component. This is what I've done in this PR, but it's open to feedback. The downside is that thefocusModeandfixedToolbarproperties are no longer available when using the selector.useEditorSettingshook that combines settings with preferences. This might be the way to go. There's some prior art in the site editor with auseSettinghook -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.jsfiles for terms that need renaming or removal).