Search: Prevent visible page scroll when the search modal is open on iOS #392
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The button is still focused when the menu is closed, which makes it look active when using a pointer device. When using a keyboard, the outline also appears, which makes it clear where focus is.
iandunn
approved these changes
Apr 25, 2023
This was referenced Apr 25, 2023
|
I'm unable to test this in my local environment, but for those able to, I've had good success with the free Xcode simulator to test. If it works, ship it. I also want to note that I appreciate that you shared your experiences on the upstream fix, so we can get this fixed at the core! Thank you 🙏 |
|
I see the search icon twice. The one in the placeholder should not exist, as my understanding. Is that a OS behavior? |
|
Good catch, Francisco. I think I actually have an upstream fix for that: WordPress/gutenberg#48259 and WordPress/gutenberg#49710 |
ryelle
added a commit
that referenced
this pull request
May 30, 2023
This prevents other nav modals (like the page subnav) which toggle the has-modal-open class from affecting the position of the global header bar. See #392, Fixes WordPress/wporg-parent-2021#85
ryelle
added a commit
that referenced
this pull request
May 31, 2023
* Header: Fix logo spacing on default English site Fixes issue introduced in 43a8a94 where the menu & search icons collapsed to the left on small screens. The logo flex size needs to be different when there is no locale name. * Header: Only apply page scroll fix to header modals This prevents other nav modals (like the page subnav) which toggle the has-modal-open class from affecting the position of the global header bar. See #392, Fixes WordPress/wporg-parent-2021#85
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
On iOS, opening the search modal in the header triggers the keyboard to open, which re-enables page scrolling. When scrolling down & back up, the page contents are visible in the gap behind the modal. This PR fixes the header bar on scroll, so that it hides that scrolled content.
It is still strange that the page can be scrolled, but the other fixes cause larger layout issues (see #371 (comment)), and this only happens when the keyboard is open (and only on iOS) this seems like the best solution.
Fixes #371, see #381, see #382.
Note: this PR also includes a small fix to prevent the icon-buttons from appearing active when the tab is closed, I only noticed it on mobile, so fixed it here.
Screenshots
To test
I used Charles.app's DNS spoofing to send *.wordpress.org traffic to my sandbox, then set my iPad to use my computer as a proxy. This let me load my sandboxed environment on my iPad. Then using the vertical orientation, I tested the header across different wporg pages.