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

RichText: remove iOS scroll adjusting #11971

Merged
merged 2 commits into from Nov 16, 2018

Conversation

Projects
None yet
5 participants
@iseulde
Member

iseulde commented Nov 16, 2018

Description

With our new smooth enter behaviour (#11287), we no longer have a focus delay when creating a new paragraph on enter. It seems that we can remove the iOS fixes that adjust the scroll position when a block receives focus and when enter/delete is pressed.

Removing the fix actually results in even nicer scrolling because it is handled natively.

Reverts #5769.

How has this been tested?

Test scroll behaviour on iOS when pressing enter and backspace.

Screenshots

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.

@iseulde iseulde added this to the 4.5 milestone Nov 16, 2018

@iseulde iseulde requested review from mcsf, jasmussen and WordPress/gutenberg-core Nov 16, 2018

@iseulde iseulde force-pushed the try/smooth-ios-enter branch from 4e1b54d to 74d1a48 Nov 16, 2018

@iseulde iseulde changed the title from RichText: remove scroll adjusting to RichText: remove iOS scroll adjusting Nov 16, 2018

@jasmussen

This comment has been minimized.

Contributor

jasmussen commented Nov 16, 2018

Wow. This is hardcore impressive refactorings you've been doing. I love the amount of red in this PR! And the experience seems great to me:

testing

I think it's important we get a few more people testing this as a sanity check, remember it has to be a physical device, or the Xcode simulator. But first impression looks seriously promising. 👏👏👏👏👏

One issue that is completely unrelated, but becomes apparent again when testing this: we probably should never invoke "isEditing" mode on mobile, and always show the toolbar and selection borders. Reason being that it's otherwise both difficult and unintuitive to figure out how to make text bold. We are a bit limited here in what we can do because we are not a native app, and we are at the mercy of Mobile Safari (see also this) — but it still seems a better experience. But again, that's unrelated to this PR.

REALLY nice work!

@jasmussen

This comment has been minimized.

Contributor

jasmussen commented Nov 16, 2018

Notably I'm super duper thrilled to see that the jumping toolbar is gone from this branch 💥

@iseulde

This comment has been minimized.

Member

iseulde commented Nov 16, 2018

I have the same result as you shared in the video on a physical device. :)

@gziolo

gziolo approved these changes Nov 16, 2018

Code removal looks legit 👍 I haven't tested but I trust you two :)

@iseulde

This comment has been minimized.

Member

iseulde commented Nov 16, 2018

Tested it also on a different device. Thanks!

@iseulde iseulde merged commit 68a6bd5 into master Nov 16, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@iseulde iseulde deleted the try/smooth-ios-enter branch Nov 16, 2018

@mcsf

This comment has been minimized.

Contributor

mcsf commented Nov 16, 2018

So glad to see this code go. :)

@mtias mtias added the Writing Flow label Nov 16, 2018

grey-rsi pushed a commit to OnTheGoSystems/gutenberg that referenced this pull request Nov 22, 2018

RichText: remove iOS scroll adjusting (WordPress#11971)
* RichText: remove scroll adjusting

* Remove isViewportSmall prop
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment