refactor(ActionMenuItem): expose ActionMenuItem as a standalone component #1457
Conversation
WalkthroughThis pull request introduces a refactoring of the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
packages/react-components/src/components/ActionMenuItem/types.ts (1)
3-16: Consider expanding the 'kind' prop optionsThe 'kind' prop only supports 'warning'. Consider adding more semantic variants like 'danger', 'info', or 'success' for better flexibility.
packages/react-components/src/components/ActionMenu/stories-constants.tsx (1)
Line range hint
46-52: Simplify story examplesUsing Array spread to generate multiple similar items makes the stories less meaningful. Consider showing fewer, more diverse examples instead.
- ...[...Array(10)].map((_, index) => ({ - key: `option${index + 8}`, - element: <ActionMenuItem>{`Menu item #${index + 8}`}</ActionMenuItem>, - onClick: noop, - })), + { + key: 'more', + element: <ActionMenuItem>More options...</ActionMenuItem>, + onClick: noop, + }packages/react-components/src/components/ActionMenuItem/ActionMenuItem.mdx (1)
22-26: Enhance example implementationAdd real-world examples demonstrating common use cases and accessibility best practices.
// Add examples like: <ActionMenuItem leftNode={<Icon source={Delete} />} kind="warning" aria-label="Delete item permanently" > Delete permanently </ActionMenuItem>packages/react-components/src/components/ActionMenuItem/ActionMenuItem.stories.tsx (1)
27-45: Consider using action handlers instead of noopReplace
noopwith Storybook's action handlers to enable click tracking in the Storybook UI.-import noop from '../../utils/noop'; +import { action } from '@storybook/addon-actions'; const defaultOptions = [ { key: 'one', element: <ActionMenuItem>Simple Item 1</ActionMenuItem>, - onClick: noop, + onClick: action('clicked-item-1'), },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
packages/react-components/src/components/ActionBar/ActionBar.tsx(1 hunks)packages/react-components/src/components/ActionMenu/ActionMenu.stories.tsx(1 hunks)packages/react-components/src/components/ActionMenu/index.ts(0 hunks)packages/react-components/src/components/ActionMenu/stories-constants.tsx(1 hunks)packages/react-components/src/components/ActionMenu/types.ts(0 hunks)packages/react-components/src/components/ActionMenuItem/ActionMenuItem.mdx(1 hunks)packages/react-components/src/components/ActionMenuItem/ActionMenuItem.stories.css(1 hunks)packages/react-components/src/components/ActionMenuItem/ActionMenuItem.stories.tsx(1 hunks)packages/react-components/src/components/ActionMenuItem/index.ts(1 hunks)packages/react-components/src/components/ActionMenuItem/types.ts(1 hunks)packages/react-components/src/components/InviteAgents/InviteAgents.tsx(1 hunks)packages/react-components/src/index.ts(1 hunks)
💤 Files with no reviewable changes (2)
- packages/react-components/src/components/ActionMenu/index.ts
- packages/react-components/src/components/ActionMenu/types.ts
✅ Files skipped from review due to trivial changes (3)
- packages/react-components/src/components/ActionMenuItem/index.ts
- packages/react-components/src/components/ActionMenuItem/ActionMenuItem.stories.css
- packages/react-components/src/components/ActionBar/ActionBar.tsx
🔇 Additional comments (4)
packages/react-components/src/index.ts (1)
11-11: LGTM: Export follows alphabetical order
The new export for ActionMenuItem is correctly placed in alphabetical order among other component exports.
packages/react-components/src/components/ActionMenuItem/ActionMenuItem.stories.tsx (1)
23-23: Verify if chromatic delay is necessary
The 300ms delay parameter might affect test execution time. Verify if it's required for proper rendering.
✅ Verification successful
The 300ms chromatic delay is consistently used across interactive components
The delay is used in all popup/interactive components (ActionMenu, Picker, AutoComplete, Popover, Tooltip) to ensure proper visual testing of animations and transitions. This is a standardized pattern in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if other menu components use similar delay
rg -l 'chromatic.*delay' packages/react-components/src/components/**/stories.tsx
Length of output: 180
Script:
#!/bin/bash
# Let's try a more accurate search for stories files with chromatic delay
fd "stories\.tsx$" packages/react-components/src/components/ -x rg -l "chromatic.*delay" {}
# Also search for any chromatic configuration in the codebase
rg -l "chromatic.*delay"
Length of output: 1006
Script:
#!/bin/bash
# Let's check the actual delay values in these components
rg "chromatic.*delay" packages/react-components/src/components/*/stories.tsx -C 1
Length of output: 180
Script:
#!/bin/bash
# Let's check the actual delay values using exact paths we found earlier
rg "chromatic.*delay" packages/react-components/src/components/ActionMenuItem/ActionMenuItem.stories.tsx packages/react-components/src/components/Tooltip/Tooltip.stories.tsx packages/react-components/src/components/Popover/Popover.stories.tsx packages/react-components/src/components/Picker/Picker.stories.tsx packages/react-components/src/components/AutoComplete/AutoComplete.stories.tsx packages/react-components/src/components/ActionMenu/ActionMenu.stories.tsx -C 2
Length of output: 3259
packages/react-components/src/components/ActionMenu/ActionMenu.stories.tsx (1)
6-6: Verify all ActionMenuItem imports are updated
The import path has been updated correctly. Let's verify all other files have been updated consistently.
✅ Verification successful
The previous search might have been too specific. Let's broaden it to find all ActionMenuItem imports and analyze their patterns.
All ActionMenuItem imports are correctly updated
All files importing ActionMenuItem are using the correct relative path ../ActionMenuItem or local imports within the ActionMenuItem component directory itself.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any remaining old imports
rg -l "from ['\"].*ActionMenu['\"].*ActionMenuItem" packages/react-components/src/
Length of output: 88
Script:
#!/bin/bash
# Search for any ActionMenuItem imports using ripgrep
rg "import.*ActionMenuItem" packages/react-components/src/
# Also search for potential relative imports
rg "from ['\"]\..*ActionMenuItem" packages/react-components/src/
Length of output: 2752
packages/react-components/src/components/InviteAgents/InviteAgents.tsx (1)
14-15: LGTM! Import changes align with component restructuring.
The separation of imports reflects the new standalone nature of ActionMenuItem.
Let's verify the new import paths are working:
✅ Verification successful
Import changes are correctly implemented across the codebase
All files importing ActionMenuItem are now using the correct standalone path. The component exists in its dedicated location with proper test, story, and implementation files.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the ActionMenuItem component exists in its new location
fd -e tsx -e ts "ActionMenuItem" packages/react-components/src/components/
# Verify no other files still import ActionMenuItem from ActionMenu
rg "import.*ActionMenuItem.*from.*ActionMenu" packages/react-components/src/
Length of output: 1291
| ## Component API <a id="ComponentAPI" /> | ||
|
|
||
| <ArgTypes of={ActionMenuItemStories.Default} sort="requiredFirst" /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add accessibility documentation
Include a section about ARIA attributes, keyboard navigation, and screen reader considerations.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/react-components/src/components/ActionMenuItem/ActionMenuItem.tsx (2)
13-18: Consider adding JSDoc commentsAdding JSDoc comments would improve the component's documentation, especially for the
kindprop values.Example:
+/** + * A menu item component that supports left/right nodes and different variants + * @param {Object} props + * @param {ReactNode} props.leftNode - Content to render on the left side + * @param {ReactNode} props.rightNode - Content to render on the right side + * @param {'warning' | undefined} props.kind - Variant of the menu item + */ export const ActionMenuItem: FC<PropsWithChildren<ActionMenuItemProps>> = ({
Line range hint
19-36: Add accessibility attributesConsider enhancing accessibility by adding appropriate ARIA attributes and keyboard interaction support.
<div className={cx(styles[baseClass], { [styles[`${baseClass}--warning`]]: kind === 'warning', })} + role="menuitem" + tabIndex={0} >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/react-components/src/components/ActionMenuItem/ActionMenuItem.stories.css(1 hunks)packages/react-components/src/components/ActionMenuItem/ActionMenuItem.stories.tsx(1 hunks)packages/react-components/src/components/ActionMenuItem/ActionMenuItem.tsx(2 hunks)packages/react-components/src/components/ActionMenuItem/types.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/react-components/src/components/ActionMenuItem/ActionMenuItem.stories.css
- packages/react-components/src/components/ActionMenuItem/types.ts
- packages/react-components/src/components/ActionMenuItem/ActionMenuItem.stories.tsx
🔇 Additional comments (1)
packages/react-components/src/components/ActionMenuItem/ActionMenuItem.tsx (1)
Line range hint 1-11: LGTM! Clean import structure
The imports are well-organized and follow best practices with proper separation of concerns.
Resolves: n/a
Description
This pull request reorganizes the
ActionMenuItemcomponent by moving it to its own directory and updating its references throughout the codebase. Additionally, it introduces new documentation and storybook entries for theActionMenuItemcomponent. The most important changes include updating imports, adding new documentation, and creating new storybook entries.Storybook
https://feature-move-actionmenuitem--613a8e945a5665003a05113b.chromatic.com
Checklist
Obligatory:
Summary by CodeRabbit
New Features
ActionMenuItemcomponent.ActionMenuItemcomponent showcasing different options.Bug Fixes
ActionMenuItemto improve modularity.Documentation
ActionMenuItemwith usage examples and API details.Chores
Alertand includeActionMenuItemin the main index.