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

[WP 6.7] Zoom Out toggle appears/disappears when Patterns inserter activates Zoom Out for non-iframed editor #66884

Open
annezazu opened this issue Nov 8, 2024 · 19 comments
Labels
[Feature] Zoom Out [Type] Bug An existing feature does not function as intended

Comments

@annezazu
Copy link
Contributor

annezazu commented Nov 8, 2024

From further discovery this is the true cause of the Issue

If a block with apiVersion below 2 is registered, the editor is not an iframe.
If the editor is not an iframe, the zoom out toggle button will not be displayed.
However, when you click the Patterns tab, the editor works as an iframe, so the zoom out toggle button appears.


When too many plugins are registering items in the toolbar, the zoom toggle disappears in 6.7-RC4:

missing.zoom.toggle.mov

Steps to replicate:

  1. Install and activate Jetpack and Yoast.
  2. Open up Pages > Add new.
  3. Notice you don't see the zoom out toggle.
  4. Notice that if you open up the Inserter > Patterns, that it appears.

I'd expect that the zoom out toggle would still be visible and, no matter how wide I try to make the viewport, it never shows up. cc @getdave I couldn't find a comparable issue on the 6.7 board for zoom out.

Props to @laurelfulford for first finding this!

@annezazu annezazu added [Feature] Zoom Out [Type] Bug An existing feature does not function as intended labels Nov 8, 2024
@annezazu annezazu moved this to 🗣️ In Discussion / Needs Decision in WordPress 6.7 Editor Tasks Nov 8, 2024
@annezazu
Copy link
Contributor Author

annezazu commented Nov 8, 2024

cc @fabiankaegy as a heads up.

@laurelfulford
Copy link
Contributor

We ran into the above when testing different Newspack plugins with WordPress 6.7. I've run into a wall trying to figure out why it's happening, but here's a few more details:

We're seeing this with plugins like Jetpack, Newspack Blocks, Newspack Campaigns, Newspack Newsletters, Events Calendar, and some others we built. If just one of these plugins are enabled on their own (in Jetpack's case, enabled and connected), the Zoom out toggle goes away.

All of these plugins register blocks; when I removed the block registration code from a couple, the issue goes away.

We also use WooCommerce, Co-Authors Plus, and Yoast, which also register blocks but don't cause this issue.

One difference is that the latter group all register and enqueue block assets using block.json. Best I can tell the ones that cause issues with the Zoom out don't -- some of the blocks have a block.json, but don't seem to use it to register the block.

When the Zoom out isn't showing, opening the Pattern inserter will zoom the page out and make the toggle appear, but it disappears again once the Pattern inserter is closed.

Hopefully this isn't a red herring! Just let me know if you have any questions about the above.

@annezazu
Copy link
Contributor Author

annezazu commented Nov 9, 2024

Thank you so much for all of the extra details. Added props above for your hard work in trying to track this down and first reporting it :)

@t-hamano
Copy link
Contributor

t-hamano commented Nov 9, 2024

This issue is the same as #66671.

The cause of this issue is as follows:

  • If a block with apiVersion below 2 is registered, the editor is not an iframe.
  • If the editor is not an iframe, the zoom out toggle button will not be displayed.
  • However, when you click the Patterns tab, the editor works as an iframe, so the zoom out toggle button appears.

This issue has been fixed in Gutenberg trunk (#66789), but we need to consider how to address this issue in WP 6.7.

My suggested approach is to either:

  1. Do not enable zoom out mode in the Patterns tab
  2. Enable the zoom out toggle too, which means applying Zoom out: Pattern inserter always forces iframe editor #66671 to WP 6.7 too.

The latter approach is probably the best, but this would also be considered an enhancement, so I'm not sure if it's acceptable for a point release.

cc @stokesman @draganescu

@laurelfulford
Copy link
Contributor

Thanks for tying that together, @t-hamano! 🙌 I focused too much on open issues when trying to search for something similar.

@getdave
Copy link
Contributor

getdave commented Nov 12, 2024

Do not enable zoom out mode in the Patterns tab

As that's a Core piece of the UX for Zoom Out interactions (it's designed for composing with Patterns) so I can't see that being viable.

Enable the zoom out toggle too, which means applying #66671 to WP 6.7 too.

I think you're suggesting applying #66789 to WP 6.7?

This is technically a bugfix but with the final release being today, in order for this to land it would need to be deemed either:

  • a severe bug
  • something that locks us into a particular API in a way that could cause backwards compatibility issues

