Skip to content

Commit b7f100c

Browse files
kraenhansenkraenhansen
authored
chore(compass-components): refactor ItemActionControls #2 (#6560)
* Clean up components, using flex gap * Fix type warnings for onClick handlers * Clean up "Actions" generic types * Drive-by removal of a comment * Remove unneeded memoization
1 parent d6348eb commit b7f100c

File tree

12 files changed

+82
-130
lines changed

12 files changed

+82
-130
lines changed

‎packages/compass-components/src/components/actions/dropdown-menu-button.tsx‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,15 +49,19 @@ export function DropdownMenuButton<Action extends string>({
4949
const menuTriggerRef = useRef<HTMLButtonElement | null>(null);
5050
const [isMenuOpen, setIsMenuOpen] = useState(false);
5151

52-
const onClick = useCallback(
52+
const onClick: React.MouseEventHandler<HTMLElement> = useCallback(
5353
(evt) => {
5454
evt.stopPropagation();
5555
if (evt.currentTarget.dataset.menuitem) {
5656
setIsMenuOpen(false);
5757
// Workaround for https://jira.mongodb.org/browse/PD-1674
5858
menuTriggerRef.current?.focus();
5959
}
60-
onAction(evt.currentTarget.dataset.action);
60+
const actionName = evt.currentTarget.dataset.action;
61+
if (typeof actionName !== 'string') {
62+
throw new Error('Expected element to have a "data-action" attribute');
63+
}
64+
onAction(actionName as Action);
6165
},
6266
[onAction]
6367
);
@@ -113,7 +117,7 @@ export function DropdownMenuButton<Action extends string>({
113117
<MenuItem
114118
active={activeAction === action}
115119
key={action}
116-
data-testid={actionTestId<Action>(dataTestId, action)}
120+
data-testid={actionTestId(dataTestId, action)}
117121
data-action={action}
118122
data-menuitem={true}
119123
glyph={<ActionGlyph glyph={icon} size={iconSize} />}

‎packages/compass-components/src/components/actions/item-action-button.tsx‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,10 @@
11
import React from 'react';
2-
import { css, cx } from '@leafygreen-ui/emotion';
3-
import { spacing } from '@leafygreen-ui/tokens';
2+
import { cx } from '@leafygreen-ui/emotion';
43

54
import { ItemActionButtonSize } from './constants';
65
import type { ItemComponentProps } from './types';
76
import { SmallIconButton } from './small-icon-button';
87

9-
// TODO: Move to a parent component - or a flex gap
10-
const buttonStyle = css({
11-
'&:not(:first-child)': {
12-
marginLeft: spacing[100],
13-
},
14-
});
15-
168
export function ItemActionButton<Action extends string>({
179
action,
1810
icon = <></>,
@@ -36,7 +28,7 @@ export function ItemActionButton<Action extends string>({
3628
data-action={action}
3729
data-testid={dataTestId}
3830
onClick={onClick}
39-
className={cx(buttonStyle, iconClassName, className)}
31+
className={cx(iconClassName, className)}
4032
style={iconStyle}
4133
disabled={isDisabled}
4234
/>

‎packages/compass-components/src/components/actions/item-action-controls.spec.tsx‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ describe('item action controls components', function () {
2020
actions={[]}
2121
onAction={() => {}}
2222
data-testid="test-actions"
23-
></ItemActionControls>
23+
/>
2424
);
2525

2626
expect(screen.queryByTestId('test-actions')).to.not.exist;
@@ -32,7 +32,7 @@ describe('item action controls components', function () {
3232
actions={[{ action: 'copy', label: 'Copy', icon: 'Copy' }]}
3333
onAction={() => {}}
3434
data-testid="test-actions"
35-
></ItemActionControls>
35+
/>
3636
);
3737

3838
expect(screen.getByTestId('test-actions')).to.exist;
@@ -47,7 +47,7 @@ describe('item action controls components', function () {
4747
onAction={() => {}}
4848
data-testid="test-actions"
4949
collapseToMenuThreshold={1}
50-
></ItemActionControls>
50+
/>
5151
);
5252

5353
const trigger = screen.getByTestId('test-actions-show-actions');
@@ -69,7 +69,7 @@ describe('item action controls components', function () {
6969
onAction={onAction}
7070
data-testid="test-actions"
7171
collapseToMenuThreshold={3}
72-
></ItemActionControls>
72+
/>
7373
);
7474

7575
expect(onAction).not.to.be.called;
@@ -90,7 +90,7 @@ describe('item action controls components', function () {
9090
onAction={onAction}
9191
data-testid="test-actions"
9292
collapseToMenuThreshold={1}
93-
></ItemActionControls>
93+
/>
9494
);
9595

9696
expect(onAction).not.to.be.called;
@@ -112,7 +112,7 @@ describe('item action controls components', function () {
112112
onAction={() => {}}
113113
data-testid="test-actions"
114114
collapseAfter={1}
115-
></ItemActionControls>
115+
/>
116116
);
117117

118118
expect(screen.queryByTestId('test-actions-connect-action')).to.exist;
@@ -146,7 +146,7 @@ describe('item action controls components', function () {
146146
onAction={() => {}}
147147
data-testid="test-actions"
148148
collapseAfter={1}
149-
></ItemActionControls>
149+
/>
150150
);
151151

152152
const actionButton = screen.getByTestId('test-actions-connect-action');

‎packages/compass-components/src/components/actions/item-action-controls.tsx‎

Lines changed: 21 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import React, { useMemo } from 'react';
1+
import React from 'react';
22
import { spacing } from '@leafygreen-ui/tokens';
33
import { css, cx } from '@leafygreen-ui/emotion';
44
import type { RenderMode } from '@leafygreen-ui/popover';
@@ -13,13 +13,7 @@ const actionControlsStyle = css({
1313
marginLeft: 'auto',
1414
alignItems: 'center',
1515
display: 'flex',
16-
});
17-
18-
// Action buttons are rendered 4px apart from each other. With this we keep the
19-
// same spacing also when action buttons are rendered alongside action menu
20-
// (happens when collapseAfter prop is specified)
21-
const actionMenuWithActionControlsStyles = css({
22-
marginLeft: spacing[100],
16+
gap: spacing[100],
2317
});
2418

2519
export type ItemActionControlsProps<Action extends string> = {
@@ -53,33 +47,20 @@ export function ItemActionControls<Action extends string>({
5347
collapseToMenuThreshold = 2,
5448
'data-testid': dataTestId,
5549
}: ItemActionControlsProps<Action>) {
56-
const sharedProps = useMemo(
57-
() => ({
58-
isVisible,
59-
onAction,
60-
className: cx('item-action-controls', className),
61-
iconClassName,
62-
iconStyle,
63-
iconSize,
64-
'data-testid': dataTestId,
65-
}),
66-
[
67-
isVisible,
68-
onAction,
69-
className,
70-
iconClassName,
71-
iconStyle,
72-
iconSize,
73-
dataTestId,
74-
]
75-
);
76-
const sharedMenuProps = useMemo(
77-
() => ({
78-
menuClassName,
79-
renderMode,
80-
}),
81-
[menuClassName, renderMode]
82-
);
50+
const sharedProps = {
51+
isVisible,
52+
onAction,
53+
className: cx('item-action-controls', className),
54+
iconClassName,
55+
iconStyle,
56+
iconSize,
57+
'data-testid': dataTestId,
58+
};
59+
const sharedMenuProps = {
60+
menuClassName,
61+
renderMode,
62+
};
63+
8364
if (actions.length === 0) {
8465
return null;
8566
}
@@ -90,19 +71,12 @@ export function ItemActionControls<Action extends string>({
9071
const collapsedActions = actions.slice(collapseAfter);
9172
return (
9273
<div className={actionControlsStyle}>
93-
<ItemActionGroup
94-
actions={visibleActions}
95-
{...sharedProps}
96-
></ItemActionGroup>
74+
<ItemActionGroup {...sharedProps} actions={visibleActions} />
9775
<ItemActionMenu
98-
actions={collapsedActions}
9976
{...sharedProps}
10077
{...sharedMenuProps}
101-
className={cx(
102-
actionMenuWithActionControlsStyles,
103-
sharedProps.className
104-
)}
105-
></ItemActionMenu>
78+
actions={collapsedActions}
79+
/>
10680
</div>
10781
);
10882
}
@@ -111,13 +85,9 @@ export function ItemActionControls<Action extends string>({
11185

11286
if (shouldShowMenu) {
11387
return (
114-
<ItemActionMenu
115-
actions={actions}
116-
{...sharedProps}
117-
{...sharedMenuProps}
118-
></ItemActionMenu>
88+
<ItemActionMenu actions={actions} {...sharedProps} {...sharedMenuProps} />
11989
);
12090
}
12191

122-
return <ItemActionGroup actions={actions} {...sharedProps}></ItemActionGroup>;
92+
return <ItemActionGroup actions={actions} {...sharedProps} />;
12393
}

‎packages/compass-components/src/components/actions/item-action-group.tsx‎

Lines changed: 11 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,7 @@ const containerStyle = css({
1919
marginLeft: 'auto',
2020
alignItems: 'center',
2121
display: 'flex',
22-
});
23-
24-
// TODO: Move to a parent component - or a flex gap
25-
const actionGroupButtonStyle = css({
26-
'&:not(:first-child)': {
27-
marginLeft: spacing[100],
28-
},
22+
gap: spacing[100],
2923
});
3024

3125
export type ItemActionGroupProps<Action extends string> = {
@@ -49,10 +43,14 @@ export function ItemActionGroup<Action extends string>({
4943
isVisible = true,
5044
'data-testid': dataTestId,
5145
}: ItemActionGroupProps<Action>) {
52-
const onClick = useCallback(
46+
const onClick: React.MouseEventHandler<HTMLElement> = useCallback(
5347
(evt) => {
5448
evt.stopPropagation();
55-
onAction(evt.currentTarget.dataset.action);
49+
const actionName = evt.currentTarget.dataset.action;
50+
if (typeof actionName !== 'string') {
51+
throw new Error('Expected element to have a "data-action" attribute');
52+
}
53+
onAction(actionName as Action);
5654
},
5755
[onAction]
5856
);
@@ -85,7 +83,7 @@ export function ItemActionGroup<Action extends string>({
8583
iconStyle={iconStyle}
8684
iconClassName={iconClassName}
8785
onClick={onClick}
88-
data-testid={actionTestId<Action>(dataTestId, itemProps.action)}
86+
data-testid={actionTestId(dataTestId, itemProps.action)}
8987
/>
9088
);
9189

@@ -94,14 +92,9 @@ export function ItemActionGroup<Action extends string>({
9492
<Tooltip
9593
key={itemProps.action}
9694
{...tooltipProps}
97-
trigger={
98-
<div
99-
className={actionGroupButtonStyle}
100-
style={{ display: 'inherit' }}
101-
>
102-
{item}
103-
</div>
104-
}
95+
// Wrapping the item in a div, because the `trigger` must accept and render `children`
96+
// See docs for the prop for more information
97+
trigger={<div style={{ display: 'inherit' }}>{item}</div>}
10598
>
10699
{tooltip}
107100
</Tooltip>

‎packages/compass-components/src/components/actions/item-action-menu.tsx‎

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import React, { useCallback, useRef, useState } from 'react';
22
import { css, cx } from '@leafygreen-ui/emotion';
3-
import { spacing } from '@leafygreen-ui/tokens';
43
import type { RenderMode } from '@leafygreen-ui/popover';
54

65
import { Menu, MenuItem, MenuSeparator } from '../leafygreen';
@@ -31,13 +30,6 @@ const containerStyle = css({
3130
display: 'flex',
3231
});
3332

34-
// TODO: Move to a parent component - or a flex gap
35-
const buttonStyle = css({
36-
'&:not(:first-child)': {
37-
marginLeft: spacing[100],
38-
},
39-
});
40-
4133
export type ItemActionMenuProps<Action extends string> = {
4234
actions: MenuAction<Action>[];
4335
onAction(actionName: Action): void;
@@ -70,15 +62,19 @@ export function ItemActionMenu<Action extends string>({
7062
const menuTriggerRef = useRef<HTMLButtonElement | null>(null);
7163
const [isMenuOpen, setIsMenuOpen] = useState(false);
7264

73-
const onClick = useCallback(
65+
const onClick: React.MouseEventHandler<HTMLElement> = useCallback(
7466
(evt) => {
7567
evt.stopPropagation();
7668
if (evt.currentTarget.dataset.menuitem) {
7769
setIsMenuOpen(false);
7870
// Workaround for https://jira.mongodb.org/browse/PD-1674
7971
menuTriggerRef.current?.focus();
8072
}
81-
onAction(evt.currentTarget.dataset.action);
73+
const actionName = evt.currentTarget.dataset.action;
74+
if (typeof actionName !== 'string') {
75+
throw new Error('Expected element to have a "data-action" attribute');
76+
}
77+
onAction(actionName as Action);
8278
},
8379
[onAction]
8480
);
@@ -120,7 +116,7 @@ export function ItemActionMenu<Action extends string>({
120116
evt.stopPropagation();
121117
onClick && onClick(evt);
122118
}}
123-
className={cx(buttonStyle, iconClassName)}
119+
className={iconClassName}
124120
style={iconStyle}
125121
>
126122
{children}
@@ -145,7 +141,7 @@ export function ItemActionMenu<Action extends string>({
145141
return (
146142
<MenuItem
147143
key={action}
148-
data-testid={actionTestId<Action>(dataTestId, action)}
144+
data-testid={actionTestId(dataTestId, action)}
149145
data-action={action}
150146
data-menuitem={true}
151147
glyph={<ActionGlyph glyph={icon} size={iconSize} />}

‎packages/compass-components/src/components/actions/utils.ts‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,3 @@
1-
export function actionTestId<Action extends string>(
2-
dataTestId: string | undefined,
3-
action: Action
4-
) {
1+
export function actionTestId(dataTestId: string | undefined, action: string) {
52
return dataTestId ? `${dataTestId}-${action}-action` : undefined;
63
}

‎packages/compass-connections-navigation/src/base-navigation-item.tsx‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import {
66
ItemActionControls,
77
cx,
88
} from '@mongodb-js/compass-components';
9-
import { ROW_HEIGHT, type Actions } from './constants';
9+
import { type Actions, ROW_HEIGHT } from './constants';
1010
import { ExpandButton } from './tree-item';
1111
import { type NavigationItemActions } from './item-actions';
1212

@@ -128,13 +128,13 @@ export const NavigationBaseItem: React.FC<NavigationBaseItemProps> = ({
128128
<span title={name}>{name}</span>
129129
</div>
130130
<div className={actionControlsWrapperStyles}>
131-
<ItemActionControls<Actions>
131+
<ItemActionControls
132132
menuClassName={menuStyles}
133133
isVisible={isActive || isHovered || isFocused}
134134
data-testid="sidebar-navigation-item-actions"
135135
iconSize="xsmall"
136136
{...actionProps}
137-
></ItemActionControls>
137+
/>
138138
{children}
139139
</div>
140140
</div>

‎packages/compass-connections-navigation/src/connect-button.tsx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,9 @@ import {
33
Button,
44
type ItemComponentProps,
55
} from '@mongodb-js/compass-components';
6+
import type { Actions } from './constants';
67

7-
type ConnectButtonProps = ItemComponentProps<string>;
8+
type ConnectButtonProps = ItemComponentProps<Actions>;
89

910
export function ConnectButton({
1011
action,

0 commit comments

Comments
 (0)