fix(Portal): click inside detection (#2384)

* fix(Portal): click inside detection

* feat(doesNodeContainClick): first check node contains

* test(doesNodeContainClick): add missing spy

src/addons/Portal/Portal.js

@@ -5,6 +5,7 @@ import ReactDOM from 'react-dom'
 
 import {
   AutoControlledComponent as Component,
+  doesNodeContainClick,
   eventStack,
   isBrowser,
   keyboardKey,
@@ -180,11 +181,11 @@ class Portal extends Component {
     if (
       !this.rootNode                                      // not mounted
       || !this.portalNode                                 // no portal
-      || _.invoke(this, 'triggerNode.contains', e.target) // event happened in trigger (delegate to trigger handlers)
-      || _.invoke(this, 'portalNode.contains', e.target)  // event happened in the portal
+      || doesNodeContainClick(this.triggerNode, e)        // event happened in trigger (delegate to trigger handlers)
+      || doesNodeContainClick(this.portalNode, e)         // event happened in the portal
     ) return                                              // ignore the click
 
-    const didClickInRootNode = this.rootNode.contains(e.target)
+    const didClickInRootNode = doesNodeContainClick(this.rootNode, e)
 
     if ((closeOnDocumentClick && !didClickInRootNode) || (closeOnRootNodeClick && didClickInRootNode)) {
       debug('handleDocumentClick()')

src/lib/doesNodeContainClick.js

@@ -0,0 +1,39 @@
+import _ from 'lodash'
+
+/**
+ * Determines if a click's coordinates are within the bounds of a node.
+ *
+ * @see https://github.com/Semantic-Org/Semantic-UI-React/pull/2384
+ *
+ * @param {object} node - A DOM node.
+ * @param {object} e - A SyntheticEvent or DOM Event.
+ * @returns {boolean}
+ */
+const doesNodeContainClick = (node, e) => {
+  if (_.some([e, node], _.isNil)) return false
+
+  // first check if the node contains the e.target, simplest use case
+  if (node.contains(e.target)) return true
+
+  // return early if the event properties aren't available
+  // prevent measuring the node and repainting if we don't need to
+  const { clientX, clientY } = e
+  if (_.some([clientX, clientY], _.isNil)) return false
+
+  // false if the node is not visible
+  const clientRects = node.getClientRects()
+  // Heads Up!
+  // getClientRects returns a DOMRectList, not an array nor a plain object
+  // We explicitly avoid _.isEmpty and check .length to cover all possible shapes
+  if (!node.offsetWidth || !node.offsetHeight || !clientRects || !clientRects.length) return false
+
+  // false if the node doesn't have a valid bounding rect
+  const { top, bottom, left, right } = _.first(clientRects)
+  if (_.some([top, bottom, left, right], _.isNil)) return false
+
+  // we add a small decimal to the upper bound just to make it inclusive
+  // don't add an whole pixel (1) as the event/node values may be decimal sensitive
+  return _.inRange(clientY, top, bottom + 0.001) && _.inRange(clientX, left, right + 0.001)
+}
+
+export default doesNodeContainClick

src/lib/index.js

@@ -33,6 +33,7 @@ export {
 } from './htmlPropsUtils'
 
 export { default as isBrowser } from './isBrowser'
+export { default as doesNodeContainClick } from './doesNodeContainClick'
 export { default as leven } from './leven'
 export * as META from './META'
 export * as SUI from './SUI'

src/modules/Dimmer/Dimmer.js

@@ -6,6 +6,7 @@ import {
   childrenUtils,
   createShorthandFactory,
   customPropTypes,
+  doesNodeContainClick,
   getElementType,
   getUnhandledProps,
   isBrowser,
@@ -91,7 +92,7 @@ export default class Dimmer extends Component {
     const { onClick, onClickOutside } = this.props
 
     if (onClick) onClick(e, this.props)
-    if (this.centerRef && (this.centerRef !== e.target && this.centerRef.contains(e.target))) return
+    if (this.centerRef && (this.centerRef !== e.target && doesNodeContainClick(this.centerRef, e))) return
     if (onClickOutside) onClickOutside(e, this.props)
   }
 

src/modules/Dropdown/Dropdown.js

@@ -7,6 +7,7 @@ import {
   AutoControlledComponent as Component,
   childrenUtils,
   customPropTypes,
+  doesNodeContainClick,
   eventStack,
   getElementType,
   getUnhandledProps,
@@ -643,7 +644,7 @@ export default class Dropdown extends Component {
     if (!this.props.closeOnBlur) return
 
     // If event happened in the dropdown, ignore it
-    if (this.ref && _.isFunction(this.ref.contains) && this.ref.contains(e.target)) return
+    if (this.ref && doesNodeContainClick(this.ref, e)) return
 
     this.close()
   }

test/specs/lib/doesNodeContainClick-test.js

@@ -0,0 +1,170 @@
+import { doesNodeContainClick } from 'src/lib'
+import { sandbox } from '../../utils'
+
+const makeEvent = event => ({ clientX: 0, clientY: 0, ...event })
+
+const makeRect = rect => ({ top: 0, bottom: 1, left: 0, right: 1, ...rect })
+
+const makeNode = (rect, node) => ({
+  contains: sandbox.spy(),
+  offsetWidth: 1,
+  offsetHeight: 1,
+  getClientRects: sandbox.spy(() => ({ length: 1, 0: makeRect(rect) })),
+  ...node,
+})
+
+describe('doesNodeContainClick', () => {
+  describe('nil arguments', () => {
+    it('returns false if the node is nil', () => {
+      doesNodeContainClick(null, makeEvent()).should.equal(false)
+      doesNodeContainClick(undefined, makeEvent()).should.equal(false)
+    })
+
+    it('returns false if the event is nil', () => {
+      doesNodeContainClick(makeNode(), null).should.equal(false)
+      doesNodeContainClick(makeNode(), undefined).should.equal(false)
+    })
+  })
+
+  describe('nil event properties', () => {
+    it('returns false if the e.clientX is nil', () => {
+      doesNodeContainClick(makeNode(), { clientX: null }).should.equal(false)
+      doesNodeContainClick(makeNode(), { clientX: undefined }).should.equal(false)
+    })
+
+    it('returns false if the e.clientY is nil', () => {
+      doesNodeContainClick(makeNode(), { clientY: null }).should.equal(false)
+      doesNodeContainClick(makeNode(), { clientY: undefined }).should.equal(false)
+    })
+
+    it('does not call node.getClientRects if e.clientX is nil', () => {
+      const node = makeNode()
+      doesNodeContainClick(node, { clientX: null })
+      doesNodeContainClick(node, { clientX: undefined })
+
+      node.getClientRects.should.not.have.been.called()
+    })
+
+    it('does not call node.getClientRects if e.clientY is nil', () => {
+      const node = makeNode()
+      doesNodeContainClick(node, { clientY: null })
+      doesNodeContainClick(node, { clientY: undefined })
+
+      node.getClientRects.should.not.have.been.called()
+    })
+  })
+
+  describe('invisible node', () => {
+    it('returns false if there is no node.offsetWidth', () => {
+      const e = makeEvent()
+
+      doesNodeContainClick(makeNode(null, { offsetWidth: 0 }), e).should.equal(false)
+      doesNodeContainClick(makeNode(null, { offsetWidth: null }), e).should.equal(false)
+      doesNodeContainClick(makeNode(null, { offsetWidth: undefined }), e).should.equal(false)
+    })
+    it('returns false if there is no node.offsetHeight', () => {
+      const e = makeEvent()
+
+      doesNodeContainClick(makeNode(null, { offsetHeight: 0 }), e).should.equal(false)
+      doesNodeContainClick(makeNode(null, { offsetHeight: null }), e).should.equal(false)
+      doesNodeContainClick(makeNode(null, { offsetHeight: undefined }), e).should.equal(false)
+    })
+    it('returns false if there is node.getClientRects is empty', () => {
+      const e = makeEvent()
+
+      doesNodeContainClick(makeNode(null, { getClientRects: () => [] }), e).should.equal(false)
+      doesNodeContainClick(makeNode(null, { getClientRects: () => ({ length: 0 }) }), e).should.equal(false)
+      doesNodeContainClick(makeNode(null, { getClientRects: () => ({ length: null }) }), e).should.equal(false)
+      doesNodeContainClick(makeNode(null, { getClientRects: () => ({ length: undefined }) }), e).should.equal(false)
+    })
+  })
+
+  describe('nil node rect properties', () => {
+    it('returns false if the node top is nil', () => {
+      const nullNode = makeNode({ top: null })
+      const undefNode = makeNode({ top: undefined })
+
+      doesNodeContainClick(nullNode, makeEvent()).should.equal(false)
+      doesNodeContainClick(undefNode, makeEvent()).should.equal(false)
+    })
+
+    it('returns false if the node bottom is nil', () => {
+      const nullNode = makeNode({ bottom: null })
+      const undefNode = makeNode({ bottom: undefined })
+
+      doesNodeContainClick(nullNode, makeEvent()).should.equal(false)
+      doesNodeContainClick(undefNode, makeEvent()).should.equal(false)
+    })
+
+    it('returns false if the node left is nil', () => {
+      const nullNode = makeNode({ left: null })
+      const undefNode = makeNode({ left: undefined })
+
+      doesNodeContainClick(nullNode, makeEvent()).should.equal(false)
+      doesNodeContainClick(undefNode, makeEvent()).should.equal(false)
+    })
+
+    it('returns false if the node right is nil', () => {
+      const nullNode = makeNode({ right: null })
+      const undefNode = makeNode({ right: undefined })
+
+      doesNodeContainClick(nullNode, makeEvent()).should.equal(false)
+      doesNodeContainClick(undefNode, makeEvent()).should.equal(false)
+    })
+  })
+
+  describe('click outside node rect', () => {
+    it('returns false if clientY < node top', () => {
+      doesNodeContainClick(makeNode({ top: 1 }), makeEvent({ clientY: 0 })).should.equal(false)
+    })
+    it('returns false if clientY > node bottom', () => {
+      doesNodeContainClick(makeNode({ bottom: 0 }), makeEvent({ clientY: 1 })).should.equal(false)
+    })
+    it('returns false if clientX < node left', () => {
+      doesNodeContainClick(makeNode({ left: 1 }), makeEvent({ clientX: 0 })).should.equal(false)
+    })
+    it('returns false if clientX > node right', () => {
+      doesNodeContainClick(makeNode({ right: 0 }), makeEvent({ clientX: 1 })).should.equal(false)
+    })
+  })
+
+  describe('click inside of node rect', () => {
+    it('returns true if clientY > node top && clientY < node bottom', () => {
+      doesNodeContainClick(makeNode({ top: 1, bottom: 3 }), makeEvent({ clientY: 2 })).should.equal(true)
+    })
+    it('returns true if clientX > node left && clientX < node right', () => {
+      doesNodeContainClick(makeNode({ left: 1, right: 3 }), makeEvent({ clientX: 2 })).should.equal(true)
+    })
+  })
+
+  describe('click on node rect boundary', () => {
+    it('returns true if clientY === node top', () => {
+      doesNodeContainClick(makeNode({ top: 1, bottom: 3 }), makeEvent({ clientY: 1 })).should.equal(true)
+    })
+    it('returns true if clientY === node bottom', () => {
+      doesNodeContainClick(makeNode({ top: 1, bottom: 3 }), makeEvent({ clientY: 3 })).should.equal(true)
+    })
+    it('returns true if clientX === node left', () => {
+      doesNodeContainClick(makeNode({ left: 1, right: 3 }), makeEvent({ clientX: 1 })).should.equal(true)
+    })
+    it('returns true if clientX === node right', () => {
+      doesNodeContainClick(makeNode({ left: 1, right: 3 }), makeEvent({ clientX: 3 })).should.equal(true)
+    })
+  })
+
+  describe('decimal event and node rect values', () => {
+    it('returns true when click is inside node rect', () => {
+      const node = makeNode({ top: 0.1, bottom: 0.9, left: 0.1, right: 0.9 })
+      const event = makeEvent({ clientX: 0.5, clientY: 0.5 })
+
+      doesNodeContainClick(node, event).should.equal(true)
+    })
+
+    it('returns false when click is outside node rect', () => {
+      const node = makeNode({ top: 0.1, bottom: 0.9, left: 0.1, right: 0.9 })
+      const event = makeEvent({ clientX: 1.1, clientY: 1.1 })
+
+      doesNodeContainClick(node, event).should.equal(false)
+    })
+  })
+})