diff --git a/CHANGELOG.md b/CHANGELOG.md index c42546069a..6067f95e43 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/packages/react/src/components/Dropdown/Dropdown.tsx b/packages/react/src/components/Dropdown/Dropdown.tsx index 38e2306d4a..ff3d1030c5 100644 --- a/packages/react/src/components/Dropdown/Dropdown.tsx +++ b/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, 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, 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, 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, Dropdo item: ShorthandValue, index: number, getItemProps: (options: GetItemPropsOptions) => 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, Dropdo }) private handleSearchInputOverrides = ( - predefinedProps: DropdownSearchInputProps, highlightedIndex: number, rtl: boolean, selectItemAtIndex: ( @@ -758,7 +762,7 @@ class Dropdown extends AutoControlledComponent, 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, Dropdo this.removeItemFromValue(item) this.tryFocusSearchInput() this.tryFocusTriggerButton() - e.stopPropagation() _.invoke(predefinedProps, 'onRemove', e, DropdownSelectedItemProps) } diff --git a/packages/react/test/specs/components/Dropdown/Dropdown-test.tsx b/packages/react/test/specs/components/Dropdown/Dropdown-test.tsx index c47b725dba..70d5cb99f3 100644 --- a/packages/react/test/specs/components/Dropdown/Dropdown-test.tsx +++ b/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() + 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() + 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) + }) + }) })