8172 update the right drawer action menu to open with command o #8375
Conversation
…open-with-command-o
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR updates the right drawer action menu to implement a dropdown-based system with keyboard shortcut support.
- Added new
RightDrawerActionMenuDropdowncomponent with⌘Oshortcut support viauseScopedHotkeyshook - Enhanced
Buttoncomponent intwenty-uito display keyboard shortcuts with styled separator - Removed
RecordShowActionMenuBarEntryandRecordShowRightDrawerActionMenuBarin favor of dropdown implementation - Added utility function
getRightDrawerActionMenuDropdownIdFromActionMenuIdfor consistent dropdown ID generation - Modified
RichTextEditorhotkey configuration to prevent shortcut conflicts
11 file(s) reviewed, 10 comment(s)
Edit PR Review Bot Settings | Greptile
| <ActionMenuContext.Provider | ||
| value={{ | ||
| isInRightDrawer: true, | ||
| onActionExecutedCallback: () => {}, |
There was a problem hiding this comment.
logic: Empty callback could lead to missed state updates if actions need to trigger UI changes
| const theme = useTheme(); | ||
|
|
||
| useScopedHotkeys( | ||
| [Key.Escape, 'ctrl+o,meta+o'], |
There was a problem hiding this comment.
logic: Potential conflict between Escape and Ctrl+O/Cmd+O being in same hotkey array - these should be separate useScopedHotkeys calls
| {actionMenuEntries.map((item, index) => ( | ||
| <MenuItem | ||
| key={index} | ||
| LeftIcon={item.Icon} |
There was a problem hiding this comment.
style: Using array index as key can cause issues with React reconciliation if items are reordered. Consider using a stable unique identifier from item
| onClick={() => { | ||
| closeDropdown( | ||
| getRightDrawerActionMenuDropdownIdFromActionMenuId( | ||
| actionMenuId, | ||
| ), | ||
| ); | ||
| item.onClick?.(); | ||
| }} |
There was a problem hiding this comment.
style: Consider handling errors that may occur during item.onClick() execution
| clickableComponent={<Button title="Actions" shortcut="⌘O" />} | ||
| dropdownPlacement="top-end" | ||
| dropdownOffset={{ | ||
| y: parseInt(theme.spacing(2)), |
There was a problem hiding this comment.
logic: parseInt without radix parameter and potential NaN result if theme.spacing(2) returns non-numeric value
| @@ -113,12 +113,21 @@ export const WithButtonClicks: Story = { | |||
| play: async ({ canvasElement }) => { | |||
There was a problem hiding this comment.
logic: Test should verify that the dropdown closes after each action. Currently it manually reopens the dropdown but doesn't verify the closing behavior.
| RightDrawerHotkeyScope.RightDrawer, | ||
| [], | ||
| { | ||
| preventDefault: false, | ||
| }, |
There was a problem hiding this comment.
logic: The empty dependencies array could cause stale closure issues if any values from the closure are used in the callback. Consider adding any dependencies used in the callback function.
| const StyledSeparator = styled.div<{ size: 'sm' | 'md' }>` | ||
| background: ${({ theme }) => theme.border.color.light}; | ||
| height: ${({ theme, size }) => theme.spacing(size === 'sm' ? 4 : 8)}; | ||
| margin: 0 ${({ theme }) => theme.spacing(1)}; | ||
| width: 1px; | ||
| `; |
There was a problem hiding this comment.
style: StyledSeparator size prop accepts 'md' but it's never used in the component
| }; | ||
|
|
||
| export const ShortcutCatalog: CatalogStory<Story, typeof Button> = { | ||
| args: { title: 'Actions', shortcut: '⌘O' }, |
There was a problem hiding this comment.
style: ⌘O is Mac-specific. Consider using a cross-platform shortcut display like 'Cmd/Ctrl+O'
| decorators: [CatalogDecorator], | ||
| }; | ||
|
|
||
| export const ShortcutCatalog: CatalogStory<Story, typeof Button> = { |
There was a problem hiding this comment.
style: ShortcutCatalog duplicates state handling code from PositionCatalog. Consider extracting shared logic into a utility function
…open-with-command-o
packages/twenty-front/src/modules/action-menu/components/RecordShowRightDrawerActionMenu.tsx
Outdated
Show resolved
Hide resolved
packages/twenty-ui/src/input/button/components/__stories__/Button.stories.tsx
Show resolved
Hide resolved
bosiraphael
deleted the
8172-update-the-right-drawer-action-menu-to-open-with-command-o
branch
Closes #8172
command + OshortcutEnregistrement.de.l.ecran.2024-11-06.a.17.33.47.mov