iOS: Prevent accidental block hover #3139

tg-ephox · 2017-10-25T05:00:03Z

Description

Fixes #2601

Adds iOS momentum scrolling, where previously scrolling would abruptly stop once the finger was lifted from the screen.
Fixes the following reported behavior:

Also have to click twice in a block to be able to get a caret and keyboard. May be related.

How Has This Been Tested?

Manual testing on device

Screenshots (jpeg or gifs if applicable):

Types of changes

Bug fix

Checklist:

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

codecov · 2017-10-25T05:54:42Z

Codecov Report

Merging #3139 into master will decrease coverage by 0.02%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #3139      +/-   ##
==========================================
- Coverage   33.81%   33.78%   -0.03%     
==========================================
  Files         250      250              
  Lines        6734     6739       +5     
  Branches     1218     1218              
==========================================
  Hits         2277     2277              
- Misses       3763     3768       +5     
  Partials      694      694

Impacted Files	Coverage Δ
editor/modes/visual-editor/block.js	`0% <0%> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 77ea75a...dd3ddc1. Read the comment docs.

ephox-mogran · 2017-10-26T06:20:19Z

With respect to the keyboard disappearing on enter, we've found a few things that might be helpful:

a) it happens when a block node is being split. You can replicate it easily on a new post by pressing enter halfway through a paragraph block.
b) the keyboard is most likely disappearing due to a blur happening. IOS is quite opinionated about when the keyboard should be visible ... a mere focus call isn't always enough to bring it back. It also tends to avoid showing a keyboard when it feels the focus doesn't require it. There are two sections of the code that if you remove their focus logic, the keyboard does not go away.

in visual-editor/block.js in componentDidMount. This focus call is firing a focus on a div that while it may be focusable, is not a container that allows text entry, therefore IOS probably feels it should hide the keyboard.

in blocks/editable/index.js in updateFocus. There is a blur call here. A blur call instantly tells IOS to hide the keyboard (in fact, it's often what developers use to intentionally hide the keyboard). Sometimes, a subsequent focus call isn't enough to bring back the keyboard. It tends to (sometimes) depend on:

whether the focus call is within the same event queue (sort of like what is mentioned in https://developer.mozilla.org/en-US/docs/Web/JavaScript/EventLoop)
where the focus was previously
whether the newly focused element accepts keyboard input
how close you are to your project deadline

As you can probably infer, a lot of this is empirically determined, rather than read from a spec somewhere. If you know where the spec is, though, please tell us :)

So in summary, if you remove both the focus and the blur line, the keyboard stays open and the block is correctly focused. I'm not suggesting that's the solution at all ... just providing some information that might help explain what's going on. I tried to replicate the issue in a fiddle outside of Gutenberg, but had no luck. It probably depends on when React is firing focus and blur on these nodes. The sequence seems to be: previous.blur > block.focus > editable.focus, but replicating that order wasn't sufficient to recreate it elsewhere.

mcsf · 2017-10-26T23:26:32Z

editor/layout/style.scss

+.mobile .editor-layout__editor {
+	overflow-y: scroll;
+	-webkit-overflow-scrolling: touch;
+}


MDN seems very wary of using -webkit-overflow-scrolling. Are we safe? Is there a risk of targeting unwanted platforms where this could have a bad impact? We could target this to iOS.

@mcsf only webkit should interpret this from the vendor prefix and body will have the .mobile class on for mobile devices? Have tested on iPad and Nexus tablet.

only webkit should interpret this

Indeed, but my concern was whether this could affect non-iOS WebKit agents negatively. However, if you've tested on a Nexus and feel confident about this, I can withdraw my question.

Should we really target .mobile here? What is the connection between the two? Can we not just add it to .editor-layout__editor?

mcsf · 2017-10-26T23:26:52Z

Thanks for the comprehensive comment, @ephox-mogran.

@tg-ephox, could you provide full testing instructions, as well as specifics on what this should be fixing? I'm not able to repro some of the problems mentioned in the parent issue, at least not on an iPhone—can grab an iPad for a fuller review. Thanks.

tg-ephox · 2017-10-27T04:42:03Z

@mcsf the fix is for

Also have to click twice in a block to be able to get a caret and keyboard. May be related.

@iseulde reported in #2601

I added the iOS momentum scrolling as the way it was scrolling was quite unnatural for an iOS user.

mcsf

This is good to go as a smaller PR. I notice the In Progress label is present, but we can defer any other work to a new PR.

ellatrix · 2017-10-27T10:05:23Z

For the scrolling, there was also #2860.

mcsf · 2017-10-27T10:16:28Z

Since #2860 is slightly broader in reach and has been previously tested and approved, let's rebase and merge that one, and we can amend this one to only deal with the hover issue.

tg-ephox · 2017-10-30T03:35:50Z

No problems @mcsf . Sorry @iseulde I didn't see #2860 and just made a fix while I was on the iOS stuff.

ellatrix · 2017-10-30T08:26:49Z

editor/modes/visual-editor/block.js

@@ -72,6 +72,8 @@ class VisualEditorBlock extends Component {
 		this.onKeyDown = this.onKeyDown.bind( this );
 		this.onBlockError = this.onBlockError.bind( this );
 		this.insertBlocksAfter = this.insertBlocksAfter.bind( this );
+		this.onTouchStart = this.onTouchStart.bind( this );
+		this.onClick = this.onClick.bind( this );


What would be the diff between touch start and click here? Might it be better stick to touch events entirely to set this.hadTouchStart?

I tried onTouchEnd but it fires too soon it needs to be after mousemove

ellatrix · 2017-10-30T08:27:58Z

It would be nice to have some inline comments for this browser fix. I will test in just a bit on a device.

ellatrix · 2017-10-30T09:36:05Z

This works for me on iOS. I'm surprised mouse enter/move fire there. :/

ellatrix · 2018-01-31T12:56:49Z

This PR closed #2601 while only one non related issue was fixed.

Enable momentum scrolling on editor body

b63154d

tg-ephox added the [Status] In Progress Tracking issues with work in progress label Oct 25, 2017

tg-ephox self-assigned this Oct 25, 2017

Prevent block hover when touchstart received

27a476a

ephox-mogran requested review from ellatrix, mcsf, youknowriad and aduth October 26, 2017 21:08

mcsf reviewed Oct 26, 2017

View reviewed changes

mcsf changed the title ~~iOS issues~~ iOS: Prevent accidental block hover, add momentum scrolling Oct 27, 2017

mcsf approved these changes Oct 27, 2017

View reviewed changes

Remove iOS touch scrolling in favour of #2860

cefd778

tg-ephox changed the title ~~iOS: Prevent accidental block hover, add momentum scrolling~~ iOS: Prevent accidental block hover Oct 30, 2017

tg-ephox removed the [Status] In Progress Tracking issues with work in progress label Oct 30, 2017

ellatrix reviewed Oct 30, 2017

View reviewed changes

tg-ephox added 2 commits November 13, 2017 16:32

Merge branch 'master' into try/2601-ios-issues

b501d9c

Add comment

dd3ddc1

tg-ephox merged commit c1ccd85 into master Nov 13, 2017

tg-ephox deleted the try/2601-ios-issues branch November 13, 2017 23:35

aduth mentioned this pull request Apr 29, 2019

Update the block component to use react hooks instead #14985

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

iOS: Prevent accidental block hover #3139

iOS: Prevent accidental block hover #3139

tg-ephox commented Oct 25, 2017 •

edited by mcsf

Loading

codecov bot commented Oct 25, 2017 •

edited

Loading

ephox-mogran commented Oct 26, 2017

mcsf Oct 26, 2017

tg-ephox Oct 27, 2017

mcsf Oct 27, 2017

ellatrix Oct 27, 2017

mcsf commented Oct 26, 2017

tg-ephox commented Oct 27, 2017 •

edited

Loading

mcsf left a comment

ellatrix commented Oct 27, 2017

mcsf commented Oct 27, 2017

tg-ephox commented Oct 30, 2017

ellatrix Oct 30, 2017

tg-ephox Nov 13, 2017

ellatrix commented Oct 30, 2017

ellatrix commented Oct 30, 2017

ellatrix commented Jan 31, 2018

iOS: Prevent accidental block hover #3139

iOS: Prevent accidental block hover #3139

Conversation

tg-ephox commented Oct 25, 2017 • edited by mcsf Loading

Description

How Has This Been Tested?

Screenshots (jpeg or gifs if applicable):

Types of changes

Checklist:

codecov bot commented Oct 25, 2017 • edited Loading

Codecov Report

ephox-mogran commented Oct 26, 2017

mcsf Oct 26, 2017

Choose a reason for hiding this comment

tg-ephox Oct 27, 2017

Choose a reason for hiding this comment

mcsf Oct 27, 2017

Choose a reason for hiding this comment

ellatrix Oct 27, 2017

Choose a reason for hiding this comment

mcsf commented Oct 26, 2017

tg-ephox commented Oct 27, 2017 • edited Loading

mcsf left a comment

Choose a reason for hiding this comment

ellatrix commented Oct 27, 2017

mcsf commented Oct 27, 2017

tg-ephox commented Oct 30, 2017

ellatrix Oct 30, 2017

Choose a reason for hiding this comment

tg-ephox Nov 13, 2017

Choose a reason for hiding this comment

ellatrix commented Oct 30, 2017

ellatrix commented Oct 30, 2017

ellatrix commented Jan 31, 2018

tg-ephox commented Oct 25, 2017 •

edited by mcsf

Loading

codecov bot commented Oct 25, 2017 •

edited

Loading

tg-ephox commented Oct 27, 2017 •

edited

Loading