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

Mobile fixed toolbar: Scroll focused element into view #19014

Merged
merged 1 commit into from
Dec 9, 2019

Conversation

frontdevde
Copy link
Contributor

Description

This is a followup PR to #18686 which added a fixed toolbar to mobile.

This PR updates the hack we're using to make this work on mobile Safari (see #18632 for more context). With the hack, we reset the unwanted scroll that mobile Safari adds to the document when the soft keyboard is opened. This update now also scrolls the editor container by the value that mobile Safari tried to scroll the document. This way we achieve what mobile Safari was trying to do: scroll the element the user wants to interact with into view.

How has this been tested?

Tested in mobile Safari on a physical iPhone 11 with iOS 13.2.3 installed.

Screenshots

Before

ezgif com-video-to-gif (5)

After

ezgif com-video-to-gif (4)

Types of changes

Bug fix

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR. .

@frontdevde frontdevde added the Mobile Web Viewport sizes for mobile and tablet devices label Dec 9, 2019
@frontdevde frontdevde requested a review from jasmussen December 9, 2019 12:59
@frontdevde frontdevde requested a review from talldan as a code owner December 9, 2019 12:59
@frontdevde frontdevde self-assigned this Dec 9, 2019
Copy link
Contributor

@jasmussen jasmussen 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 keeping on this. Creating a usable mobile web experience is so key.

The difference in this PR to master is very subtle, blink and you'll miss it. But it seems to work well for me — which is impressive considering the state of Mobile Safari.

Here's the behavior in master:

master

Notice how when you click the bottom of a long paragraph, that bit gets covered by the soft keyboard so you have to swipe it into view to edit it.

This branch:

this branch

It's consistent most of the time: the place where you set the caret is what gets scrolled into view. It's not 100% solid, but it's much better than what we have today.

Code also looks well documented, simple, and iPhone specific.

@frontdevde
Copy link
Contributor Author

It's consistent most of the time: the place where you set the caret is what gets scrolled into view. It's not 100% solid, but it's much better than what we have today.

@jasmussen This is due to a 100px threshold that you can find in the code. I probably should've added a comment around that. If the browser tried to scroll less than 100px we don't scroll the editor container. The reason being that otherwise unintentional scrolls on the toolbar could lead the document in the editor to scroll. The same goes for over scrolling. 100px seemed like the right point to draw the line where it worked consistently enough to create the wanted effect in the editor while eliminating most of the unwanted side effects. As you said, "it's much better than what we have today."

@frontdevde frontdevde merged commit 86b1f6e into master Dec 9, 2019
@frontdevde frontdevde deleted the update/fixed-mobile-toolbar-2 branch December 9, 2019 15:07
@youknowriad youknowriad added this to the Gutenberg 7.1 milestone Dec 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Mobile Web Viewport sizes for mobile and tablet devices
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants