Skip to content
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

Only engage zoom out mode if the editor is focused on certain entities #61044

Closed
draganescu opened this issue Apr 24, 2024 · 4 comments · Fixed by #65732
Closed

Only engage zoom out mode if the editor is focused on certain entities #61044

draganescu opened this issue Apr 24, 2024 · 4 comments · Fixed by #65732
Labels
[Feature] Zoom Out [Type] Enhancement A suggestion for improvement.

Comments

@draganescu
Copy link
Contributor

draganescu commented Apr 24, 2024

Advances #50739

What problem does this address?

It doesn't make sense - for now - to engage zoom out mode when we're editing a template part or a single pattern. In these situations the detailed focused editing of a small amount of content makes no use of a bird's eye view.

However, currently when engaging Global Styles > Browse styles or Insert > Patterns > [any category], then zoom out mode is engage automatically.

What is your proposed solution?

We should not engage zoom out mode if the editor is focused on an entity.

@draganescu
Copy link
Contributor Author

draganescu commented Apr 24, 2024

Did some digging for this, surprisingly more "complicated" for such a seemingly simple thing:

  • 1st there is something sort of a naming conflict where we use focus mode as a setting that enables the faded out of the blocks except the selected one, but also focus mode for the flag that sets the editor into the focused entity editing UI. This is confusing.

Not sure if/what to do about that.

  • The focus on an entity depends on the kind of entity being edited, that means the "knowledge" of this focused editing is at the level of the editor.
  • The block editor does not know about what the final implementation displays.
  • The zoom out mode is a feature of the block editor.

These point to the idea that there should be some setting that higher level editors should send to the block editor to disable zoom out from engaging, when it does so automatically.

However, I thought that yet another setting for a small impact feature, maybe there is another way, like not allowing the user to engage the mode at all. Alas, this can't happen because:

  • The activation of the zoom out mode happens automatically via a useZoomOut hook used both inside the block editor package in the Inserter component, but also in the edit-site package in the ScreenStyleVariations component.

So we can't pass a new param based on the current edited entity to the hook because we call the hook once in the block editor and once in the site editor (for now, likely it will exist in other places).

Options:

  1. In a previous implementation this was handled by passing some new tab change handler prop to the Inserter component. We could do the same, and create some handlers on the Inserter. I think this use case is not enough to add handlers for tab changes? This would also not cover the global styles case, that would need it's own focused entity check.
  2. Add a disableZoomOutMode setting to the block editor and set it to false if the editor is in focused entity editing mode, then check this setting in the useZoomOutMode hook. This would cover both cases and future cases. But yet another setting.
  3. Refactor how we use zoom out and always engage it from the higher level implementation (not from the block editor rendering one of its components, this seems like a bad idea).

Also, I may have missed something 😊 obvious and this is really very simple.

@ajlende
Copy link
Contributor

ajlende commented Apr 25, 2024

1st there is something sort of a naming conflict where we use focus mode as a setting that enables the faded out of the blocks except the selected one

I've seen this referred to as "spotlight mode" in the block editor docs. So maybe it's worthwhile updating it in the code?

but also focus mode for the flag that sets the editor into the focused entity editing UI.

This is what's referred to as "focus mode" in the site editor docs, and what I think of when I hear "focus mode".

  • The focus on an entity depends on the kind of entity being edited, that means the "knowledge" of this focused editing is at the level of the editor.

  • The block editor does not know about what the final implementation displays.

  • The zoom out mode is a feature of the block editor.

These point to the idea that there should be some setting that higher level editors should send to the block editor to disable zoom out from engaging, when it does so automatically.

Right, the editor is the layer that is aware of WordPress specific entities, and block-editor should be able to handle all the generic non-WordPress stuff to my understanding. And the edit-post and edit-site packages are in charge of unique behaviors between them, layout of the editor components, and some additional editor chrome.

However, I thought that yet another setting for a small impact feature, maybe there is another way, like not allowing the user to engage the mode at all. Alas, this can't happen because:

  • The activation of the zoom out mode happens automatically via a useZoomOut hook used both inside the block editor package in the Inserter component, but also in the edit-site package in the ScreenStyleVariations component.

In general, I don't like the idea of setting state as a side-effect of component mounting/unmounting like we have. It should come directly from some user-generated action so that the behavior is easy to trace. If that user generated action needs to be aware of WordPress-specific things, it should come from the editor level or above.

  1. Refactor how we use zoom out and always engage it from the higher level implementation (not from the block editor rendering one of its components, this seems like a bad idea).

I'm curious why you think this is a bad idea? It seems ,to me, to be the most reasonable way to handle things given the requirements of enabling the mode and the division of responsibilities for our packages. The __unstableEditorMode is already part of the store, so we can dispatch actions to that store from the parent packages as a way of programmatically interacting with the block editor. No new APIs are needed.

I will say that the lines between edit-post, edit-site, editor, and block-editor are a little blurry as far as what is currently implemented in them. So maybe my mental model of things isn't quite right. 😅

@draganescu
Copy link
Contributor Author

I'm curious why you think this is a bad idea?

I meant the exact opposite, that performing mode changes "from the block editor rendering one of its components" is a bad idea, and that refactoring out of that hook is a good idea 😁

@draganescu draganescu changed the title Only engage zoom out mode if the editor is focused on an entity Only engage zoom out mode if the editor is focused on certain entities Sep 30, 2024
@draganescu
Copy link
Contributor Author

Following the de-coupling of zoom out mode from the scaling of the canvas, the styles panel do not engage the mode anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Zoom Out [Type] Enhancement A suggestion for improvement.
Projects
Development

Successfully merging a pull request may close this issue.

2 participants