From ac1280c8ce9a912b80df39ad2e4904642f40e00d Mon Sep 17 00:00:00 2001 From: Becky Siegel Date: Mon, 17 Oct 2016 17:49:31 -0700 Subject: [PATCH] Don't display tooltips on touch devices Previously on iOS devices, tooltips in reply dialog did not always close when clicking outside of the button. The focusOut event was not triggered and thus _handleHideTooltip was not called. On other devices, the tooltip remained open when clicking on it. This change detects when a device triggers a touch event and sets a boolean variable that it is a touch device. If touch, we do not open and close tooltips. This was necessary because mouseenter and mouseleave events are still triggered on touch devices with a touch. Also, mouseover and mouseout events were replaced with mousenter and mouseleave events because it seemed better practice since they don't bubble to child elements & this is what is done in paper-tooltip. Bug: Issue 4649 Change-Id: Ia5605617b00cd9aa8b6d9ef3a4c453faf4b1a4b2 --- .../gr-tooltip-behavior.js | 22 +++++++--- .../gr-reply-dialog/gr-reply-dialog_test.html | 40 +++++++++++++++++++ 2 files changed, 56 insertions(+), 6 deletions(-) diff --git a/polygerrit-ui/app/behaviors/gr-tooltip-behavior/gr-tooltip-behavior.js b/polygerrit-ui/app/behaviors/gr-tooltip-behavior/gr-tooltip-behavior.js index 3702c841d2..c910d8f003 100644 --- a/polygerrit-ui/app/behaviors/gr-tooltip-behavior/gr-tooltip-behavior.js +++ b/polygerrit-ui/app/behaviors/gr-tooltip-behavior/gr-tooltip-behavior.js @@ -22,6 +22,12 @@ properties: { hasTooltip: Boolean, + _isTouchDevice: { + type: Boolean, + value: function() { + return 'ontouchstart' in document.documentElement; + }, + }, _tooltip: Element, _titleText: String, }, @@ -29,10 +35,10 @@ attached: function() { if (!this.hasTooltip) { return; } - this.addEventListener('mouseover', this._handleShowTooltip.bind(this)); - this.addEventListener('mouseout', this._handleHideTooltip.bind(this)); - this.addEventListener('focusin', this._handleShowTooltip.bind(this)); - this.addEventListener('focusout', this._handleHideTooltip.bind(this)); + this.addEventListener('mouseenter', this._handleShowTooltip.bind(this)); + this.addEventListener('mouseleave', this._handleHideTooltip.bind(this)); + this.addEventListener('tap', this._handleHideTooltip.bind(this)); + this.listen(window, 'scroll', '_handleWindowScroll'); }, @@ -41,6 +47,8 @@ }, _handleShowTooltip: function(e) { + if (this._isTouchDevice) { return; } + if (!this.hasAttribute('title') || this.getAttribute('title') === '' || this._tooltip) { @@ -66,9 +74,11 @@ }, _handleHideTooltip: function(e) { + if (this._isTouchDevice) { return; } if (!this.hasAttribute('title') || - this._titleText == null || - this === document.activeElement) { return; } + this._titleText == null) { + return; + } this.setAttribute('title', this._titleText); if (this._tooltip && this._tooltip.parentNode) { diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html index 6c77afbb71..a8a849407a 100644 --- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html +++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html @@ -415,5 +415,45 @@ limitations under the License. MockInteractions.tap(element.$$('.send')); }); }); + + test('don"t display tooltips on touch devices', function() { + element.labels = { + Verified: { + values: { + '-1': 'Fails', + ' 0': 'No score', + '+1': 'Verified' + }, + default_value: 0 + }, + 'Code-Review': { + values: { + '-2': 'Do not submit', + '-1': 'I would prefer that you didn\'t submit this', + ' 0': 'No score', + '+1': 'Looks good to me, but someone else must approve', + '+2': 'Looks good to me, approved' + }, + default_value: 0 + } + }; + var verifiedBtn = element.$$( + 'iron-selector[data-label="Verified"] > ' + + 'gr-button[data-value="-1"]'); + + // On touch devices, tooltips should not be shown + verifiedBtn._isTouchDevice = true; + verifiedBtn._handleShowTooltip(); + assert.isNotOk(verifiedBtn._tooltip); + verifiedBtn._handleHideTooltip(); + assert.isNotOk(verifiedBtn._tooltip); + + // On other devices, tooltips should be shown. + verifiedBtn._isTouchDevice = false; + verifiedBtn._handleShowTooltip(); + assert.isOk(verifiedBtn._tooltip); + verifiedBtn._handleHideTooltip(); + assert.isNotOk(verifiedBtn._tooltip); + }); });