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

Inserter: Small width when Pattern tab is selected in mobile layout #66205

Closed
2 tasks done
t-hamano opened this issue Oct 17, 2024 · 9 comments · Fixed by #66341
Closed
2 tasks done

Inserter: Small width when Pattern tab is selected in mobile layout #66205

t-hamano opened this issue Oct 17, 2024 · 9 comments · Fixed by #66341
Labels
[Feature] Zoom Out [Priority] High Used to indicate top priority items that need quick attention [Type] Bug An existing feature does not function as intended

Comments

@t-hamano
Copy link
Contributor

Description

This issue does not occur in WordPress 6.7 Beta3. It occurs in the wp/6.7 branch and the latest Gutenberg.

I think we need to fix this so that it doesn't ship in WordPress 6.7 RC1.

Step-by-step reproduction instructions

  • Open the post editor.
  • Open the main inserter.
  • Click the Patterns tab.

Screenshots, screen recording, code snippet

Image

Environment info

No response

Please confirm that you have searched existing issues in the repo.

  • Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

  • Yes
@t-hamano t-hamano added [Feature] Zoom Out [Type] Bug An existing feature does not function as intended labels Oct 17, 2024
@ndiego ndiego moved this to 📥 Todo in WordPress 6.7 Editor Tasks Oct 17, 2024
@PARTHVATALIYA
Copy link
Contributor

Hi @t-hamano,

I’ve tested the issue and created a patch that resolves it. I’ve also attached a video demonstrating the solution. If the patch looks good, I’d be happy to submit a PR.

patch:

diff --git a/packages/block-editor/src/components/inserter/menu.js b/packages/block-editor/src/components/inserter/menu.js
index bdd4ff11ab..c342760460 100644
--- a/packages/block-editor/src/components/inserter/menu.js
+++ b/packages/block-editor/src/components/inserter/menu.js
@@ -16,7 +16,7 @@ import {
 } from '@wordpress/element';
 import { VisuallyHidden, SearchControl, Popover } from '@wordpress/components';
 import { __ } from '@wordpress/i18n';
-import { useDebouncedInput } from '@wordpress/compose';
+import { useDebouncedInput, useViewportMatch } from '@wordpress/compose';
 import { useSelect } from '@wordpress/data';
 
 /**
@@ -80,7 +80,10 @@ function InserterMenu(
 
 	const shouldUseZoomOut =
 		selectedTab === 'patterns' || selectedTab === 'media';
-	useZoomOut( shouldUseZoomOut );
+
+	const isWideViewport = useViewportMatch( 'large' );
+
+	useZoomOut( shouldUseZoomOut && isWideViewport );
 
 	const [ destinationRootClientId, onInsertBlocks, onToggleInsertionPoint ] =
 		useInsertionPoint( {
@@ -312,7 +315,7 @@ function InserterMenu(
 		<div
 			className={ clsx( 'block-editor-inserter__menu', {
 				'show-panel': showPatternPanel || showMediaPanel,
-				'is-zoom-out': isZoomOutMode,
+				'is-zoom-out': isZoomOutMode && isWideViewport,
 			} ) }
 			ref={ ref }
 		>

After patch:
https://github.com/user-attachments/assets/9b841df8-7239-48b7-b628-a0b3023b5145

thanks.

@t-hamano
Copy link
Contributor Author

@PARTHVATALIYA Thanks for the suggestion. My guess is that the useZoomOut hook itself may need to be modified. Please see this comment.

@getdave
Copy link
Contributor

getdave commented Oct 21, 2024

@PARTHVATALIYA Will you be able to raise a PR for that changeset? It looks like it is the correct solution.

@PARTHVATALIYA
Copy link
Contributor

I’m happy to raise a PR for this issue, but I’m a bit unsure whether I should modify the useZoomOut hook or the menu inserter itself.

@getdave
Copy link
Contributor

getdave commented Oct 21, 2024

To me it feels like zoom out needs to be aware of the it's restriction to only be active on larger viewports.

So for me we should modify the hook because the viewport information is relevant to that context.

@draganescu draganescu added [Priority] High Used to indicate top priority items that need quick attention Good First Issue An issue that's suitable for someone looking to contribute for the first time labels Oct 21, 2024
@t-hamano
Copy link
Contributor Author

@PARTHVATALIYA

Would you try submitting a PR? However, since the useZoomOut hook is heavily changed by #66141, we'll probably need to submit separate PRs to both the trunk and the wp/6.7 branch. I'd be happy to help out.

@PARTHVATALIYA
Copy link
Contributor

@t-hamano, I have submitted a PR. Could you please review it and provide any suggestions if necessary?

@getdave getdave moved this from 📥 Todo to 🏗️ In Progress in WordPress 6.7 Editor Tasks Oct 22, 2024
@t-hamano
Copy link
Contributor Author

Fixed by #66308

The PR for WordPress 6.7 is #66330.

@github-project-automation github-project-automation bot moved this from 🏗️ In Progress to ✅ Done in WordPress 6.7 Editor Tasks Oct 22, 2024
@getdave
Copy link
Contributor

getdave commented Oct 24, 2024

As this is still an Issue in WP 6.7 I'm going to reopen this until a fix lands there.

@getdave getdave reopened this Oct 24, 2024
@getdave getdave moved this from ✅ Done to 🏗️ In Progress in WordPress 6.7 Editor Tasks Oct 24, 2024
@getdave getdave removed the Good First Issue An issue that's suitable for someone looking to contribute for the first time label Oct 24, 2024
@t-hamano t-hamano linked a pull request Oct 25, 2024 that will close this issue
@github-project-automation github-project-automation bot moved this from 🏗️ In Progress to ✅ Done in WordPress 6.7 Editor Tasks Oct 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Zoom Out [Priority] High Used to indicate top priority items that need quick attention [Type] Bug An existing feature does not function as intended
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants