Merge pull request #68 from gkchestertron/improve-popover-hover

improve hover behavior, simplify interface, and improve readme
ciena-frost · Jan 2, 2018 · e78e22d · e78e22d
2 parents c4fa545 + 92b7f48
commit e78e22d
Show file tree

Hide file tree

Showing 4 changed files with 121 additions and 20 deletions.
diff --git a/README.md b/README.md
@@ -46,25 +46,31 @@ 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 `$(<target>)` (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`.
 
+### 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
 
 The `popover` is displayed using `absolute` positioning. The `target`'s coordinates are determined from the `offsets`

diff --git 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,20 @@ 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 && event.split(' ').length === 2) {
+      [handlerIn, handlerOut] = event.split(' ')
+      this.setProperties({handlerIn, handlerOut})
+    }
+
     if (handlerIn && handlerOut) {
       this._eventHandlerIn = (event) => {
         if (stopPropagation) {
@@ -82,7 +90,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 +109,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 +130,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,31 +146,74 @@ export default Component.extend(PropTypeMixin, {
 
       $(target).on(event, this._eventHandler)
     }
+
+    // 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()
+        }
+      }
+      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 () {
     const target = this.getTarget()
     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) {
+    const handlerIn = this.get('handlerIn')
+    const handlerOut = this.get('handlerOut')
     const popover = this.get('element')
+
     if ($(event.target).closest(popover).length === 0 ||
-      (get(this, 'includeContentInEvents') === true && event.target === popover)) {
+        (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
@@ -46,7 +46,7 @@
 
 <h2>Activating Event</h2>
 {{#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}}
     <span class='inside'>Tooltip is toggled on mouse enter and mouse leave</span>
   {{/frost-popover}}
 {{/frost-button}}
@@ -66,15 +66,15 @@
 {{/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}}
     <span class='inside'>Hover me delayed!</span>
   {{/frost-popover}}
 {{/frost-button}}
 
 <h2>Hide Delay display</h2>
 This feature doesn't properly work with 'click' at the moment.<br><br>
 {{#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}}
     <span class='inside'>Hover me hide delayed!</span>
   {{/frost-popover}}
 {{/frost-button}}

diff --git 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`
+        <div id='foo' class='target'>
+          hover test
+        </div>
+        {{#frost-popover target='#foo' hideDelay=500 event='mouseenter mouseleave'}}
+          <span class='inside'>Inside</span>
+        {{/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`
       <div id='viewport' style='width: 400px; height: 400px;'>