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

Fix: Activate zoom out on large viewport #66308

Merged
merged 3 commits into from
Oct 22, 2024

Conversation

PARTHVATALIYA
Copy link
Contributor

What?

Solve #66205

Why?

Part of #66205

Testing Instructions

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

Copy link

github-actions bot commented Oct 22, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: PARTHVATALIYA <parthvataliya@git.wordpress.org>
Co-authored-by: t-hamano <wildworks@git.wordpress.org>
Co-authored-by: getdave <get_dave@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@getdave getdave added [Type] Bug An existing feature does not function as intended [Feature] Zoom Out labels Oct 22, 2024
Copy link
Contributor

@getdave getdave left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this PR 👍

To test this accurately I needed to set my viewport to around 587px. Is that right?

Here's what I saw

Before (trunk)

Screenshot 2024-10-22 at 10 08 43

After (this branch)

Screenshot 2024-10-22 at 10 09 42

Once this is merged we'll need a similar fix targeting wp/6.7 branch. Let's wait for this one to merge before we tackle that though so as not to duplicate effort.

Thanks again for your contribution here 🙇

@getdave getdave added the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Oct 22, 2024
Copy link
Contributor

@t-hamano t-hamano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

I think we need to add isWideViewport to the useEffect hook dependencies (here and here) to completely solve this issue, otherwise the inserter width will not change if we change the viewport width with the Patterns tab open:

1e4f54c3670a65fe34194ccbf6949c3c.mp4

Copy link
Contributor

@t-hamano t-hamano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@getdave Does this PR need a backport label? Is it okay to merge it as is, since the automatic cherry-pick will probably fail?

Either way, I think we need a separate PR for wp/6.7.

@t-hamano
Copy link
Contributor

Merged trunk to fix lint errors. See #66316.

@t-hamano
Copy link
Contributor

Just to be safe, I will remove the backport label before merging. I will create a PR for WP 6.7 now.

@t-hamano t-hamano removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Oct 22, 2024
@t-hamano t-hamano merged commit c926d69 into WordPress:trunk Oct 22, 2024
62 checks passed
@github-actions github-actions bot added this to the Gutenberg 19.6 milestone Oct 22, 2024
@getdave
Copy link
Contributor

getdave commented Oct 22, 2024

@t-hamano Yes let's do a manual backport and then add to the 6.7 board for review. Thanks

@getdave
Copy link
Contributor

getdave commented Oct 22, 2024

Manual backport in #66330

@jeryj
Copy link
Contributor

jeryj commented Oct 22, 2024

I don't think this is the fix we want. I think the viewport check should be passed into the useZoomOut() hook. I opened a PR to demonstrate what I mean: #66341

karthick-murugan pushed a commit to karthick-murugan/gutenberg that referenced this pull request Nov 13, 2024
* Activate zoom out on large viewport

* Reset zoom out when view-port change to medium

---------

Co-authored-by: PARTHVATALIYA <parthvataliya@git.wordpress.org>
Co-authored-by: t-hamano <wildworks@git.wordpress.org>
Co-authored-by: getdave <get_dave@git.wordpress.org>
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
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants