Wrap overflow: auto in media query for smaller screens only
What?
Addresses #66161 and #66156
Why?
The appearance of double vertical scrollbars was occurring in certain instances of the interface where the viewport height was small. It seemed to be triggered by the overflow: auto, which had a previous code comment indicating that the overflow: auto was implemented to circumvent issues with mobile Safari. Based on the commentary it seems we would want the overflow: auto to only impact smaller horizontal screens and not smaller vertical screens.
How?
By wrapping the overflow: auto in a media query: @media (max-width: #{ ($break-medium - 1) }) we make sure the overflow is only applied to smaller horizontal screens.
Testing Instructions
The original bug reported in #66161 impacts small vertical screens.
- Open the block editor on a desktop device with a small vertical height.
- Open the block inserter sidebar and note the right-most vertical scrollbar(s)
- Begin typing to find a block and note the right-most vertical scrollbar(s)
Screenshots or screencast
| Before | After |
|---|---|
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: colorful-tones <[email protected]>
Co-authored-by: t-hamano <[email protected]>
Co-authored-by: dhruvang21 <[email protected]>
Co-authored-by: talldan <[email protected]>
Co-authored-by: youknowriad <[email protected]>
Co-authored-by: stokesman <[email protected]>
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.
Probably could use some more cross-device and browser size testing.
This also addresses #66156
Hello @colorful-tones,
I have tested this PR using the Gutenberg PR reviewer, as well as in my local Chrome and Firefox browsers across various screen sizes, and found that it resolves the issue perfectly. I’m also attaching a video of the testing for your reference.
Thanks!
https://github.com/user-attachments/assets/dedbef0b-ff6c-4e9b-92be-cc4468265f01
I tested this PR, I don't think it's the right approach - the removed overflow: auto makes the Code Editor mode unscrollable. I have sympathies as trying to navigate through these styles is very challenging.
I'll also ping @stokesman and @talldan, who have been involved with this issue in the past.
This was a fix for the device previews third scrollbar in 6.5 - https://github.com/WordPress/gutenberg/pull/62952. The display: flex that was added there to fix things was removed by this PR - https://github.com/WordPress/gutenberg/pull/65977 (cc @youknowriad). Adding it back again does seem to fix the issue again, though maybe it also regresses something that Riad was trying to fix. I should've added a comment above the line in my original PR to explain what it was for.
The problem that occurs when searching for blocks has, in the past, been triggered by visually hidden text that overflows outside the inserter sidebar (see https://github.com/WordPress/gutenberg/pull/44853, and I'm pretty there was also a fix in 6.5 as well). I'm not sure if it's the same issue that we're seeing now though, the fixes from previous releases still seem to be there.
The main issue I was trying to fix on that PR is that the "height" prop of the block canvas should actually work (you can confirm this by running the Box story in storybook) and that the "width" of the canvas is 100%. I do remember removing that flex but I'm not sure about the reason, I thought it was breaking something but after a quick manual check, I'm not sure if it breaks anything. So we could consider restoring it and just checking the height/width of the Box story.
the removed overflow: auto makes the Code Editor mode unscrollable.
I pushed up another change that addresses this.
I have sympathies as trying to navigate through these styles is very challenging.
Yes, I remember staring at that exact line of code overflow: auto during the WP 6.5 release and trying to fix a layout issue. It has been haunting me, and surfacing again and again.
After the latest change, the PR doesn't seem to be fixing either of the bugs unfortunately. I now get the extra scrollbars again when testing.
I think it might be worth looking to reinstate the display: flex if we can be sure it doesn't regress the story mentioned by Riad.
I did some further digging into the issue with the inserter and it looks like it's caused by this bit of VisuallyHidden text: https://github.com/WordPress/gutenberg/blob/d330f30a528a252c56ee4adbee2f8ee5f82a0312/packages/block-editor/src/components/inserter/search-results.js#L186-L190
I'm not sure why it suddenIy caused an issue (the code has been there for a while), perhaps some changes to the styles. I put together a PR to fix it - #66229.
I think it might be worth looking to reinstate the
display: flexif we can be sure it doesn't regress the story mentioned by Riad.
Yes, or otherwise find another way to fix #66156.
Now that #66229 has been merged, can we close this PR and the following issue?
- https://github.com/WordPress/gutenberg/issues/66156
Now that #66229 has been merged, can we close this PR and the following issue?
I still see #66156 in trunk and in the wp/6.7 branch so it appears yet to be resolved. I’ve opened up #66494 aimed to fix it.
I think the problem this PR is trying to solve is solved by #66229 and #66494.