chore(compass-components): refactor ItemActionControls #2

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

@@ -49,15 +49,19 @@ export function DropdownMenuButton<Action extends string>({
   const menuTriggerRef = useRef<HTMLButtonElement | null>(null);
   const [isMenuOpen, setIsMenuOpen] = useState(false);
 
-  const onClick = useCallback(
+  const onClick: React.MouseEventHandler<HTMLElement> = useCallback(
     (evt) => {
       evt.stopPropagation();
       if (evt.currentTarget.dataset.menuitem) {
         setIsMenuOpen(false);
         // Workaround for https://jira.mongodb.org/browse/PD-1674
         menuTriggerRef.current?.focus();
       }
-      onAction(evt.currentTarget.dataset.action);
+      const actionName = evt.currentTarget.dataset.action;
+      if (typeof actionName !== 'string') {
+        throw new Error('Expected element to have a "data-action" attribute');
+      }
+      onAction(actionName as Action);
     },
     [onAction]
   );
@@ -113,7 +117,7 @@ export function DropdownMenuButton<Action extends string>({
           <MenuItem
             active={activeAction === action}
             key={action}
-            data-testid={actionTestId<Action>(dataTestId, action)}
+            data-testid={actionTestId(dataTestId, action)}
             data-action={action}
             data-menuitem={true}
             glyph={<ActionGlyph glyph={icon} size={iconSize} />}

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

@@ -1,18 +1,10 @@
 import React from 'react';
-import { css, cx } from '@leafygreen-ui/emotion';
-import { spacing } from '@leafygreen-ui/tokens';
+import { cx } from '@leafygreen-ui/emotion';
 
 import { ItemActionButtonSize } from './constants';
 import type { ItemComponentProps } from './types';
 import { SmallIconButton } from './small-icon-button';
 
-// TODO: Move to a parent component - or a flex gap
-const buttonStyle = css({
-  '&:not(:first-child)': {
-    marginLeft: spacing[100],
-  },
-});
-
 export function ItemActionButton<Action extends string>({
   action,
   icon = <></>,
@@ -36,7 +28,7 @@ export function ItemActionButton<Action extends string>({
       data-action={action}
       data-testid={dataTestId}
       onClick={onClick}
-      className={cx(buttonStyle, iconClassName, className)}
+      className={cx(iconClassName, className)}
       style={iconStyle}
       disabled={isDisabled}
     />

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

@@ -20,7 +20,7 @@ describe('item action controls components', function () {
           actions={[]}
           onAction={() => {}}
           data-testid="test-actions"
-        ></ItemActionControls>
+        />
       );
 
       expect(screen.queryByTestId('test-actions')).to.not.exist;
@@ -32,7 +32,7 @@ describe('item action controls components', function () {
           actions={[{ action: 'copy', label: 'Copy', icon: 'Copy' }]}
           onAction={() => {}}
           data-testid="test-actions"
-        ></ItemActionControls>
+        />
       );
 
       expect(screen.getByTestId('test-actions')).to.exist;
@@ -47,7 +47,7 @@ describe('item action controls components', function () {
           onAction={() => {}}
           data-testid="test-actions"
           collapseToMenuThreshold={1}
-        ></ItemActionControls>
+        />
       );
 
       const trigger = screen.getByTestId('test-actions-show-actions');
@@ -69,7 +69,7 @@ describe('item action controls components', function () {
           onAction={onAction}
           data-testid="test-actions"
           collapseToMenuThreshold={3}
-        ></ItemActionControls>
+        />
       );
 
       expect(onAction).not.to.be.called;
@@ -90,7 +90,7 @@ describe('item action controls components', function () {
           onAction={onAction}
           data-testid="test-actions"
           collapseToMenuThreshold={1}
-        ></ItemActionControls>
+        />
       );
 
       expect(onAction).not.to.be.called;
@@ -112,7 +112,7 @@ describe('item action controls components', function () {
           onAction={() => {}}
           data-testid="test-actions"
           collapseAfter={1}
-        ></ItemActionControls>
+        />
       );
 
       expect(screen.queryByTestId('test-actions-connect-action')).to.exist;
@@ -146,7 +146,7 @@ describe('item action controls components', function () {
           onAction={() => {}}
           data-testid="test-actions"
           collapseAfter={1}
-        ></ItemActionControls>
+        />
       );
 
       const actionButton = screen.getByTestId('test-actions-connect-action');

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

@@ -1,4 +1,4 @@
-import React, { useMemo } from 'react';
+import React from 'react';
 import { spacing } from '@leafygreen-ui/tokens';
 import { css, cx } from '@leafygreen-ui/emotion';
 import type { RenderMode } from '@leafygreen-ui/popover';
@@ -13,13 +13,7 @@ const actionControlsStyle = css({
   marginLeft: 'auto',
   alignItems: 'center',
   display: 'flex',
-});
-
-// Action buttons are rendered 4px apart from each other. With this we keep the
-// same spacing also when action buttons are rendered alongside action menu
-// (happens when collapseAfter prop is specified)
-const actionMenuWithActionControlsStyles = css({
-  marginLeft: spacing[100],
+  gap: spacing[100],
 });
 
 export type ItemActionControlsProps<Action extends string> = {
@@ -53,33 +47,20 @@ export function ItemActionControls<Action extends string>({
   collapseToMenuThreshold = 2,
   'data-testid': dataTestId,
 }: ItemActionControlsProps<Action>) {
-  const sharedProps = useMemo(
-    () => ({
-      isVisible,
-      onAction,
-      className: cx('item-action-controls', className),
-      iconClassName,
-      iconStyle,
-      iconSize,
-      'data-testid': dataTestId,
-    }),
-    [
-      isVisible,
-      onAction,
-      className,
-      iconClassName,
-      iconStyle,
-      iconSize,
-      dataTestId,
-    ]
-  );
-  const sharedMenuProps = useMemo(
-    () => ({
-      menuClassName,
-      renderMode,
-    }),
-    [menuClassName, renderMode]
-  );
+  const sharedProps = {
+    isVisible,
+    onAction,
+    className: cx('item-action-controls', className),
+    iconClassName,
+    iconStyle,
+    iconSize,
+    'data-testid': dataTestId,
+  };
+  const sharedMenuProps = {
+    menuClassName,
+    renderMode,
+  };
+
   if (actions.length === 0) {
     return null;
   }
@@ -90,19 +71,12 @@ export function ItemActionControls<Action extends string>({
     const collapsedActions = actions.slice(collapseAfter);
     return (
       <div className={actionControlsStyle}>
-        <ItemActionGroup
-          actions={visibleActions}
-          {...sharedProps}
-        ></ItemActionGroup>
+        <ItemActionGroup {...sharedProps} actions={visibleActions} />
         <ItemActionMenu
-          actions={collapsedActions}
           {...sharedProps}
           {...sharedMenuProps}
-          className={cx(
-            actionMenuWithActionControlsStyles,
-            sharedProps.className
-          )}
-        ></ItemActionMenu>
+          actions={collapsedActions}
+        />
       </div>
     );
   }
@@ -111,13 +85,9 @@ export function ItemActionControls<Action extends string>({
 
   if (shouldShowMenu) {
     return (
-      <ItemActionMenu
-        actions={actions}
-        {...sharedProps}
-        {...sharedMenuProps}
-      ></ItemActionMenu>
+      <ItemActionMenu actions={actions} {...sharedProps} {...sharedMenuProps} />
     );
   }
 
-  return <ItemActionGroup actions={actions} {...sharedProps}></ItemActionGroup>;
+  return <ItemActionGroup actions={actions} {...sharedProps} />;
 }

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

@@ -19,13 +19,7 @@ const containerStyle = css({
   marginLeft: 'auto',
   alignItems: 'center',
   display: 'flex',
-});
-
-// TODO: Move to a parent component - or a flex gap
-const actionGroupButtonStyle = css({
-  '&:not(:first-child)': {
-    marginLeft: spacing[100],
-  },
+  gap: spacing[100],
 });
 
 export type ItemActionGroupProps<Action extends string> = {
@@ -49,10 +43,14 @@ export function ItemActionGroup<Action extends string>({
   isVisible = true,
   'data-testid': dataTestId,
 }: ItemActionGroupProps<Action>) {
-  const onClick = useCallback(
+  const onClick: React.MouseEventHandler<HTMLElement> = useCallback(
     (evt) => {
       evt.stopPropagation();
-      onAction(evt.currentTarget.dataset.action);
+      const actionName = evt.currentTarget.dataset.action;
+      if (typeof actionName !== 'string') {
+        throw new Error('Expected element to have a "data-action" attribute');
+      }
+      onAction(actionName as Action);
     },
     [onAction]
   );
@@ -85,7 +83,7 @@ export function ItemActionGroup<Action extends string>({
             iconStyle={iconStyle}
             iconClassName={iconClassName}
             onClick={onClick}
-            data-testid={actionTestId<Action>(dataTestId, itemProps.action)}
+            data-testid={actionTestId(dataTestId, itemProps.action)}
           />
         );
 
@@ -94,14 +92,9 @@ export function ItemActionGroup<Action extends string>({
             <Tooltip
               key={itemProps.action}
               {...tooltipProps}
-              trigger={
-                <div
-                  className={actionGroupButtonStyle}
-                  style={{ display: 'inherit' }}
-                >
-                  {item}
-                </div>
-              }
+              // Wrapping the item in a div, because the `trigger` must accept and render `children`
+              // See docs for the prop for more information
+              trigger={<div style={{ display: 'inherit' }}>{item}</div>}
             >
               {tooltip}
             </Tooltip>

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

@@ -1,6 +1,5 @@
 import React, { useCallback, useRef, useState } from 'react';
 import { css, cx } from '@leafygreen-ui/emotion';
-import { spacing } from '@leafygreen-ui/tokens';
 import type { RenderMode } from '@leafygreen-ui/popover';
 
 import { Menu, MenuItem, MenuSeparator } from '../leafygreen';
@@ -31,13 +30,6 @@ const containerStyle = css({
   display: 'flex',
 });
 
-// TODO: Move to a parent component - or a flex gap
-const buttonStyle = css({
-  '&:not(:first-child)': {
-    marginLeft: spacing[100],
-  },
-});
-
 export type ItemActionMenuProps<Action extends string> = {
   actions: MenuAction<Action>[];
   onAction(actionName: Action): void;
@@ -70,15 +62,19 @@ export function ItemActionMenu<Action extends string>({
   const menuTriggerRef = useRef<HTMLButtonElement | null>(null);
   const [isMenuOpen, setIsMenuOpen] = useState(false);
 
-  const onClick = useCallback(
+  const onClick: React.MouseEventHandler<HTMLElement> = useCallback(
     (evt) => {
       evt.stopPropagation();
       if (evt.currentTarget.dataset.menuitem) {
         setIsMenuOpen(false);
         // Workaround for https://jira.mongodb.org/browse/PD-1674
         menuTriggerRef.current?.focus();
       }
-      onAction(evt.currentTarget.dataset.action);
+      const actionName = evt.currentTarget.dataset.action;
+      if (typeof actionName !== 'string') {
+        throw new Error('Expected element to have a "data-action" attribute');
+      }
+      onAction(actionName as Action);
     },
     [onAction]
   );
@@ -120,7 +116,7 @@ export function ItemActionMenu<Action extends string>({
                 evt.stopPropagation();
                 onClick && onClick(evt);
               }}
-              className={cx(buttonStyle, iconClassName)}
+              className={iconClassName}
               style={iconStyle}
             >
               {children}
@@ -145,7 +141,7 @@ export function ItemActionMenu<Action extends string>({
           return (
             <MenuItem
               key={action}
-              data-testid={actionTestId<Action>(dataTestId, action)}
+              data-testid={actionTestId(dataTestId, action)}
               data-action={action}
               data-menuitem={true}
               glyph={<ActionGlyph glyph={icon} size={iconSize} />}

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

@@ -1,6 +1,3 @@
-export function actionTestId<Action extends string>(
-  dataTestId: string | undefined,
-  action: Action
-) {
+export function actionTestId(dataTestId: string | undefined, action: string) {
   return dataTestId ? `${dataTestId}-${action}-action` : undefined;
 }

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

@@ -6,7 +6,7 @@ import {
   ItemActionControls,
   cx,
 } from '@mongodb-js/compass-components';
-import { ROW_HEIGHT, type Actions } from './constants';
+import { type Actions, ROW_HEIGHT } from './constants';
 import { ExpandButton } from './tree-item';
 import { type NavigationItemActions } from './item-actions';
 
@@ -128,13 +128,13 @@ export const NavigationBaseItem: React.FC<NavigationBaseItemProps> = ({
           <span title={name}>{name}</span>
         </div>
         <div className={actionControlsWrapperStyles}>
-          <ItemActionControls<Actions>
+          <ItemActionControls
             menuClassName={menuStyles}
             isVisible={isActive || isHovered || isFocused}
             data-testid="sidebar-navigation-item-actions"
             iconSize="xsmall"
             {...actionProps}
-          ></ItemActionControls>
+          />
           {children}
         </div>
       </div>

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

@@ -3,8 +3,9 @@ import {
   Button,
   type ItemComponentProps,
 } from '@mongodb-js/compass-components';
+import type { Actions } from './constants';
 
-type ConnectButtonProps = ItemComponentProps<string>;
+type ConnectButtonProps = ItemComponentProps<Actions>;
 
 export function ConnectButton({
   action,