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

Polish mobile, notably iPhone #4081

Merged
merged 5 commits into from
Dec 21, 2017
Merged

Polish mobile, notably iPhone #4081

merged 5 commits into from
Dec 21, 2017

Conversation

jasmussen
Copy link
Contributor

This is sort of a kitchen sink PR of small fixes. It:

  • Polishes the inserter tabs so the focus style isn't clippsed
  • Tweaks margins in the toolbar on mobile, so things aren't shifted around
  • It undoes that individually scrolling content area and makes the body scroll. More on this in a second.
  • It polishes how the fixed-to-block toolbar looks on mobile
  • It changs how the fixed position toolbars behave, making them sticky, otherwise it wouldn't work with the WP admin bar properly
  • It rotates the ellipsis button that shows mobile tools so it matches the top ellipsis. This needs more work separately, with the idea of instead showing these tools when the block is selected.
  • It prevents Mobile Safari from zooming the entire page when you open the inserter (it does this because the textfield receives focus, and Mobile Safari deems the 13px font too small, so it zooms)

As such, the above fixes part of the issues described in #4077, notably the zooming and scrolling issues.

How Has This Been Tested?

Tested in Chrome simulator, as well as the Xcode iPhone 8 simulator.

Screenshots

screen shot 2017-12-19 at 11 46 40
screen shot 2017-12-19 at 11 46 45
screen shot 2017-12-19 at 11 46 50
screen shot 2017-12-19 at 11 46 59

Here's a video showing the iOS simulator:

https://cloudup.com/ccvLjdy6cT3

Outside of the iOS changes, which make Gutenberg usable, I'm going to open a separate ticket about modal improvements (some work to be done there). But because these changes also affect the desktop breakpoints, we should be sure to test this PR fairly widely.

@jasmussen jasmussen added Design Mobile Web Viewport sizes for mobile and tablet devices [Type] Enhancement A suggestion for improvement. labels Dec 19, 2017
@jasmussen jasmussen self-assigned this Dec 19, 2017
@karmatosed
Copy link
Member

This is great. It does need really thorough testing though. It's important as fixes some of the experience issues 'right now' and we can then iterate.

overflow-y: auto;
-webkit-overflow-scrolling: touch;

// on mobile the main content area has to scroll
Copy link
Member

Choose a reason for hiding this comment

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

Is this targeting iOS specifically? Does it impact larger-display iOS like iPads? Is the -webkit-overflow-scrolling style unnecessary if we're intending to exclude it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm essentially normalizing the scrolling behavior across the mobile breakpoint, yes. And though the iPad also suffers from the same scrolling issue, the larger screen real estate makes the scroll issue less annoying, so it'd be good to keep the property there.

Joen Asmussen added 4 commits December 20, 2017 15:48
That is, scroll the `html` element, instead of a specially dedicated area. Although the latter has a ton of benefits, the former is the only thing that works stably on Mobile Safari. The thing is, there's overscroll bounce that you can't really turn off, so if you aren't scrolling the main content area things are just going to be all kinds of janky.
Also rotate ellipsis so it matches top toolbar one.
@youknowriad
Copy link
Contributor

youknowriad commented Dec 20, 2017

Nice work, I have some remarks:

  • You removed the margins around the block mobile toolbar, should we do the same for the "permalink toolbar" when you focus the title?

  • A small glitch, I think related to the fact that you changed how to fix the block toolbar: I expect the editor's top padding/margin to stay the same if the same toolbars are shown. But there's a difference in one of the breakpoints. Take a look at the gif and see how the editor's content jumps when resizing the window.

margin change

- Add padding when fixed toolbar is enabled so the white sheet doesn't cover the permalink bar
- Fit the permalink bar snugly to the screen edges on mobile
@jasmussen
Copy link
Contributor Author

Great great catches, Riad. I believe I fixed them now:

fixes

@jasmussen
Copy link
Contributor Author

Going to merge in a bit unless you have last minute objections, Riad?

Thanks.

@jasmussen jasmussen merged commit af913fa into master Dec 21, 2017
@jasmussen jasmussen deleted the try/mobile-polish branch December 21, 2017 10:20
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 [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants