From 435dae1caf2a09e314d961f756223da71ca8e323 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Tue, 9 Apr 2024 15:30:52 -0700 Subject: [PATCH 1/4] Pass back the originating click `event` to actions - this allows consumers to do things like `event.preventDefault()` for buttons with, e.g. React Router links if needed --- src/components/basic_table/action_types.ts | 8 +++--- .../basic_table/default_item_action.test.tsx | 26 +++++++++++++++++++ .../basic_table/default_item_action.tsx | 11 ++++++-- 3 files changed, 40 insertions(+), 5 deletions(-) diff --git a/src/components/basic_table/action_types.ts b/src/components/basic_table/action_types.ts index 1ac557ee5dd..908a0193d27 100644 --- a/src/components/basic_table/action_types.ts +++ b/src/components/basic_table/action_types.ts @@ -6,7 +6,7 @@ * Side Public License, v 1. */ -import { ReactElement, ReactNode } from 'react'; +import { ReactElement, ReactNode, MouseEvent } from 'react'; import { EuiIconType } from '../icon/icon'; import { EuiButtonIconProps } from '../button/button_icon/button_icon'; import { EuiButtonEmptyProps } from '../button/button_empty'; @@ -26,9 +26,11 @@ export interface DefaultItemActionBase { */ description: string | ((item: T) => string); /** - * A handler function to execute the action + * A handler function to execute the action. Passes back the current row + * item as the first argument, and the originating React click event + * as a second argument. */ - onClick?: (item: T) => void; + onClick?: (item: T, event: MouseEvent) => void; href?: string | ((item: T) => string); target?: string; /** diff --git a/src/components/basic_table/default_item_action.test.tsx b/src/components/basic_table/default_item_action.test.tsx index bafd8918835..6514004e1b3 100644 --- a/src/components/basic_table/default_item_action.test.tsx +++ b/src/components/basic_table/default_item_action.test.tsx @@ -122,4 +122,30 @@ describe('DefaultItemAction', () => { await waitForEuiToolTipVisible(); expect(getByText('goodbye tooltip')).toBeInTheDocument(); }); + + it('passes back the original click event as well as the row item to onClick', () => { + const onClick = jest.fn((item, event) => { + event.preventDefault(); + }); + + const action: EmptyButtonAction = { + name: 'onClick', + description: 'test', + onClick, + 'data-test-subj': 'onClick', + }; + const props = { + action, + enabled: true, + item: { id: 'xyz' }, + }; + + const { getByTestSubject } = render(); + + fireEvent.click(getByTestSubject('onClick')); + expect(onClick).toHaveBeenCalledWith( + props.item, + expect.objectContaining({ preventDefault: expect.any(Function) }) + ); + }); }); diff --git a/src/components/basic_table/default_item_action.tsx b/src/components/basic_table/default_item_action.tsx index 667da0589d7..8fbe7571aa8 100644 --- a/src/components/basic_table/default_item_action.tsx +++ b/src/components/basic_table/default_item_action.tsx @@ -6,7 +6,7 @@ * Side Public License, v 1. */ -import React, { ReactElement, ReactNode } from 'react'; +import React, { ReactElement, ReactNode, MouseEvent, useCallback } from 'react'; import { isString } from '../../services/predicate'; import { @@ -42,7 +42,14 @@ export const DefaultItemAction = ({ or 'href' string. If you want to provide a custom action control, make sure to define the 'render' callback`); } - const onClick = action.onClick ? () => action.onClick!(item) : undefined; + const onClick = useCallback( + (event: MouseEvent) => { + if (!action.onClick) return; + event.persist(); // TODO: Remove once React 16 support is dropped + action.onClick!(item, event); + }, + [action.onClick, item] + ); const buttonColor = action.color; let color: EuiButtonIconProps['color'] = 'primary'; From 9eef965fdf834dd64524903085a1492134bdba8f Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Tue, 9 Apr 2024 16:41:04 -0700 Subject: [PATCH 2/4] Pass back the originating click `event` to collapsed actions + allow consumers to prevent the popover from closing if needed via `event.stopPropagation()` --- .../collapsed_item_actions.test.tsx | 42 +++++++++++++++++++ .../basic_table/collapsed_item_actions.tsx | 21 +++++----- 2 files changed, 52 insertions(+), 11 deletions(-) diff --git a/src/components/basic_table/collapsed_item_actions.test.tsx b/src/components/basic_table/collapsed_item_actions.test.tsx index fc5b341b4af..a569da1a586 100644 --- a/src/components/basic_table/collapsed_item_actions.test.tsx +++ b/src/components/basic_table/collapsed_item_actions.test.tsx @@ -84,6 +84,48 @@ describe('CollapsedItemActions', () => { await waitForEuiPopoverClose(); }); + test('default actions - passes back the original click event as well as the row item to onClick', async () => { + const onClick = jest.fn(); + const onClickStopPropagation = jest.fn((item, event) => { + event.stopPropagation(); + }); + + const props = { + actions: [ + { + name: '1', + description: '', + onClick, + 'data-test-subj': 'onClick', + }, + { + name: '2', + description: '', + onClick: onClickStopPropagation, + 'data-test-subj': 'onClickStopPropagation', + }, + ], + itemId: 'id', + item: { id: 'xyz' }, + actionsDisabled: false, + }; + + const { getByTestSubject } = render(); + fireEvent.click(getByTestSubject('euiCollapsedItemActionsButton')); + await waitForEuiPopoverOpen(); + + fireEvent.click(getByTestSubject('onClickStopPropagation')); + expect(onClickStopPropagation).toHaveBeenCalledWith( + props.item, + expect.objectContaining({ stopPropagation: expect.any(Function) }) + ); + // Popover should still be open if propagation was stopped + await waitForEuiPopoverOpen(); + + fireEvent.click(getByTestSubject('onClick')); + await waitForEuiPopoverClose(); + }); + test('custom actions', async () => { const props = { actions: [ diff --git a/src/components/basic_table/collapsed_item_actions.tsx b/src/components/basic_table/collapsed_item_actions.tsx index e4a62b59d56..4a6ecd2737b 100644 --- a/src/components/basic_table/collapsed_item_actions.tsx +++ b/src/components/basic_table/collapsed_item_actions.tsx @@ -45,11 +45,7 @@ export const CollapsedItemActions = ({ className, }: CollapsedItemActionsProps) => { const [popoverOpen, setPopoverOpen] = useState(false); - - const onClickItem = useCallback((onClickAction?: () => void) => { - setPopoverOpen(false); - onClickAction?.(); - }, []); + const closePopover = useCallback(() => setPopoverOpen(false), []); const controls = useMemo(() => { return actions.reduce((controls, action, index) => { @@ -69,7 +65,7 @@ export const CollapsedItemActions = ({ key={index} className="euiBasicTable__collapsedCustomAction" > - onClickItem()}>{actionControl} + {actionControl} ); } else { @@ -97,9 +93,12 @@ export const CollapsedItemActions = ({ target={target} icon={icon} data-test-subj={dataTestSubj} - onClick={() => - onClickItem(onClick ? () => onClick(item) : undefined) - } + onClick={(event) => { + event.persist(); + onClick?.(item, event); + // Allow consumer events to prevent the popover from closing if necessary + if (!event.isPropagationStopped()) closePopover(); + }} toolTipContent={toolTipContent} toolTipProps={{ delay: 'long' }} > @@ -109,7 +108,7 @@ export const CollapsedItemActions = ({ } return controls; }, []); - }, [actions, actionsDisabled, item, onClickItem]); + }, [actions, actionsDisabled, item, closePopover]); const popoverButton = ( ({ id={`${itemId}-actions`} isOpen={popoverOpen} button={withTooltip || popoverButton} - closePopover={() => setPopoverOpen(false)} + closePopover={closePopover} panelPaddingSize="none" anchorPosition="leftCenter" > From c3bd4c1fa46d34ade20b7084dc6b79ce42c5155e Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Tue, 9 Apr 2024 15:37:53 -0700 Subject: [PATCH 3/4] [misc cleanup] Remove manual conditions in favor of `callWithItemIfFunction` util --- .../basic_table/collapsed_item_actions.tsx | 10 +++------ .../basic_table/default_item_action.tsx | 21 +++++++------------ 2 files changed, 10 insertions(+), 21 deletions(-) diff --git a/src/components/basic_table/collapsed_item_actions.tsx b/src/components/basic_table/collapsed_item_actions.tsx index 4a6ecd2737b..8e7e865cce5 100644 --- a/src/components/basic_table/collapsed_item_actions.tsx +++ b/src/components/basic_table/collapsed_item_actions.tsx @@ -14,7 +14,6 @@ import React, { ReactElement, } from 'react'; -import { isString } from '../../services/predicate'; import { EuiContextMenuItem, EuiContextMenuPanel } from '../context_menu'; import { EuiPopover } from '../popover'; import { EuiButtonIcon } from '../button'; @@ -69,12 +68,9 @@ export const CollapsedItemActions = ({ ); } else { - const buttonIcon = action.icon; - let icon; - if (buttonIcon) { - icon = isString(buttonIcon) ? buttonIcon : buttonIcon(item); - } - + const icon = action.icon + ? callWithItemIfFunction(item)(action.icon) + : undefined; const buttonContent = callWithItemIfFunction(item)(action.name); const toolTipContent = callWithItemIfFunction(item)(action.description); const href = callWithItemIfFunction(item)(action.href); diff --git a/src/components/basic_table/default_item_action.tsx b/src/components/basic_table/default_item_action.tsx index 8fbe7571aa8..e026adf6ceb 100644 --- a/src/components/basic_table/default_item_action.tsx +++ b/src/components/basic_table/default_item_action.tsx @@ -8,7 +8,6 @@ import React, { ReactElement, ReactNode, MouseEvent, useCallback } from 'react'; -import { isString } from '../../services/predicate'; import { EuiButtonEmpty, EuiButtonIcon, @@ -51,19 +50,12 @@ export const DefaultItemAction = ({ [action.onClick, item] ); - const buttonColor = action.color; - let color: EuiButtonIconProps['color'] = 'primary'; - if (buttonColor) { - color = isString(buttonColor) ? buttonColor : buttonColor(item); - } - - const buttonIcon = action.icon; - let icon; - if (buttonIcon) { - icon = isString(buttonIcon) ? buttonIcon : buttonIcon(item); - } - - let button; + const color: EuiButtonIconProps['color'] = action.color + ? callWithItemIfFunction(item)(action.color) + : 'primary'; + const icon = action.icon + ? callWithItemIfFunction(item)(action.icon) + : undefined; const actionContent = callWithItemIfFunction(item)(action.name); const tooltipContent = callWithItemIfFunction(item)(action.description); const href = callWithItemIfFunction(item)(action.href); @@ -71,6 +63,7 @@ export const DefaultItemAction = ({ const ariaLabelId = useGeneratedHtmlId(); let ariaLabelledBy: ReactNode; + let button; if (action.type === 'icon') { if (!icon) { From 184d6bf49ad074dd5317c57f8f10cfe9507c7385 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Tue, 9 Apr 2024 16:54:54 -0700 Subject: [PATCH 4/4] changelog --- changelogs/upcoming/7667.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelogs/upcoming/7667.md diff --git a/changelogs/upcoming/7667.md b/changelogs/upcoming/7667.md new file mode 100644 index 00000000000..672b49cecdd --- /dev/null +++ b/changelogs/upcoming/7667.md @@ -0,0 +1 @@ +- Updated `EuiBasicTable` and `EuiInMemoryTable`'s `columns[].actions[]`'s to pass back click events to `onClick` callbacks as the second callback