Codify editor focus mode concept #56827
Closed
Conversation
This file contains 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
|
Pinging @youknowriad here as he's been working to refactor a lot of the editor recently. Riad - I couldn't find this concept codified elsewhere but you might know of something I don't. |
|
Size Change: +37 B (0%) Total Size: 1.72 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 5ca28eb. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7116100753
|
|
It's not codified anywhere but we do have a "renderingMode" at the moment where you could navigate between a "record"/"post"/"page" and its template. The "record" can be of any post type. (including navigation) I'd like to understand more what would this "focus mode" be about. If it corresponds to a distinct URL in the site editor, It's not a "focus mode" from the "editor"'s perspective and just a concept that is external to the editor (just a path basically) What would make sense as a "focus mode" from the editor's perspective would be the possibility to focus parts of the post/record (say a block clientId for example). So it's more of a "focus block" maybe. So I guess at this point, it's probably better to keep it as an "edit-site" specific thing but also I'd like to understand what it means for the "editor". What does it impact? I'm assuming it's going to change the edited entity shown by the editor, is it going to change something else? |
This is slightly more nuanced than it might appear, for example a template is also a post instance in that it is saved in the posts table just like template parts and navigation menus. |
scruffian
approved these changes
Dec 8, 2023
draganescu
reviewed
Dec 8, 2023
| @@ -19,3 +26,8 @@ export function getCanvasMode( state ) { | |||
| export function getEditorCanvasContainerView( state ) { | |||
| return state.editorCanvasContainerView; | |||
| } | |||
|
|
|||
| export function isEntityFocusMode( state ) { | |||
There was a problem hiding this comment.
isEntityFocusMode sounds a big confusing to me. Why not name it like what it does, checking focusable entities, so more like isEntityFocusable?
draganescu
reviewed
Dec 8, 2023
There was a problem hiding this comment.
After writing this comment I realise that this is not quite OK because:
- we are determining the mode of the editor from the name of the edited entity
It doesn't seem a good connection to make. The editor should know what mode it's in and we should ask it. The UX of entering focus mode sets this for entity X or Y but they are not connected by anything else except UX.
My worry is: that selector is private, some time will pass, it will graduate and in the future we'll want to edit these entities in normal mode and a back button will render because this selector makes it show based on entity name.
I think we need to codify the notion, and do the improvement you did but not rely on entity name IMO.
|
@draganescu Thanks for review. To confirm, are you saying you approve of codifying the concept of "focus mode"?
Can you expand on this a bit for me? |
Not much to expand: we should know the editor is in focus mode from a prop, context, the query string, something else, except the name of the entity which can be constrained or not to focused editing. |
|
In terms of the synced pattern overrides, we are using focused mode to refer to the editing of the source pattern in the post and site editor in isolation from the post content. This is on order to give the user a clear indication that in the post editor when the edit the pattern overrides they are only affecting that post, but when they switch to editing the source pattern in isolation (focused) they are making changes globally that will affect every instance of the pattern. While referring to it as focus mode we are currently just using the existing |
I don't think we should be doing that. It's becoming clear that we need a history of edited entities outside the EditorProvider. In other words, I think we should remove the existing "template-mode" in favor of just passing the template as the "post" prop of the EditorProvider. a "pattern-only-mode" just means we keep introducing a "entity-only-mode" for each entity (navigation, template part... and it's clearly a limited approach) we want to just update the edited entity. |
Thanks for the feedback @youknowriad. The pattern-edit-original.mp4although the padding around the edge has now been removed from the template edit Design are keen to keep this for source pattern editing. So as well as passing the pattern entity id into the editor provider we also need a way to indicate the padding change and to add the document bar in the header - which we can do by checking the entity type maybe instead of the renderMethod or focus mode suggested here. I will do some more experimenting, but let me know if you have any suggested approaches. |
|
I haven't had time to iterate on this and things have moved on. Closing out. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What?
Codifies the concept of "focus mode" by implementing a selector which you can call to determine whether the editor is in focus mode.
"Focus mode" means the different layout and visual state the editor moves to when editing a particular post instance, for example:
Note: the name "focus mode" feels a little vague. There are far too many "modes" floating around. I'm open to better suggestions.
Why?
Templates are becoming littered with inline calls to check for this concept. The odd call was acceptable but the proliferation of this concept means it's now time to codify it.
How?
Implements an experimental selector to determine whether the editor is in focus mode.
There's no need for new state because focus mode is determine purely by whether we are editing an individual post type.
Testing Instructions
trunk.Testing Instructions for Keyboard
Screenshots or screencast