This Issue is clearly far from ideal, but does it critically compromise the Editor? We also need to weigh this against any potential instability that could be caused by unknown side effects of enabling Zoom Out in the non-iframed editor.

Update: The 24hour code freeze for the release is in place so we cannot include this in 6.7 final release and it will necessarily need to be punted to 6.7.1.

@annezazu @ndiego What is your feeling on the scope of this?

I know also that @ellatrix raised concerns about Zoom Out in non-iframe mode.

@getdave getdave moved this from 🗣️ In Discussion / Needs Decision to 🐛 Punted to 6.7.1 in WordPress 6.7 Editor Tasks Nov 12, 2024
@ndiego
Copy link
Member

ndiego commented Nov 12, 2024

Yes, I agree this will need to be punted to 6.7.1.

@annezazu
Copy link
Contributor Author

Agreed to punt to 6.7.1 and I'd argue that it should be included as it's less of an enhancement and more of a bug in that you can't toggle this option on/off.

@t-hamano
Copy link
Contributor

I think you're suggesting applying #66789 to WP 6.7?

Yes, I was referring to that PR.

@cbravobernal
Copy link
Contributor

I'm moving this to 6.7.2 as is not a critical bug.

@cbravobernal cbravobernal moved this from 🔎 Needs Review to 🐛 Punted to 6.7.2 in WordPress 6.7.x Editor Tasks Nov 19, 2024
@MaggieCabrera
Copy link
Contributor

Should we avoid going into zoom out mode on the first place when we are in this situation?

@getdave getdave changed the title Zoom out: toggle missing in toolbar when too many plugins are registered in toolbar Zoom Out toggle appears/disappears when Patterns inserter activates Zoom Out for non-iframed editor Dec 3, 2024
@t-hamano
Copy link
Contributor

t-hamano commented Dec 4, 2024

My suggestions for WordPress 6.7.2 are to either:

  • Don't enable zoom out mode when you go to the Inserter > Patterns tab. That means if the editor is not an iframe, the zoom out mode is not available in any way in the post editor.
  • Expose the zoom out toggle button to non-iframe editors too. That means backporting #66789 to WP 6.7.2. A manual backport might be needed.

@getdave
Copy link
Contributor

getdave commented Dec 4, 2024

Expose the zoom out toggle button to non-iframe editors too. That means backporting #66789 to WP 6.7.2. A manual backport might be needed.

Do you know if there are downsides to this? Otherwise it would seem the best route.

@t-hamano t-hamano changed the title Zoom Out toggle appears/disappears when Patterns inserter activates Zoom Out for non-iframed editor [WP 6.7] Zoom Out toggle appears/disappears when Patterns inserter activates Zoom Out for non-iframed editor Dec 7, 2024
@t-hamano
Copy link
Contributor

t-hamano commented Dec 7, 2024

@getdave I found that zoom out mode doesn't work in the post editor in Gutenberg trunk.

#67232 seems to be related and is caused by hasSectionRootClientId being false. Is this the expected behavior and will the zoom out mode remain disabled in the post editor going forward?

If so, I'm wondering what the best approach is for the post editor in WP 6.7.2.

@getdave
Copy link
Contributor

getdave commented Dec 9, 2024

@t-hamano Yes it's my understanding that is expected.

That said, we might be able to adjust so that we can restore Zoom Out in the Post Editor but we need to avoid a situation whereby that would falsely enable Zoom Out in the Site Editor when the main or post-content (depending on Editor mode) do not exist.

@draganescu
Copy link
Contributor

Just adding some reasoning for why this is expected: if we can't find a good section root the zoom out experience is highly likely to be broken. So until there are better options we should only enable the zoomed out UX where it's a fact that it will work as expected.

@t-hamano
Copy link
Contributor

I tried applying #66789 on the wp/6.7 branch: https://github.com/WordPress/gutenberg/compare/wp/6.7...zoom-out-non-iframed-6.7?expand=1

Technically, the zoom out toggle works in the non-iframe editor, but it's not visually smooth:

94de28d87e0e35fcc374303d0500cb8c.mp4

Of course, this is a known issue that occurs when you access the Patterns tab. Is this acceptable in WP 6.7?

@getdave
Copy link
Contributor

getdave commented Dec 11, 2024

Thank you for testing this. Perhaps if we just backported #67232 to WP 6.7 this becomes a non-Issue. I don't see a huge value in Zoom Out when there is no true section root.

What do you think @richtabor @draganescu @annezazu?

@annezazu
Copy link
Contributor Author

I agree completely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Zoom Out [Type] Bug An existing feature does not function as intended
Projects
Development

No branches or pull requests

8 participants