fix(Popup): call onOpenChange on all user actions

CHANGELOG.md

@@ -25,6 +25,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - Fix `Popup` - do not stop event propagation when pressing Esc on trigger element @sophieH29 ([#750](https://github.com/stardust-ui/react/pull/750))
 - Fix alignment of `Layout`'s `main` area @kuzhelov ([#752](https://github.com/stardust-ui/react/pull/752))
 - Forwarding props for `createShorthand` calls if the value is a React element @mnajdova ([#759](https://github.com/stardust-ui/react/pull/759))
+- Call `Popup` `onOpenChange` on all user initiated events @levithomason ([#619](https://github.com/stardust-ui/react/pull/619))
 
 ### Features
 - Rename `Slot` component to `Box` and export it @Bugaa92 ([#713](https://github.com/stardust-ui/react/pull/713))

docs/src/examples/components/Popup/Types/PopupControlledExample.shorthand.tsx

@@ -1,22 +1,20 @@
 import * as React from 'react'
 import { Button, Input, Popup } from '@stardust-ui/react'
 
-class PopupControlledExample extends React.Component<any, any> {
-  state = { popupOpen: false }
+class PopupControlledExample extends React.Component {
+  state = { open: false }
 
-  togglePopup() {
-    this.setState(prev => ({ popupOpen: !prev.popupOpen }))
+  handleOpenChange = (e, { open }) => {
+    alert(`Popup requested to change its open state to "${open}".`)
+    this.setState({ open })
   }
 
   render() {
     return (
       <Popup
-        open={this.state.popupOpen}
-        onOpenChange={(e, newProps) => {
-          alert(`Popup is requested to change its open state to "${newProps.open}".`)
-          this.setState({ popupOpen: newProps.open })
-        }}
-        trigger={<Button icon="expand" onClick={() => this.togglePopup()} />}
+        open={this.state.open}
+        onOpenChange={this.handleOpenChange}
+        trigger={<Button icon="expand" />}
         content={{ content: <Input icon="search" placeholder="Search..." /> }}
       />
     )

docs/src/examples/components/Popup/Types/PopupControlledExample.tsx

@@ -1,24 +1,22 @@
 import * as React from 'react'
 import { Button, Input, Popup } from '@stardust-ui/react'
 
-class PopupControlledExample extends React.Component<any, any> {
-  state = { popupOpen: false }
+class PopupControlledExample extends React.Component {
+  state = { open: false }
 
-  togglePopup() {
-    this.setState(prev => ({ popupOpen: !prev.popupOpen }))
+  handleOpenChange = (e, { open }) => {
+    alert(`Popup requested to change its open state to "${open}".`)
+    this.setState({ open })
   }
 
   render() {
     return (
       <Popup
-        open={this.state.popupOpen}
-        onOpenChange={(e, newProps) => {
-          alert(`Popup is requested to change its open state to "${newProps.open}".`)
-          this.setState({ popupOpen: newProps.open })
-        }}
+        open={this.state.open}
+        onOpenChange={this.handleOpenChange}
         content={{ content: <Input icon="search" placeholder="Search..." /> }}
       >
-        <Button icon="expand" onClick={() => this.togglePopup()} />
+        <Button icon="expand" />
       </Popup>
     )
   }

src/components/Popup/Popup.tsx

@@ -231,7 +231,7 @@ export default class Popup extends AutoControlledComponent<ReactProps<PopupProps
               this.triggerDomElement && !this.triggerDomElement.contains(e.target)
 
             if (isOutsidePopupElement && isOutsideTriggerElement) {
-              this.state.open && this.trySetOpen(false, e, true)
+              this.state.open && this.trySetOpen(false, e)
             }
           },
           {
@@ -465,10 +465,9 @@ export default class Popup extends AutoControlledComponent<ReactProps<PopupProps
     )
   }
 
-  private trySetOpen(newValue: boolean, eventArgs: any, forceChangeEvent: boolean = false) {
-    if (this.trySetState({ open: newValue }) || forceChangeEvent) {
-      _.invoke(this.props, 'onOpenChange', eventArgs, { ...this.props, ...{ open: newValue } })
-    }
+  private trySetOpen(newValue: boolean, eventArgs: any) {
+    this.trySetState({ open: newValue })
+    _.invoke(this.props, 'onOpenChange', eventArgs, { ...this.props, ...{ open: newValue } })
   }
 
   private setPopupOpen(newOpen, e) {
@@ -486,7 +485,7 @@ export default class Popup extends AutoControlledComponent<ReactProps<PopupProps
 
   private close = (e, onClose?: Function) => {
     if (this.state.open) {
-      this.trySetOpen(false, e, true)
+      this.trySetOpen(false, e)
       onClose && onClose()
     }
   }

src/lib/AutoControlledComponent.tsx

@@ -188,7 +188,7 @@ export default class AutoControlledComponent<P = {}, S = {}> extends UIComponent
    * @param {object} maybeState State that corresponds to controlled props.
    * @param {object} [state] Actual state, useful when you also need to setState.
    */
-  trySetState = (maybeState, state?): boolean => {
+  trySetState = (maybeState, state?) => {
     const { autoControlledProps } = this.constructor as any
     if (process.env.NODE_ENV !== 'production') {
       const { name } = this.constructor
@@ -218,11 +218,6 @@ export default class AutoControlledComponent<P = {}, S = {}> extends UIComponent
 
     if (state) newState = { ...newState, ...state }
 
-    if (Object.keys(newState).length > 0) {
-      this.setState(newState)
-      return true
-    }
-
-    return false
+    if (Object.keys(newState).length > 0) this.setState(newState)
   }
 }

test/specs/components/Popup/Popup-test.tsx

@@ -1,10 +1,14 @@
+import { Placement } from 'popper.js'
+import * as React from 'react'
+
 import {
   getPopupPlacement,
   applyRtlToOffset,
   Position,
   Alignment,
 } from 'src/components/Popup/positioningHelper'
-import { Placement } from 'popper.js'
+import Popup from 'src/components/Popup/Popup'
+import { mountWithProvider } from '../../../utils'
 
 type PositionTestInput = {
   align: Alignment
@@ -102,4 +106,29 @@ describe('Popup', () => {
       expect(yOffsetTransformed.trim()).toBe(yOffset)
     })
   })
+
+  describe('onOpenChange', () => {
+    test('is called on click', () => {
+      const spy = jest.fn()
+
+      mountWithProvider(<Popup trigger={<button />} content="Hi" onOpenChange={spy} />)
+        .find('button')
+        .simulate('click')
+
+      expect(spy).toHaveBeenCalledTimes(1)
+      expect(spy.mock.calls[0][1]).toMatchObject({ open: true })
+    })
+
+    // https://github.com/stardust-ui/react/pull/619
+    test('is called on click when controlled', () => {
+      const spy = jest.fn()
+
+      mountWithProvider(<Popup open={false} trigger={<button />} content="Hi" onOpenChange={spy} />)
+        .find('button')
+        .simulate('click')
+
+      expect(spy).toHaveBeenCalledTimes(1)
+      expect(spy.mock.calls[0][1]).toMatchObject({ open: true })
+    })
+  })
 })