1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-26 15:30:58 +01:00

Don't mutate DOM by showing a tooltip on touch events

Summary:
This fix further addresses T10229. The problem and solution are the same:

- If the DOM is mutated during a touch, it never registers as a 'click' so the tapped button does not activate.
- This was partially addressed in D15136, which covered taps on code lines in a Differential view.
- Tapping on some buttons, like "Reply" or "Hide Comment" still caused the problem by showing a tooltip.
- There are probably similar buttons elsewhere, other than in Differential, exhibiting the same 'needs multiple taps to work' behaviour.
- The testing in the iOS simulator performed for D15136 did not reveal that the problem with "Hide comment" and such remained because the small device size used for testing triggered the `!= 'desktop'` path for tooltips.

To fix it:

- Don't show tooltips for touch events. You can't 'hover' with a finger (with today's tech) so that UI paradigm doesn't apply.
- Show the tooltips for regular mouse events, even if they are on the same device. Some devices have both touch and a mouse.
- No longer try to rely on a distinction between 'desktop' and 'mobile' devices. Mobile devices like the iPad Pro are essentially desktop like, and as mentioned above, a single device could be both touch and mouse enabled. It's not about the nature of the device, it's about the nature of the interaction.

Test Plan:
- Tapped "Hide Comment", "Reply" on an iPad Pro running iOS 9.2 and got single touch responses with no tooltips.
- Tried the same on an iPhone 6 running iOS 9.2.
- Hovered over the same on a regular desktop in Safari and saw tooltips. Clicked and saw regular reactions.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D15310
This commit is contained in:
Alexander Ljungberg 2016-02-19 05:06:14 -08:00 committed by epriestley
parent 5eecff91cd
commit 4e40b17aca
2 changed files with 10 additions and 10 deletions

View file

@ -8,7 +8,7 @@
return array(
'names' => array(
'core.pkg.css' => '7935f211',
'core.pkg.js' => 'd7daa6d8',
'core.pkg.js' => '298d5888',
'darkconsole.pkg.js' => 'e7393ebb',
'differential.pkg.css' => '2de124c9',
'differential.pkg.js' => 'd0cd0df6',
@ -501,7 +501,7 @@ return array(
'rsrc/js/core/behavior-time-typeahead.js' => 'f80d6bf0',
'rsrc/js/core/behavior-toggle-class.js' => '5d7c9f33',
'rsrc/js/core/behavior-tokenizer.js' => 'b3a4b884',
'rsrc/js/core/behavior-tooltip.js' => '3ee3408b',
'rsrc/js/core/behavior-tooltip.js' => '42fcb747',
'rsrc/js/core/behavior-watch-anchor.js' => '9f36c42d',
'rsrc/js/core/behavior-workflow.js' => '0a3f3021',
'rsrc/js/core/phtize.js' => 'd254d646',
@ -648,7 +648,7 @@ return array(
'javelin-behavior-phabricator-reveal-content' => '60821bc7',
'javelin-behavior-phabricator-search-typeahead' => '06c32383',
'javelin-behavior-phabricator-show-older-transactions' => 'dbbf48b6',
'javelin-behavior-phabricator-tooltips' => '3ee3408b',
'javelin-behavior-phabricator-tooltips' => '42fcb747',
'javelin-behavior-phabricator-transaction-comment-form' => 'b23b49e6',
'javelin-behavior-phabricator-transaction-list' => '13c739ea',
'javelin-behavior-phabricator-watch-anchor' => '9f36c42d',
@ -1119,12 +1119,6 @@ return array(
'javelin-util',
'javelin-uri',
),
'3ee3408b' => array(
'javelin-behavior',
'javelin-behavior-device',
'javelin-stratcom',
'phabricator-tooltip',
),
'3f5d6dbf' => array(
'javelin-behavior',
'javelin-dom',
@ -1140,6 +1134,12 @@ return array(
'javelin-dom',
'javelin-request',
),
'42fcb747' => array(
'javelin-behavior',
'javelin-behavior-device',
'javelin-stratcom',
'phabricator-tooltip',
),
'44959b73' => array(
'javelin-util',
'javelin-uri',

View file

@ -18,7 +18,7 @@ JX.behavior('phabricator-tooltips', function() {
return;
}
if (JX.Device.getDevice() != 'desktop') {
if (e.getIsTouchEvent()) {
return;
}