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

[ETK][GB 16.5.0] Workaround for disappearing toolbar settings button in (some) mobile viewport sizes #81050

Merged
merged 2 commits into from
Aug 25, 2023

Conversation

fullofcaffeine
Copy link
Contributor

@fullofcaffeine fullofcaffeine commented Aug 24, 2023

Proposed Changes

Make sure the settings button in the toolbar in the post editor is always visible.

The problem was introduced in this PR: WordPress/gutenberg#53412

I'm not acquainted with the rationale/specifics behind those changes, instead, I'm going to focus here on the UX issue this caused in WPCOM: the settings button can't be seen in some mobile viewport sizes and there's no alternative (that I could find) to bring it anywhere else in the UI -- this means if the user has the bad luck of reaching that specific size where the button doesn't appear, then they won't be able to bring up the settings panel at all:

Peek.2023-08-24.16-47.mp4

There's an ongoing discussion on Slack here: p1692639188443899-slack , and @draganescu created an issue for it in the GB core repo: WordPress/gutenberg#53899. There's also a WIP PR by him: WordPress/gutenberg#53908, but unfortunately, my tests were unsuccessful. Because of that, I decided to work on this temporary workaround.

Testing Instructions

  1. Make sure GB 16.5.0 is active in your simple or AT site.
  2. Create a new post and then turn the responsive view on. Resize it until you're able to reproduce the issue (see video above)
  3. From this branch, go to apps/editing-toolkit and run yarn dev --sync to sync the custom ETK build to your sandbox. For the AT site, you'll need to turn it into a dev site and then sync using rsync like so wp-calypso/apps/editing-toolkit/editing-toolkit-plugin $ yarn && yarn build && rsync -azP -vvv . --exclude .git --exclude node_modules --delete <yourtestatsite>.wordpress.com@sftp.wp.com:/home/<site-id>/htdocs/wp-content/plugins/full-site-editing
  4. Refresh the new post page - resize again and you should always see the settings button now.

Here are some screencasts of the happy paths with GB 16.5 in the post editor:

It does make the settings button sticky in the site editor -- before this change, in the site editor, the settings button would disappear in some mobile viewports, but now, due to this change, it will always be visible. I think it's a benign change and considering this is a temporary workaround, it's okay to leave it like this until a fix in core is implemented cc @draganescu[

⚠️ One thing I noticed is that in the site editor, the "settings" item/button is also shown in the ellipsis dropdown menu under "plugins". This is not the case in the post editor and what made this fix more important as there's no other way to access the button if it's hidden there. So probably better implementation for this fix would take care of this, too, though it's out of the scope of this quick workaround (and would probably/ideally need to be done in GB, not here):

Post editor Site editor
2023-08-24_19-05 2023-08-24_19-04

@github-actions
Copy link

github-actions bot commented Aug 24, 2023

@matticbot
Copy link
Contributor

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

Copy link
Contributor

@griffbrad griffbrad left a comment

Choose a reason for hiding this comment

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

Looks good and tests well for me!

@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Aug 25, 2023
@matticbot
Copy link
Contributor

This PR modifies the release build for editing-toolkit

To test your changes on WordPress.com, run install-plugin.sh editing-toolkit fix/settings-icon-disappearing-mobile-viewports on your sandbox.

To deploy your changes after merging, see the documentation: PCYsg-mMA-p2

@fullofcaffeine fullofcaffeine merged commit 1a50a7e into trunk Aug 25, 2023
10 checks passed
@fullofcaffeine fullofcaffeine deleted the fix/settings-icon-disappearing-mobile-viewports branch August 25, 2023 01:21
@fullofcaffeine fullofcaffeine added the caused-by-gutenberg-upgrade Labels an issue (or PR) as caused by or related to a Gutenberg upgrade on WPCOM. label Aug 25, 2023
@github-actions github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Aug 25, 2023
fullofcaffeine added a commit that referenced this pull request Oct 9, 2023
… button in (some) mobile viewport sizes (#81050)"

This reverts commit 1a50a7e.
fullofcaffeine added a commit that referenced this pull request Oct 9, 2023
… button in (some) mobile viewport sizes (#81050)" (#82777)

This reverts commit 1a50a7e.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
caused-by-gutenberg-upgrade Labels an issue (or PR) as caused by or related to a Gutenberg upgrade on WPCOM.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants