From a44778b4ba83b54004cbd259adc965f03c7b5872 Mon Sep 17 00:00:00 2001 From: John Fellman Date: Thu, 21 Dec 2017 15:56:09 -0800 Subject: [PATCH 1/2] improve hover behavior, simplify interface, and improve readme --- README.md | 15 ++++++------ addon/components/frost-popover.js | 40 ++++++++++++++++++++----------- 2 files changed, 34 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index 40e3623..edb2974 100644 --- a/README.md +++ b/README.md @@ -46,24 +46,25 @@ ember install ember-frost-popover | Interface | Attributes | Value | Description | | ----------| ---------- | ----- | ----------- | | Action | `close` | | Close the popover and optionally fire an external action | -| Option | `offset` | | The amount in pixels the popover should appear from the target (defaults to `10`) | -| Option | `position` | `top`,`right`,`bottom`,`left`, `auto`| The location of the popover relative to the target. When `auto` is specified, it will dynamically reorient the popover. For example, if position is `auto left`, the popover will display to the left when possible, otherwise it will display right. (defaults to `bottom`) | | Option | `closest` | boolean | When true uses JQuery's [closest function](https://api.jquery.com/closest/). Otherwise just uses main selector `$()` (defaults to `false`). | -| Option | `excludePadding` | boolean | When true removes the padding from position calculations (defaults to `false`).| +| Option | `delay` | number | Delay the open of the popover if provided, unit in ms.| | Option | `event` | | The event that will trigger the popover (defaults to on `click`). Uses [on()](http://api.jquery.com/on/)| +| Option | `excludePadding` | boolean | When true removes the padding from position calculations (defaults to `false`).| | Option | `handlerIn` | | The event that will open the popover (replaces the `event` when `handlerOut` is also set). Uses [on()](http://api.jquery.com/on/)| | Option | `handlerOut` | | The event that will close the popover (replaces the `event` when `handlerIn` is also set). Uses [on()](http://api.jquery.com/on/)| -| Option | `target` | | The selector string of the target that activates the popover | -| Option | `viewport`| | The selector for the viewport. Defaults to 'body' | +| Option | `hideDelay` | number | Delay the close of the popover if provided, unit in ms.| +| Option | `offset` | | The amount in pixels the popover should appear from the target (defaults to `10`) | +| Option | `position` | `top`,`right`,`bottom`,`left`, `auto`| The location of the popover relative to the target. When `auto` is specified, it will dynamically reorient the popover. For example, if position is `auto left`, the popover will display to the left when possible, otherwise it will display right. (defaults to `bottom`) | | Option | `resize` | | If set to false, will prevent the browser from resizing at the edges of the viewport. This preserves the *expand to fit content* behavior of `width: auto`. It defaults to true. | | Option | `stopPropagation` | | If set to true event handlers will call `event.stopPropagation()` | -| Option | `delay` | number | Delay the open of the popover if provided, unit in ms.| +| Option | `target` | | The selector string of the target that activates the popover | +| Option | `viewport`| | The selector for the viewport. Defaults to 'body' | ## Specifying Target If the `frost-popover` component is placed next to the `target`, be careful to use a selector that will uniquely identify the `target`. If it is nested inside the `target`, you can set `closest` to true which will search the -nearest ancestor from the `popover`. +nearest ancestor from the `popover` - which is far more performant than a full dom traversal. ### A Note On Positioning diff --git a/addon/components/frost-popover.js b/addon/components/frost-popover.js index ee293b5..11c0d5a 100644 --- a/addon/components/frost-popover.js +++ b/addon/components/frost-popover.js @@ -1,11 +1,11 @@ import Ember from 'ember' -const {$, Component, get, isPresent, run, typeOf} = Ember import {task, timeout} from 'ember-concurrency' import PropTypeMixin, {PropTypes} from 'ember-prop-types' import layout from '../templates/components/frost-popover' import {checkBottom, checkLeft, checkRight, checkTop} from './util' +const {$, Component, isPresent, run, typeOf} = Ember const arrowMargin = 5 const maxPlacementRetries = 5 @@ -22,7 +22,6 @@ export default Component.extend(PropTypeMixin, { excludePadding: PropTypes.bool, handlerIn: PropTypes.string, handlerOut: PropTypes.string, - includeContentInEvents: PropTypes.bool, // making this true lets the content also inheret events properly index: PropTypes.number, offset: PropTypes.number, onDisplay: PropTypes.func, @@ -43,7 +42,6 @@ export default Component.extend(PropTypeMixin, { event: 'click', excludePadding: false, index: 0, - includeContentInEvents: false, offset: 10, position: 'bottom', resize: true, @@ -66,10 +64,17 @@ export default Component.extend(PropTypeMixin, { didInsertElement () { const target = this.getTarget() + const delay = this.get('delay') const event = this.get('event') - const handlerIn = this.get('handlerIn') - const handlerOut = this.get('handlerOut') + const hideDelay = this.get('hideDelay') + const popover = this.get('element') const stopPropagation = this.get('stopPropagation') + + let handlerIn = this.get('handlerIn') + let handlerOut = this.get('handlerOut') + + if (event.split(' ').length === 2) { [handlerIn, handlerOut] = event.split(' ') } + if (handlerIn && handlerOut) { this._eventHandlerIn = (event) => { if (stopPropagation) { @@ -82,7 +87,6 @@ export default Component.extend(PropTypeMixin, { } this.cancelShowDelayTask() if (!this.get('visible')) { - const delay = this.get('delay') if (delay) { this.showDelay(event, delay) } else { @@ -102,7 +106,6 @@ export default Component.extend(PropTypeMixin, { } this.cancelShowDelayTask() if (this.get('visible')) { - const hideDelay = this.get('hideDelay') if (hideDelay) { this.showDelay(event, hideDelay) } else { @@ -124,8 +127,6 @@ export default Component.extend(PropTypeMixin, { return } this.cancelShowDelayTask() - const delay = this.get('delay') - const hideDelay = this.get('hideDelay') if (delay || hideDelay) { let delayToUse = this.get('visible') ? hideDelay : delay @@ -142,6 +143,21 @@ export default Component.extend(PropTypeMixin, { $(target).on(event, this._eventHandler) } + + // handle mouse events on visible popover + $(popover).on('mouseenter', event => { + if (this.get('visible')) this.cancelShowDelayTask() + }) + $(popover).on('mouseleave', event => { + if (this.get('visible')) { + if (hideDelay) { + this.showDelay(event, hideDelay) + } else { + this.togglePopover(event) + } + } + }) + $(popover).on('click', event => event.stopPropagation()) }, willDestroyElement () { @@ -164,11 +180,7 @@ export default Component.extend(PropTypeMixin, { * @param {DOMEvent} event - click event */ togglePopover (event) { - const popover = this.get('element') - if ($(event.target).closest(popover).length === 0 || - (get(this, 'includeContentInEvents') === true && event.target === popover)) { - this.send('togglePopover', event) - } + this.send('togglePopover', event) }, /** From 92b7f48eb8da33f84a07971c44f355038e504b52 Mon Sep 17 00:00:00 2001 From: John Fellman Date: Tue, 2 Jan 2018 13:32:32 -0800 Subject: [PATCH 2/2] fix click handling, remove hover listeners, add/cleanup tests --- README.md | 7 +- addon/components/frost-popover.js | 65 ++++++++++++++----- tests/dummy/app/pods/demo/template.hbs | 6 +- .../components/frost-popover-test.js | 48 ++++++++++++++ 4 files changed, 107 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index edb2974..18d7822 100644 --- a/README.md +++ b/README.md @@ -64,7 +64,12 @@ ember install ember-frost-popover If the `frost-popover` component is placed next to the `target`, be careful to use a selector that will uniquely identify the `target`. If it is nested inside the `target`, you can set `closest` to true which will search the -nearest ancestor from the `popover` - which is far more performant than a full dom traversal. +nearest ancestor from the `popover`. + +### Hover Behavior + +The `popover` will by default maintain its visible state when hovered. +If the events are `mouseenter` and `mouseleave`, adding a `hideDelay` makes hovering over the popover much easier for the user. ### A Note On Positioning diff --git a/addon/components/frost-popover.js b/addon/components/frost-popover.js index 11c0d5a..ff8f8d1 100644 --- a/addon/components/frost-popover.js +++ b/addon/components/frost-popover.js @@ -73,7 +73,10 @@ export default Component.extend(PropTypeMixin, { let handlerIn = this.get('handlerIn') let handlerOut = this.get('handlerOut') - if (event.split(' ').length === 2) { [handlerIn, handlerOut] = event.split(' ') } + if (event && event.split(' ').length === 2) { + [handlerIn, handlerOut] = event.split(' ') + this.setProperties({handlerIn, handlerOut}) + } if (handlerIn && handlerOut) { this._eventHandlerIn = (event) => { @@ -144,20 +147,35 @@ export default Component.extend(PropTypeMixin, { $(target).on(event, this._eventHandler) } - // handle mouse events on visible popover - $(popover).on('mouseenter', event => { - if (this.get('visible')) this.cancelShowDelayTask() - }) - $(popover).on('mouseleave', event => { - if (this.get('visible')) { - if (hideDelay) { - this.showDelay(event, hideDelay) - } else { - this.togglePopover(event) + // add handlers for persisting visible state when hovering + if (handlerIn === 'mouseenter' && handlerOut === 'mouseleave') { + // functions declared here for scope + this._hoverHandlerIn = event => { + if (this.get('visible')) { + this.cancelShowDelayTask() } } - }) - $(popover).on('click', event => event.stopPropagation()) + this._hoverHandlerOut = event => { + const hideDelay = this.get('hideDelay') + if (this.get('visible')) { + if (hideDelay) { + this.showDelay(event, hideDelay) + } else { + this.togglePopover(event) + } + } + } + this._hoverClickHandler = event => { + if (this.get('stopPropagation')) { + event.stopPropagation() + } + } + + // handle mouse events on visible popover + $(popover).on(handlerIn, this._hoverHandlerIn) + $(popover).on(handlerOut, this._hoverHandlerOut) + $(popover).on('click', this._hoverClickHandler) + } }, willDestroyElement () { @@ -165,22 +183,39 @@ export default Component.extend(PropTypeMixin, { const event = this.get('event') const handlerIn = this.get('handlerIn') const handlerOut = this.get('handlerOut') + const popover = this.get('element') + if (handlerIn && handlerOut) { $(target).off(handlerIn, this._eventHandlerIn) $(target).off(handlerOut, this._eventHandlerOut) } else { $(target).off(event, this._eventHandler) } + this.cancelShowDelayTask() this.unregisterClickOff() + + // remove listeners attached for hover behavior + if (handlerIn === 'mouseenter' && handlerOut === 'mouseleave') { + $(popover).off(handlerIn, this._hoverHandlerIn) + $(popover).off(handlerOut, this._hoverHandlerOut) + $(popover).off('click', this._hoverClickHandler) + } }, /** * Toggles the popover - * @param {DOMEvent} event - click event + * @param {DOMEvent} event - mouse event */ togglePopover (event) { - this.send('togglePopover', event) + const handlerIn = this.get('handlerIn') + const handlerOut = this.get('handlerOut') + const popover = this.get('element') + + if ($(event.target).closest(popover).length === 0 || + (handlerIn === 'mouseenter' && handlerOut === 'mouseleave')) { + this.send('togglePopover', event) + } }, /** diff --git a/tests/dummy/app/pods/demo/template.hbs b/tests/dummy/app/pods/demo/template.hbs index 44a3f1f..a87a33b 100644 --- a/tests/dummy/app/pods/demo/template.hbs +++ b/tests/dummy/app/pods/demo/template.hbs @@ -46,7 +46,7 @@

Activating Event

{{#frost-button hook='mouseEnterButton' size='small' priority='primary' class='event-mouse' text='Mouseenter'}} - {{#frost-popover target='.event-mouse' closest=true handlerIn='mouseenter' handlerOut='mouseleave'}} + {{#frost-popover target='.event-mouse' closest=true handlerIn='mouseenter' handlerOut='mouseleave' hideDelay=100}} Tooltip is toggled on mouse enter and mouse leave {{/frost-popover}} {{/frost-button}} @@ -66,7 +66,7 @@ {{/frost-button}} {{#frost-button hook='mouseEnterDelayButton' size='small' priority='primary' class='child' text='Hover me!'}} - {{#frost-popover target='.child' delay=500 handlerIn='mouseenter' handlerOut='mouseleave' excludePadding=true closest=true includeContentInEvents=true}} + {{#frost-popover target='.child' delay=500 handlerIn='mouseenter' handlerOut='mouseleave' excludePadding=true closest=true}} Hover me delayed! {{/frost-popover}} {{/frost-button}} @@ -74,7 +74,7 @@

Hide Delay display

This feature doesn't properly work with 'click' at the moment.

{{#frost-button hook='mouseEnterHideDelayButton' size='small' priority='primary' class='child' text='Hover me!'}} - {{#frost-popover target='.child' hideDelay=500 handlerIn='mouseenter' handlerOut='mouseleave' excludePadding=true closest=true includeContentInEvents=true}} + {{#frost-popover target='.child' hideDelay=500 handlerIn='mouseenter' handlerOut='mouseleave' excludePadding=true closest=true}} Hover me hide delayed! {{/frost-popover}} {{/frost-button}} diff --git a/tests/integration/components/frost-popover-test.js b/tests/integration/components/frost-popover-test.js index 0e81fb2..6bb7862 100644 --- a/tests/integration/components/frost-popover-test.js +++ b/tests/integration/components/frost-popover-test.js @@ -198,6 +198,54 @@ describe(test.label, function () { }) }) + describe('when hovering with mouseenter/mouseleave for handlerIn/handlerOut', function () { + this.timeout(5000) + + beforeEach(function () { + this.render(hbs` +
+ hover test +
+ {{#frost-popover target='#foo' hideDelay=500 event='mouseenter mouseleave'}} + Inside + {{/frost-popover}} + `) + + return wait() + .then(() => { + $('#foo').mouseenter() + return wait() + }) + .then(() => { + $('#foo').mouseleave() + + run.later(function () { + $('.tooltip-frost-popover').mouseenter() + }, 100) + + return wait() + }) + }) + + it('should still be visible 700ms later if mouseenter has been triggered on popover', function (done) { + run.later(function () { + expect($('.visible')).to.have.length(1) + done() + }, 700) + }) + + it('should dismiss after mouseleave after hovering', function (done) { + run.later(function () { + $('.tooltip-frost-popover').mouseleave() + }, 700) + + run.later(function () { + expect($('.visible')).to.have.length(0) + done() + }, 1300) + }) + }) + it('should constrain to the viewport', function (done) { this.render(hbs`