From e9200e5bc04a0945215caf7cd02ddb68111454d1 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Fri, 7 Oct 2016 17:02:44 -0600 Subject: [PATCH 1/7] WIP: Add ability to use custom elements inside tooltips --- src/tooltip.js | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/tooltip.js b/src/tooltip.js index ad5ce0cdd..d1558d0be 100644 --- a/src/tooltip.js +++ b/src/tooltip.js @@ -294,13 +294,21 @@ Tooltip.prototype.replaceArrow = function (delta, dimension, isVertical) { Tooltip.prototype.setContent = function () { var tip = this.getTooltipElement() - var title = this.getTitle() + + if (this.options.tooltipClass) { + tip.classList.add(this.options.tooltipClass) + } var inner = tip.querySelector('.tooltip-inner') - if (this.options.html) { - inner.innerHTML = title + if (this.options.tooltipElement) { + inner.appendChild(this.options.tooltipElement) } else { - inner.textContent = title + var title = this.getTitle() + if (this.options.html) { + inner.innerHTML = title + } else { + inner.textContent = title + } } tip.classList.remove('fade', 'in', 'top', 'bottom', 'left', 'right') @@ -328,7 +336,7 @@ Tooltip.prototype.fixTitle = function () { } Tooltip.prototype.hasContent = function () { - return this.getTitle() + return this.getTitle() || this.options.tooltipElement } Tooltip.prototype.getCalculatedOffset = function (placement, pos, actualWidth, actualHeight) { From 253917f0071160d282a6c1e41e32d406bd96a5ab Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Mon, 10 Oct 2016 08:17:44 -0600 Subject: [PATCH 2/7] Make tooltip accept an item option instead of tooltipElement ...and use view registry to resolve it to a view when showing the tooltip. Signed-off-by: Antonio Scandurra --- spec/tooltip-manager-spec.coffee | 10 ++++++++-- src/tooltip-manager.coffee | 4 ++-- src/tooltip.js | 9 +++++---- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/spec/tooltip-manager-spec.coffee b/spec/tooltip-manager-spec.coffee index d4bfc1bd6..4649479f5 100644 --- a/spec/tooltip-manager-spec.coffee +++ b/spec/tooltip-manager-spec.coffee @@ -8,7 +8,7 @@ describe "TooltipManager", -> ctrlY = _.humanizeKeystroke("ctrl-y") beforeEach -> - manager = new TooltipManager(keymapManager: atom.keymaps) + manager = new TooltipManager(keymapManager: atom.keymaps, viewRegistry: atom.views) element = document.createElement('div') element.classList.add('foo') jasmine.attachToDOM(element) @@ -23,7 +23,7 @@ describe "TooltipManager", -> advanceClock(manager.defaults.delay.hide) describe "::add(target, options)", -> - it "creates a tooltip based on the given options when hovering over the target element", -> + it "creates a tooltip when hovering over the target element if no trigger is specified", -> manager.add element, title: "Title" hover element, -> expect(document.body.querySelector(".tooltip")).toHaveText("Title") @@ -34,6 +34,12 @@ describe "TooltipManager", -> disposable.dispose() expect(document.body.querySelector(".tooltip")).toBeNull() + it "allows a custom item to be specified for the content of the tooltip", -> + tooltipElement = document.createElement('div') + manager.add element, item: {element: tooltipElement} + hover element, -> + expect(tooltipElement.closest(".tooltip")).not.toBeNull() + it "allows jQuery elements to be passed as the target", -> element2 = document.createElement('div') jasmine.attachToDOM(element2) diff --git a/src/tooltip-manager.coffee b/src/tooltip-manager.coffee index 90f0ab8e6..dbb0b7c44 100644 --- a/src/tooltip-manager.coffee +++ b/src/tooltip-manager.coffee @@ -54,7 +54,7 @@ class TooltipManager placement: 'auto top' viewportPadding: 2 - constructor: ({@keymapManager}) -> + constructor: ({@keymapManager, @viewRegistry}) -> # Essential: Add a tooltip to the given element. # @@ -92,7 +92,7 @@ class TooltipManager else if keystroke? options.title = getKeystroke(bindings) - tooltip = new Tooltip(target, _.defaults(options, @defaults)) + tooltip = new Tooltip(target, _.defaults(options, @defaults), @viewRegistry) hideTooltip = -> tooltip.leave(currentTarget: target) diff --git a/src/tooltip.js b/src/tooltip.js index d1558d0be..eba47ad3b 100644 --- a/src/tooltip.js +++ b/src/tooltip.js @@ -7,13 +7,14 @@ 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 Tooltip = function (element, options) { +var Tooltip = function (element, options, viewRegistry) { this.options = null this.enabled = null this.timeout = null this.hoverState = null this.element = null this.inState = null + this.viewRegistry = viewRegistry this.init(element, options) } @@ -300,8 +301,8 @@ Tooltip.prototype.setContent = function () { } var inner = tip.querySelector('.tooltip-inner') - if (this.options.tooltipElement) { - inner.appendChild(this.options.tooltipElement) + if (this.options.item) { + inner.appendChild(this.viewRegistry.getView(this.options.item)) } else { var title = this.getTitle() if (this.options.html) { @@ -336,7 +337,7 @@ Tooltip.prototype.fixTitle = function () { } Tooltip.prototype.hasContent = function () { - return this.getTitle() || this.options.tooltipElement + return this.getTitle() || this.options.item } Tooltip.prototype.getCalculatedOffset = function (placement, pos, actualWidth, actualHeight) { From 718cc017e60eccb282947f5d77e8e0e8be7973be Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Mon, 10 Oct 2016 10:52:42 -0600 Subject: [PATCH 3/7] Hide click-triggered tooltips when clicking anywhere outside of tooltip Signed-off-by: Antonio Scandurra --- spec/tooltip-manager-spec.coffee | 49 +++++++++++++++++++++++++------- src/atom-environment.coffee | 2 +- src/tooltip-manager.coffee | 13 ++++++--- src/tooltip.js | 21 ++++++++++++-- 4 files changed, 67 insertions(+), 18 deletions(-) diff --git a/spec/tooltip-manager-spec.coffee b/spec/tooltip-manager-spec.coffee index 4649479f5..6c15b0608 100644 --- a/spec/tooltip-manager-spec.coffee +++ b/spec/tooltip-manager-spec.coffee @@ -16,23 +16,50 @@ describe "TooltipManager", -> hover = (element, fn) -> element.dispatchEvent(new CustomEvent('mouseenter', bubbles: false)) element.dispatchEvent(new CustomEvent('mouseover', bubbles: true)) - advanceClock(manager.defaults.delay.show) + advanceClock(manager.hoverDefaults.delay.show) fn() element.dispatchEvent(new CustomEvent('mouseleave', bubbles: false)) element.dispatchEvent(new CustomEvent('mouseout', bubbles: true)) - advanceClock(manager.defaults.delay.hide) + advanceClock(manager.hoverDefaults.delay.hide) describe "::add(target, options)", -> - it "creates a tooltip when hovering over the target element if no trigger is specified", -> - manager.add element, title: "Title" - hover element, -> - expect(document.body.querySelector(".tooltip")).toHaveText("Title") + describe "when the trigger is 'hover' (the default)", -> + it "creates a tooltip when hovering over the target element", -> + manager.add element, title: "Title" + hover element, -> + expect(document.body.querySelector(".tooltip")).toHaveText("Title") - it "creates a tooltip immediately if the trigger type is manual", -> - disposable = manager.add element, title: "Title", trigger: "manual" - expect(document.body.querySelector(".tooltip")).toHaveText("Title") - disposable.dispose() - expect(document.body.querySelector(".tooltip")).toBeNull() + 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" + expect(document.body.querySelector(".tooltip")).toHaveText("Title") + disposable.dispose() + expect(document.body.querySelector(".tooltip")).toBeNull() + + describe "when the trigger is 'click'", -> + it "shows and hides the tooltip when the target element is clicked", -> + disposable = manager.add element, title: "Title", trigger: "click" + expect(document.body.querySelector(".tooltip")).toBeNull() + element.click() + expect(document.body.querySelector(".tooltip")).not.toBeNull() + element.click() + expect(document.body.querySelector(".tooltip")).toBeNull() + + # Hide the tooltip when clicking anywhere but inside the tooltip element + element.click() + expect(document.body.querySelector(".tooltip")).not.toBeNull() + document.body.querySelector(".tooltip").click() + expect(document.body.querySelector(".tooltip")).not.toBeNull() + document.body.querySelector(".tooltip").firstChild.click() + expect(document.body.querySelector(".tooltip")).not.toBeNull() + document.body.click() + expect(document.body.querySelector(".tooltip")).toBeNull() + + # Tooltip can show again after hiding due to clicking outside of the tooltip + element.click() + expect(document.body.querySelector(".tooltip")).not.toBeNull() + element.click() + expect(document.body.querySelector(".tooltip")).toBeNull() it "allows a custom item to be specified for the content of the tooltip", -> tooltipElement = document.createElement('div') diff --git a/src/atom-environment.coffee b/src/atom-environment.coffee index 92a8d1ad3..ca6a342f4 100644 --- a/src/atom-environment.coffee +++ b/src/atom-environment.coffee @@ -153,7 +153,7 @@ class AtomEnvironment extends Model @keymaps = new KeymapManager({@configDirPath, resourcePath, notificationManager: @notifications}) - @tooltips = new TooltipManager(keymapManager: @keymaps) + @tooltips = new TooltipManager(keymapManager: @keymaps, viewRegistry: @views) @commands = new CommandRegistry @commands.attach(@window) diff --git a/src/tooltip-manager.coffee b/src/tooltip-manager.coffee index dbb0b7c44..7226ca9ee 100644 --- a/src/tooltip-manager.coffee +++ b/src/tooltip-manager.coffee @@ -46,14 +46,15 @@ Tooltip = null module.exports = class TooltipManager defaults: - delay: - show: 1000 - hide: 100 + trigger: 'hover' container: 'body' html: true placement: 'auto top' viewportPadding: 2 + hoverDefaults: + {delay: {show: 1000, hide: 100}} + constructor: ({@keymapManager, @viewRegistry}) -> # Essential: Add a tooltip to the given element. @@ -92,7 +93,11 @@ class TooltipManager else if keystroke? options.title = getKeystroke(bindings) - tooltip = new Tooltip(target, _.defaults(options, @defaults), @viewRegistry) + options = _.defaults(options, @defaults) + if options.trigger is 'hover' + options = _.defaults(options, @hoverDefaults) + + tooltip = new Tooltip(target, options, @viewRegistry) hideTooltip = -> tooltip.leave(currentTarget: target) diff --git a/src/tooltip.js b/src/tooltip.js index eba47ad3b..1baede496 100644 --- a/src/tooltip.js +++ b/src/tooltip.js @@ -65,6 +65,14 @@ Tooltip.prototype.init = function (element, options) { if (trigger === 'click') { this.disposables.add(listen(this.element, 'click', this.options.selector, this.toggle.bind(this))) + this.hideOnClickOutsideOfTooltip = (event) => { + const tooltipElement = this.getTooltipElement() + if (tooltipElement === event.target) return + if (tooltipElement.contains(event.target)) return + if (this.element === event.target) return + if (this.element.contains(event.target)) return + this.hide() + } } else if (trigger === 'manual') { this.show() } else { @@ -183,8 +191,11 @@ Tooltip.prototype.leave = function (event) { Tooltip.prototype.show = function () { if (this.hasContent() && this.enabled) { - var tip = this.getTooltipElement() + if (this.hideOnClickOutsideOfTooltip) { + window.addEventListener('click', this.hideOnClickOutsideOfTooltip, true) + } + var tip = this.getTooltipElement() var tipId = this.getUID('tooltip') this.setContent() @@ -316,6 +327,12 @@ Tooltip.prototype.setContent = function () { } Tooltip.prototype.hide = function (callback) { + this.inState = {} + + if (this.hideOnClickOutsideOfTooltip) { + window.removeEventListener('click', this.hideOnClickOutsideOfTooltip, true) + } + this.tip && this.tip.classList.remove('in') if (this.hoverState !== 'in') this.tip && this.tip.remove() @@ -445,7 +462,7 @@ Tooltip.prototype.destroy = function () { Tooltip.prototype.getDelegateComponent = function (element) { var component = tooltipComponentsByElement.get(element) if (!component) { - component = new Tooltip(element, this.getDelegateOptions()) + component = new Tooltip(element, this.getDelegateOptions(), this.viewRegistry) tooltipComponentsByElement.set(element, component) } return component From e71e1f4ed132b0b4424286baa5a2aa8b97b3a44a Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Mon, 10 Oct 2016 10:55:19 -0600 Subject: [PATCH 4/7] Test custom class option --- spec/tooltip-manager-spec.coffee | 6 ++++++ src/tooltip.js | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/spec/tooltip-manager-spec.coffee b/spec/tooltip-manager-spec.coffee index 6c15b0608..3783c904d 100644 --- a/spec/tooltip-manager-spec.coffee +++ b/spec/tooltip-manager-spec.coffee @@ -67,6 +67,12 @@ describe "TooltipManager", -> hover element, -> expect(tooltipElement.closest(".tooltip")).not.toBeNull() + it "allows a custom class to be specified for the tooltip", -> + tooltipElement = document.createElement('div') + manager.add element, title: 'Title', class: 'custom-tooltip-class' + hover element, -> + expect(document.body.querySelector(".tooltip").classList.contains('custom-tooltip-class')).toBe(true) + it "allows jQuery elements to be passed as the target", -> element2 = document.createElement('div') jasmine.attachToDOM(element2) diff --git a/src/tooltip.js b/src/tooltip.js index 1baede496..f0f9d1a3f 100644 --- a/src/tooltip.js +++ b/src/tooltip.js @@ -307,8 +307,8 @@ Tooltip.prototype.replaceArrow = function (delta, dimension, isVertical) { Tooltip.prototype.setContent = function () { var tip = this.getTooltipElement() - if (this.options.tooltipClass) { - tip.classList.add(this.options.tooltipClass) + if (this.options.class) { + tip.classList.add(this.options.class) } var inner = tip.querySelector('.tooltip-inner') From 0c7fdea695852b9bcdf1d69fdb869f5ac21a65c7 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Mon, 10 Oct 2016 14:00:40 -0600 Subject: [PATCH 5/7] Document tooltip API directly instead of referring to Bootstrap docs I want to eliminate the selector option and open the door to diverging from the lesser used parts of that API in the future by only documenting a subset. --- src/tooltip-manager.coffee | 41 ++++++++++++++++++++++++++++++++------ 1 file changed, 35 insertions(+), 6 deletions(-) diff --git a/src/tooltip-manager.coffee b/src/tooltip-manager.coffee index 7226ca9ee..fc89ebff4 100644 --- a/src/tooltip-manager.coffee +++ b/src/tooltip-manager.coffee @@ -2,7 +2,7 @@ _ = require 'underscore-plus' {Disposable, CompositeDisposable} = require 'event-kit' Tooltip = null -# Essential: Associates tooltips with HTML elements or selectors. +# Essential: Associates tooltips with HTML elements. # # You can get the `TooltipManager` via `atom.tooltips`. # @@ -60,12 +60,41 @@ class TooltipManager # Essential: Add a tooltip to the given element. # # * `target` An `HTMLElement` - # * `options` See http://getbootstrap.com/javascript/#tooltips-options for a - # full list of options. You can also supply the following additional options: + # * `options` An object with one or more of the following options: # * `title` A {String} or {Function} to use for the text in the tip. If - # given a function, `this` will be set to the `target` element. - # * `trigger` A {String} that's the same as Bootstrap 'click | hover | focus - # | manual', except 'manual' will show the tooltip immediately. + # a function is passed, `this` will be set to the `target` element. This + # option is mutually exclusive with the `item` option. + # * `html` A {Boolean} affecting the interpetation of the `title` option. + # If `true` (the default), the `title` string will be interpreted as HTML. + # Otherwise it will be interpreted as plain text. + # * `item` A view (object with an `.element` property) or a DOM element + # containing custom content for the tooltip. This option is mutually + # exclusive with the `title` option. + # * `class` A {String} with a class to apply to the tooltip element to + # enable custom styling. + # * `placement` A {String} or {Function} returning a string to indicate + # the position of the tooltip relative to `element`. Can be `'top'`, + # `'bottom'`, `'left'`, `'right'`, or `'auto'`. When `'auto'` is + # specified, it will dynamically reorient the tooltip. For example, if + # placement is `'auto left'`, the tooltip will display to the left when + # possible, otherwise it will display right. + # When a function is used to determine the placement, it is called with + # the tooltip DOM node as its first argument and the triggering element + # DOM node as its second. The `this` context is set to the tooltip + # instance. + # * `trigger` A {String} indicating how the tooltip should be displayed. + # Choose from one of the following options: + # * `'hover'` Show the tooltip when the mouse hovers over the element. + # This is the default. + # * `'click'` Show the tooltip when the element is clicked. The tooltip + # will be hidden after clicking the element again or anywhere else + # outside of the tooltip itself. + # * `'focus'` Show the tooltip when the element is focused. + # * `'manual'` Show the tooltip immediately and only hide it when the + # returned disposable is disposed. + # * `delay` An object specifying the show and hide delay in milliseconds. + # Defaults to `{show: 1000, hide: 100}` if the `trigger` is `hover` and + # otherwise defaults to `0` for both values. # * `keyBindingCommand` A {String} containing a command name. If you specify # this option and a key binding exists that matches the command, it will # be appended to the title or rendered alone if no title is specified. From 72c5fcad8222ce5a67cbcc7da3adb65cc586b419 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Mon, 10 Oct 2016 14:03:08 -0600 Subject: [PATCH 6/7] Eliminate selector option in tooltip manager API No packages use it currently, and it's really complex to support so we should kill it while we have the chance. When it comes time to rewrite the tooltip code or add features, not worrying about selectors will make it easier. --- spec/tooltip-manager-spec.coffee | 14 -------------- src/tooltip-manager.coffee | 1 + 2 files changed, 1 insertion(+), 14 deletions(-) diff --git a/spec/tooltip-manager-spec.coffee b/spec/tooltip-manager-spec.coffee index 3783c904d..6bebd6e76 100644 --- a/spec/tooltip-manager-spec.coffee +++ b/spec/tooltip-manager-spec.coffee @@ -91,20 +91,6 @@ describe "TooltipManager", -> hover element, -> expect(document.body.querySelector(".tooltip")).toBeNull() hover element2, -> expect(document.body.querySelector(".tooltip")).toBeNull() - describe "when a selector is specified", -> - it "creates a tooltip when hovering over a descendant of the target that matches the selector", -> - child = document.createElement('div') - child.classList.add('bar') - grandchild = document.createElement('div') - element.appendChild(child) - child.appendChild(grandchild) - - manager.add element, selector: '.bar', title: 'Bar' - - hover grandchild, -> - expect(document.body.querySelector('.tooltip')).toHaveText('Bar') - expect(document.body.querySelector('.tooltip')).toBeNull() - describe "when a keyBindingCommand is specified", -> describe "when a title is specified", -> it "appends the key binding corresponding to the command to the title", -> diff --git a/src/tooltip-manager.coffee b/src/tooltip-manager.coffee index fc89ebff4..4419ec740 100644 --- a/src/tooltip-manager.coffee +++ b/src/tooltip-manager.coffee @@ -122,6 +122,7 @@ class TooltipManager else if keystroke? options.title = getKeystroke(bindings) + delete options.selector options = _.defaults(options, @defaults) if options.trigger is 'hover' options = _.defaults(options, @hoverDefaults) From f1f600dbab44a7c8c3a1a2ba8dbca36cc724de7a Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Mon, 10 Oct 2016 16:24:55 -0600 Subject: [PATCH 7/7] :arrow_up: status-bar --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index e555e6244..d9a76c9be 100644 --- a/package.json +++ b/package.json @@ -112,7 +112,7 @@ "settings-view": "0.243.0", "snippets": "1.0.3", "spell-check": "0.68.3", - "status-bar": "1.4.1", + "status-bar": "1.4.2", "styleguide": "0.47.2", "symbols-view": "0.113.1", "tabs": "0.102.1",