From 6f5e0ec48a10a6b6921c29a628663150024788b7 Mon Sep 17 00:00:00 2001 From: Christopher Chedeau Date: Fri, 28 Oct 2016 16:12:00 -0700 Subject: [PATCH 1/3] Introduce follow through behavior for tooltips Inside of Nuclide, we have multiple places where we have multiple icons close together that have a tooltip: the left toolbar, the bottom status bar, the debugger icons... The current behavior is very frustrating as you've got to wait for the delay on every single one of them. But, we have a clear signal that the user wants a tooltip when s/he waits the time to see it and it's likely that moving the mouse over the next item quickly means that s/he wants to see it as well. This pull request introduces the concept of follow through: if you have seen a tooltip, you're going to instantly see tooltip on the next element you mouse over within a short timer (300ms right now). Test Plan: Video before: ![](http://g.recordit.co/7PCg0hjohP.gif) Video after: ![](http://g.recordit.co/9OnZCvy9oI.gif) Released under CC0 --- spec/tooltip-manager-spec.coffee | 48 +++++++++++++++++++++++++++----- src/tooltip.js | 18 ++++++++++-- 2 files changed, 57 insertions(+), 9 deletions(-) diff --git a/spec/tooltip-manager-spec.coffee b/spec/tooltip-manager-spec.coffee index 6bebd6e76..3d7124d68 100644 --- a/spec/tooltip-manager-spec.coffee +++ b/spec/tooltip-manager-spec.coffee @@ -1,4 +1,5 @@ TooltipManager = require '../src/tooltip-manager' +Tooltip = require '../src/tooltip' _ = require 'underscore-plus' describe "TooltipManager", -> @@ -9,17 +10,27 @@ describe "TooltipManager", -> beforeEach -> manager = new TooltipManager(keymapManager: atom.keymaps, viewRegistry: atom.views) - element = document.createElement('div') - element.classList.add('foo') - jasmine.attachToDOM(element) + element = createElement 'foo' - hover = (element, fn) -> + createElement = (className) -> + el = document.createElement('div') + el.classList.add(className) + jasmine.attachToDOM(el) + el + + mouseEnter = (element) -> element.dispatchEvent(new CustomEvent('mouseenter', bubbles: false)) element.dispatchEvent(new CustomEvent('mouseover', bubbles: true)) - advanceClock(manager.hoverDefaults.delay.show) - fn() + + mouseLeave = (element) -> element.dispatchEvent(new CustomEvent('mouseleave', bubbles: false)) element.dispatchEvent(new CustomEvent('mouseout', bubbles: true)) + + hover = (element, fn) -> + mouseEnter(element) + advanceClock(manager.hoverDefaults.delay.show) + fn() + mouseLeave(element) advanceClock(manager.hoverDefaults.delay.hide) describe "::add(target, options)", -> @@ -149,6 +160,29 @@ describe "TooltipManager", -> it "hides the tooltips", -> manager.add element, title: "Title" hover element, -> - expect(document.body.querySelector(".tooltip")).toBeDefined() + expect(document.body.querySelector(".tooltip")).not.toBeNull() window.dispatchEvent(new CustomEvent('resize')) expect(document.body.querySelector(".tooltip")).toBeNull() + + it "works with follow-through", -> + element1 = createElement('foo') + manager.add element1, title: 'Title' + element2 = createElement('bar') + manager.add element2, title: 'Title' + element3 = createElement('baz') + manager.add element3, title: 'Title' + + hover element1, -> + expect(document.body.querySelector(".tooltip")).toBeNull() + + mouseEnter(element2) + expect(document.body.querySelector(".tooltip")).not.toBeNull() + mouseLeave(element2) + advanceClock(manager.hoverDefaults.delay.hide) + expect(document.body.querySelector(".tooltip")).toBeNull() + + advanceClock(Tooltip.FOLLOW_THROUGH_DURATION) + mouseEnter(element3) + expect(document.body.querySelector(".tooltip")).toBeNull() + advanceClock(manager.hoverDefaults.delay.show) + expect(document.body.querySelector(".tooltip")).not.toBeNull() diff --git a/src/tooltip.js b/src/tooltip.js index f0f9d1a3f..2573b1fb9 100644 --- a/src/tooltip.js +++ b/src/tooltip.js @@ -7,6 +7,8 @@ const listen = require('./delegated-listener') // This tooltip class is derived from Bootstrap 3, but modified to not require // jQuery, which is an expensive dependency we want to eliminate. +var followThroughTimer = null + var Tooltip = function (element, options, viewRegistry) { this.options = null this.enabled = null @@ -21,7 +23,7 @@ var Tooltip = function (element, options, viewRegistry) { Tooltip.VERSION = '3.3.5' -Tooltip.TRANSITION_DURATION = 150 +Tooltip.FOLLOW_THROUGH_DURATION = 300 Tooltip.DEFAULTS = { animation: true, @@ -151,7 +153,11 @@ Tooltip.prototype.enter = function (event) { this.hoverState = 'in' - if (!this.options.delay || !this.options.delay.show) return this.show() + if (!this.options.delay || + !this.options.delay.show || + followThroughTimer) { + return this.show() + } this.timeout = setTimeout(function () { if (this.hoverState === 'in') this.show() @@ -343,6 +349,14 @@ Tooltip.prototype.hide = function (callback) { this.hoverState = null + clearTimeout(followThroughTimer) + followThroughTimer = setTimeout( + function () { + followThroughTimer = null + }, + Tooltip.FOLLOW_THROUGH_DURATION + ) + return this } From 44917fd568f5e7feee1c054fa22ba4cb27166353 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Tue, 1 Nov 2016 14:13:44 -0600 Subject: [PATCH 2/3] Relocate/rephrase tooltip follow-through test Also make sure it doesn't leave tooltips on the DOM, which causes subsequent tests to fail --- spec/tooltip-manager-spec.coffee | 50 +++++++++++++++++--------------- 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/spec/tooltip-manager-spec.coffee b/spec/tooltip-manager-spec.coffee index 3d7124d68..a7e8ccb1f 100644 --- a/spec/tooltip-manager-spec.coffee +++ b/spec/tooltip-manager-spec.coffee @@ -1,3 +1,4 @@ +{CompositeDisposable} = require 'atom' TooltipManager = require '../src/tooltip-manager' Tooltip = require '../src/tooltip' _ = require 'underscore-plus' @@ -40,6 +41,32 @@ describe "TooltipManager", -> hover element, -> expect(document.body.querySelector(".tooltip")).toHaveText("Title") + it "displays tooltips immediately when hovering over new elements once a tooltip has been displayed once", -> + disposables = new CompositeDisposable + element1 = createElement('foo') + disposables.add(manager.add element1, title: 'Title') + element2 = createElement('bar') + disposables.add(manager.add element2, title: 'Title') + element3 = createElement('baz') + disposables.add(manager.add element3, title: 'Title') + + hover element1, -> + expect(document.body.querySelector(".tooltip")).toBeNull() + + mouseEnter(element2) + expect(document.body.querySelector(".tooltip")).not.toBeNull() + mouseLeave(element2) + advanceClock(manager.hoverDefaults.delay.hide) + expect(document.body.querySelector(".tooltip")).toBeNull() + + advanceClock(Tooltip.FOLLOW_THROUGH_DURATION) + mouseEnter(element3) + expect(document.body.querySelector(".tooltip")).toBeNull() + advanceClock(manager.hoverDefaults.delay.show) + expect(document.body.querySelector(".tooltip")).not.toBeNull() + + disposables.dispose() + describe "when the trigger is 'manual'", -> it "creates a tooltip immediately and only hides it on dispose", -> disposable = manager.add element, title: "Title", trigger: "manual" @@ -163,26 +190,3 @@ describe "TooltipManager", -> expect(document.body.querySelector(".tooltip")).not.toBeNull() window.dispatchEvent(new CustomEvent('resize')) expect(document.body.querySelector(".tooltip")).toBeNull() - - it "works with follow-through", -> - element1 = createElement('foo') - manager.add element1, title: 'Title' - element2 = createElement('bar') - manager.add element2, title: 'Title' - element3 = createElement('baz') - manager.add element3, title: 'Title' - - hover element1, -> - expect(document.body.querySelector(".tooltip")).toBeNull() - - mouseEnter(element2) - expect(document.body.querySelector(".tooltip")).not.toBeNull() - mouseLeave(element2) - advanceClock(manager.hoverDefaults.delay.hide) - expect(document.body.querySelector(".tooltip")).toBeNull() - - advanceClock(Tooltip.FOLLOW_THROUGH_DURATION) - mouseEnter(element3) - expect(document.body.querySelector(".tooltip")).toBeNull() - advanceClock(manager.hoverDefaults.delay.show) - expect(document.body.querySelector(".tooltip")).not.toBeNull() From 00b6122bae36310565f8154cb5acb253db94f8f1 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Wed, 2 Nov 2016 08:16:51 -0600 Subject: [PATCH 3/3] :arrow_up: about --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 8731dceca..3c78b828d 100644 --- a/package.json +++ b/package.json @@ -81,7 +81,7 @@ "one-light-syntax": "1.6.0", "solarized-dark-syntax": "1.1.1", "solarized-light-syntax": "1.1.1", - "about": "1.7.0", + "about": "1.7.1", "archive-view": "0.62.0", "autocomplete-atom-api": "0.10.0", "autocomplete-css": "0.14.1",