fix(Dropdown): add onClick override on DropdownItem (#1248)

* added override capability to DropdownItem

* removed the mouse listeners

* updated changelog

* added code review changes

* added unit tests

* updated changelog again

* Update packages/react/test/specs/components/Dropdown/Dropdown-test.tsx

Co-Authored-By: silviuavram <silviu.avram91@gmail.com>

* added another assertion to the tests

* updated changelog

* stop immediate propagation from native event

* fixed tests by adding mock fn

* fixed import issue

* fixed a couple more tests

Author: silviuaavram

CHANGELOG.md

@@ -21,6 +21,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - Rename `inputFocusBorderBottomColor` to `inputFocusBorderColor` in `InputVariables` @layershifter ([#1247](https://github.com/stardust-ui/react/pull/1247))
 
 ### Fixes
+- Fix onClick in `DropdownItem` to accept user callback and have its event propagation stopped @silviuavram ([#1248](https://github.com/stardust-ui/react/pull/1248))
 - Fix a11y message cleanup for add and remove items in `Dropdown` @silviuavram ([#1237](https://github.com/stardust-ui/react/pull/1237))
 
 ### Features

packages/react/src/components/Dropdown/Dropdown.tsx

@@ -32,7 +32,7 @@ import {
 } from '../../lib'
 import List from '../List/List'
 import Ref from '../Ref/Ref'
-import DropdownItem from './DropdownItem'
+import DropdownItem, { DropdownItemProps } from './DropdownItem'
 import DropdownSelectedItem, { DropdownSelectedItemProps } from './DropdownSelectedItem'
 import DropdownSearchInput, { DropdownSearchInputProps } from './DropdownSearchInput'
 import Button from '../Button/Button'
@@ -512,16 +512,14 @@ class Dropdown extends AutoControlledComponent<Extendable<DropdownProps>, Dropdo
         variables,
         inputRef: this.inputRef,
       },
-      overrideProps: (predefinedProps: DropdownSearchInputProps) =>
-        this.handleSearchInputOverrides(
-          predefinedProps,
-          highlightedIndex,
-          rtl,
-          selectItemAtIndex,
-          toggleMenu,
-          accessibilityComboboxProps,
-          getInputProps,
-        ),
+      overrideProps: this.handleSearchInputOverrides(
+        highlightedIndex,
+        rtl,
+        selectItemAtIndex,
+        toggleMenu,
+        accessibilityComboboxProps,
+        getInputProps,
+      ),
     })
   }
 
@@ -598,7 +596,7 @@ class Dropdown extends AutoControlledComponent<Extendable<DropdownProps>, Dropdo
               key: (item as any).header,
             }),
         },
-        overrideProps: () => this.handleItemOverrides(item, index, getItemProps),
+        overrideProps: this.handleItemOverrides(item, index, getItemProps),
         render: renderItem,
       }),
     )
@@ -640,8 +638,7 @@ class Dropdown extends AutoControlledComponent<Extendable<DropdownProps>, Dropdo
               key: (item as any).header,
             }),
         },
-        overrideProps: (predefinedProps: DropdownSelectedItemProps) =>
-          this.handleSelectedItemOverrides(predefinedProps, item, rtl),
+        overrideProps: this.handleSelectedItemOverrides(item, rtl),
         render: renderSelectedItem,
       }),
     )
@@ -724,12 +721,20 @@ class Dropdown extends AutoControlledComponent<Extendable<DropdownProps>, Dropdo
     item: ShorthandValue,
     index: number,
     getItemProps: (options: GetItemPropsOptions<ShorthandValue>) => any,
-  ) => ({ accessibilityItemProps: getItemProps({ item, index }) })
+  ) => (predefinedProps: DropdownItemProps) => ({
+    accessibilityItemProps: getItemProps({
+      item,
+      index,
+      onClick: e => {
+        e.stopPropagation()
+        e.nativeEvent.stopImmediatePropagation()
+        _.invoke(predefinedProps, 'onClick', e, predefinedProps)
+      },
+    }),
+  })
 
-  private handleSelectedItemOverrides = (
+  private handleSelectedItemOverrides = (item: ShorthandValue, rtl: boolean) => (
     predefinedProps: DropdownSelectedItemProps,
-    item: ShorthandValue,
-    rtl: boolean,
   ) => ({
     onRemove: (e: React.SyntheticEvent, dropdownSelectedItemProps: DropdownSelectedItemProps) => {
       this.handleSelectedItemRemove(e, item, predefinedProps, dropdownSelectedItemProps)
@@ -747,7 +752,6 @@ class Dropdown extends AutoControlledComponent<Extendable<DropdownProps>, Dropdo
   })
 
   private handleSearchInputOverrides = (
-    predefinedProps: DropdownSearchInputProps,
     highlightedIndex: number,
     rtl: boolean,
     selectItemAtIndex: (
@@ -758,7 +762,7 @@ class Dropdown extends AutoControlledComponent<Extendable<DropdownProps>, Dropdo
     toggleMenu: () => void,
     accessibilityComboboxProps: Object,
     getInputProps: (options?: GetInputPropsOptions) => any,
-  ) => {
+  ) => (predefinedProps: DropdownSearchInputProps) => {
     const handleInputBlur = (
       e: React.SyntheticEvent,
       searchInputProps: DropdownSearchInputProps,
@@ -1027,7 +1031,6 @@ class Dropdown extends AutoControlledComponent<Extendable<DropdownProps>, Dropdo
     this.removeItemFromValue(item)
     this.tryFocusSearchInput()
     this.tryFocusTriggerButton()
-    e.stopPropagation()
     _.invoke(predefinedProps, 'onRemove', e, DropdownSelectedItemProps)
   }
 

packages/react/test/specs/components/Dropdown/Dropdown-test.tsx

@@ -104,7 +104,7 @@ describe('Dropdown', () => {
 
       triggerButton.simulate('click')
       const firstItem = wrapper.find(`li.${Dropdown.slotClassNames.item}`).at(0)
-      firstItem.simulate('click')
+      firstItem.simulate('click', { nativeEvent: { stopImmediatePropagation: jest.fn() } })
 
       expect(onOpenChange).toBeCalledTimes(2)
       expect(onOpenChange).toHaveBeenLastCalledWith(
@@ -630,7 +630,7 @@ describe('Dropdown', () => {
 
       triggerButton.simulate('click')
       const item = wrapper.find(`li.${Dropdown.slotClassNames.item}`).at(itemSelectedIndex)
-      item.simulate('click')
+      item.simulate('click', { nativeEvent: { stopImmediatePropagation: jest.fn() } })
 
       expect(onSelectedChange).toHaveBeenCalledTimes(1)
       expect(onSelectedChange).toHaveBeenCalledWith(
@@ -716,10 +716,10 @@ describe('Dropdown', () => {
 
       triggerButton.simulate('click')
       const firstItem = wrapper.find(`li.${Dropdown.slotClassNames.item}`).at(1)
-      firstItem.simulate('click')
+      firstItem.simulate('click', { nativeEvent: { stopImmediatePropagation: jest.fn() } })
       triggerButton.simulate('click')
       const itemAtIndex = wrapper.find(`li.${Dropdown.slotClassNames.item}`).at(itemSelectedIndex)
-      itemAtIndex.simulate('click')
+      itemAtIndex.simulate('click', { nativeEvent: { stopImmediatePropagation: jest.fn() } })
 
       expect(onSelectedChange).toHaveBeenCalledTimes(2)
       expect(onSelectedChange).toHaveBeenLastCalledWith(
@@ -739,10 +739,10 @@ describe('Dropdown', () => {
 
       triggerButton.simulate('click')
       const itemAtIndex1 = wrapper.find(`li.${Dropdown.slotClassNames.item}`).at(1)
-      itemAtIndex1.simulate('click')
+      itemAtIndex1.simulate('click', { nativeEvent: { stopImmediatePropagation: jest.fn() } })
       triggerButton.simulate('click')
       const itemAtIndex2 = wrapper.find(`li.${Dropdown.slotClassNames.item}`).at(3)
-      itemAtIndex2.simulate('click')
+      itemAtIndex2.simulate('click', { nativeEvent: { stopImmediatePropagation: jest.fn() } })
 
       expect(onSelectedChange).toHaveBeenCalledTimes(2)
       expect(onSelectedChange).toHaveBeenLastCalledWith(
@@ -763,10 +763,10 @@ describe('Dropdown', () => {
 
       toggleIndicator.simulate('click')
       let firstItem = wrapper.find(`li.${Dropdown.slotClassNames.item}`).at(0)
-      firstItem.simulate('click')
+      firstItem.simulate('click', { nativeEvent: { stopImmediatePropagation: jest.fn() } })
       toggleIndicator.simulate('click')
       firstItem = wrapper.find(`li.${Dropdown.slotClassNames.item}`).at(0)
-      firstItem.simulate('click')
+      firstItem.simulate('click', { nativeEvent: { stopImmediatePropagation: jest.fn() } })
       searchInput
         .simulate('click')
         .simulate('keydown', { keyCode: keyboardKey.Backspace, key: 'Backspace' })
@@ -808,7 +808,7 @@ describe('Dropdown', () => {
 
       triggerButton.simulate('click')
       const firstItem = wrapper.find(`li.${Dropdown.slotClassNames.item}`).at(0)
-      firstItem.simulate('click')
+      firstItem.simulate('click', { nativeEvent: { stopImmediatePropagation: jest.fn() } })
 
       expect(dropdown.state('a11ySelectionStatus')).toBe('bla bla added')
 
@@ -830,7 +830,7 @@ describe('Dropdown', () => {
 
       triggerButton.simulate('click')
       const firstItem = wrapper.find(`li.${Dropdown.slotClassNames.item}`).at(0)
-      firstItem.simulate('click')
+      firstItem.simulate('click', { nativeEvent: { stopImmediatePropagation: jest.fn() } })
       jest.runAllTimers()
       const removeIcon = wrapper.find(`span.${DropdownSelectedItem.slotClassNames.icon}`)
       removeIcon.simulate('click')
@@ -866,7 +866,7 @@ describe('Dropdown', () => {
 
       toggleIndicator.simulate('click')
       const itemAtIndex = wrapper.find(`li.${Dropdown.slotClassNames.item}`).at(itemSelectedIndex)
-      itemAtIndex.simulate('click')
+      itemAtIndex.simulate('click', { nativeEvent: { stopImmediatePropagation: jest.fn() } })
 
       expect(onSelectedChange).toHaveBeenCalledTimes(1)
       expect(onSelectedChange).toHaveBeenCalledWith(
@@ -909,7 +909,7 @@ describe('Dropdown', () => {
 
       toggleIndicator.simulate('click')
       const itemAtIndex = wrapper.find(`li.${Dropdown.slotClassNames.item}`).at(itemSelectedIndex)
-      itemAtIndex.simulate('click')
+      itemAtIndex.simulate('click', { nativeEvent: { stopImmediatePropagation: jest.fn() } })
 
       expect(onSelectedChange).toHaveBeenCalledTimes(1)
       expect(onSelectedChange).toHaveBeenCalledWith(
@@ -1010,4 +1010,53 @@ describe('Dropdown', () => {
       expect(preventDefault).not.toBeCalled()
     })
   })
+
+  describe('items', () => {
+    it('have onClick called when passed stop event from being propagated', () => {
+      const onClick = jest.fn()
+      const stopPropagation = jest.fn()
+      const stopImmediatePropagation = jest.fn()
+      const mockedEvent = { stopPropagation, nativeEvent: { stopImmediatePropagation } }
+      const items = [{ header: 'Venom', onClick }]
+      const wrapper = mountWithProvider(<Dropdown items={items} />)
+      const triggerButton = wrapper.find(`button.${Dropdown.slotClassNames.triggerButton}`)
+
+      triggerButton.simulate('click')
+      const firstItem = wrapper.find(`li.${Dropdown.slotClassNames.item}`)
+      firstItem.simulate('click', mockedEvent)
+
+      expect(onClick).toBeCalledTimes(1)
+      expect(onClick).toHaveBeenCalledWith(
+        expect.objectContaining(mockedEvent),
+        expect.objectContaining({
+          header: 'Venom',
+        }),
+      )
+      expect(stopPropagation).toBeCalledTimes(1)
+      expect(stopImmediatePropagation).toBeCalledTimes(1)
+    })
+
+    it('when selected have onClick called when passed stop event from being propagated', () => {
+      const onClick = jest.fn()
+      const stopPropagation = jest.fn()
+      const stopImmediatePropagation = jest.fn()
+      const mockedEvent = { stopPropagation, nativeEvent: { stopImmediatePropagation } }
+      const items = [{ header: 'Venom', onClick }]
+      const wrapper = mountWithProvider(<Dropdown items={items} value={items} multiple />)
+      const selectedItemHeaderAtIndex0 = wrapper
+        .find(`span.${DropdownSelectedItem.slotClassNames.header}`)
+        .at(0)
+
+      selectedItemHeaderAtIndex0.simulate('click', mockedEvent)
+
+      expect(onClick).toBeCalledTimes(1)
+      expect(onClick).toHaveBeenCalledWith(
+        expect.objectContaining(mockedEvent),
+        expect.objectContaining({
+          header: 'Venom',
+        }),
+      )
+      expect(stopPropagation).toBeCalledTimes(1)
+    })
+  })
 })