Merge pull request #18044 from atom/tt-18-sept-tooltip-bug

Fix bug with keyboard input in click tooltips

spec/tooltip-manager-spec.js

@@ -55,6 +55,18 @@ describe('TooltipManager', () => {
 
         disposables.dispose()
       })
+
+      it('hides the tooltip on keydown events', () => {
+        const disposable = manager.add(element, { title: 'Title', trigger: 'hover' })
+        hover(element, function () {
+          expect(document.body.querySelector('.tooltip')).not.toBeNull()
+          window.dispatchEvent(new CustomEvent('keydown', {
+            bubbles: true
+          }))
+          expect(document.body.querySelector('.tooltip')).toBeNull()
+          disposable.dispose()
+        })
+      })
     })
 
     describe("when the trigger is 'manual'", () =>
@@ -93,6 +105,19 @@ describe('TooltipManager', () => {
       })
     )
 
+    it('does not hide the tooltip on keyboard input', () => {
+      manager.add(element, {title: 'Title', trigger: 'click'})
+      element.click()
+      expect(document.body.querySelector('.tooltip')).not.toBeNull()
+      window.dispatchEvent(new CustomEvent('keydown', {
+        bubbles: true
+      }))
+      expect(document.body.querySelector('.tooltip')).not.toBeNull()
+      // click again to hide the tooltip because otherwise state leaks
+      // into other tests.
+      element.click()
+    })
+
     it('allows a custom item to be specified for the content of the tooltip', () => {
       const tooltipElement = document.createElement('div')
       manager.add(element, {item: {element: tooltipElement}})
@@ -213,20 +238,6 @@ describe('TooltipManager', () => {
       })
     )
 
-    describe('when a user types', () =>
-      it('hides the tooltips', () => {
-        const disposable = manager.add(element, { title: 'Title' })
-        hover(element, function () {
-          expect(document.body.querySelector('.tooltip')).not.toBeNull()
-          window.dispatchEvent(new CustomEvent('keydown', {
-            bubbles: true
-          }))
-          expect(document.body.querySelector('.tooltip')).toBeNull()
-          disposable.dispose()
-        })
-      })
-    )
-
     describe('findTooltips', () => {
       it('adds and remove tooltips correctly', () => {
         expect(manager.findTooltips(element).length).toBe(0)

src/tooltip-manager.js

@@ -152,12 +152,12 @@ class TooltipManager {
       tooltip.hide()
     }
 
+    // note: adding a listener here adds a new listener for every tooltip element that's registered.  Adding unnecessary listeners is bad for performance.  It would be better to add/remove listeners when tooltips are actually created in the dom.
     window.addEventListener('resize', hideTooltip)
-    window.addEventListener('keydown', hideTooltip, { capture: true })
 
     const disposable = new Disposable(() => {
       window.removeEventListener('resize', hideTooltip)
-      window.removeEventListener('keydown', hideTooltip, { capture: true })
+
       hideTooltip()
       tooltip.destroy()
 

src/tooltip.js

@@ -82,6 +82,7 @@ Tooltip.prototype.init = function (element, options) {
       var eventIn, eventOut
 
       if (trigger === 'hover') {
+        this.hideOnKeydownOutsideOfTooltip = () => this.hide()
         if (this.options.selector) {
           eventIn = 'mouseover'
           eventOut = 'mouseout'
@@ -220,6 +221,10 @@ Tooltip.prototype.show = function () {
       window.addEventListener('click', this.hideOnClickOutsideOfTooltip, true)
     }
 
+    if (this.hideOnKeydownOutsideOfTooltip) {
+      window.addEventListener('keydown', this.hideOnKeydownOutsideOfTooltip, true)
+    }
+
     var tip = this.getTooltipElement()
     this.startObservingMutations()
     var tipId = this.getUID('tooltip')
@@ -359,6 +364,10 @@ Tooltip.prototype.hide = function (callback) {
     window.removeEventListener('click', this.hideOnClickOutsideOfTooltip, true)
   }
 
+  if (this.hideOnKeydownOutsideOfTooltip) {
+    window.removeEventListener('keydown', this.hideOnKeydownOutsideOfTooltip, true)
+  }
+
   this.tip && this.tip.classList.remove('in')
   this.stopObservingMutations()