Windows Menu Support #24
Conversation
…/reactotron-macos into feature/windows-menu
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…/reactotron-macos into feature/windows-menu
| const isSeparator = (item: MenuItem | typeof MENU_SEPARATOR): item is typeof MENU_SEPARATOR => { | ||
| return item === MENU_SEPARATOR | ||
| } |
There was a problem hiding this comment.
I'd move this function out of the component, since it doesn't seem to rely on anything in scope here in the component. Could either move it below the component as a hoisted function isSeparator or into a util somewhere.
| @@ -0,0 +1,21 @@ | |||
| import { PlatformShortcut } from "app/utils/useSystemMenu/types" | |||
There was a problem hiding this comment.
Change to relative path (e.g. ../../...); while this works okay for types, if you try to import something from app/* at runtime, Metro won't find it.
| export type DropdownMenuItem = MenuItem | ||
|
|
||
| // Menu separator constant | ||
| export const MENU_SEPARATOR = "menu-item-separator" as const |
There was a problem hiding this comment.
I think this const could live in useSystemMenu instead ... that way this file could be entirely types and would be removed at runtime. I get why it kinda makes sense here too, just a suggestion.
| }: MenuDropdownItemProps) => { | ||
| const [hoveredItem, setHoveredItem] = useState<string | null>(null) | ||
| const hoverTimeoutRef = useRef<NodeJS.Timeout | null>(null) | ||
| const enabled = item.enabled !== false |
There was a problem hiding this comment.
Does it make more sense to go with disabled rather than enabled, so you don't have to check for false explicitly, and it aligns better with HTML's disabled prop?
| const handlePress = useCallback(() => { | ||
| if (!item.action || !enabled) return | ||
| item.action() | ||
| onItemPress(item) |
There was a problem hiding this comment.
This line (calling onItemPress) gets skipped even if the item is enabled and has an onItemPress callback, but is missing the item.action. Is that intentional?
| onHoverIn={handleHoverIn} | ||
| onHoverOut={handleHoverOut} | ||
| onPress={handlePress} | ||
| disabled={!enabled} |
There was a problem hiding this comment.
This gives more weight to the idea that we might want to use disabled instead of enabled.
Adds a menu to the Windows version of Reactotron.
Screen.Recording.2025-11-04.at.1.38.14.PM.mov
(Click to view screen recording.)
We went with a React Native menu system rather than native for flexibility; however, we may want to replace with a system menu in the future.
Here's a system menu as an example:
For the future, we can possibly use GetSystemMenu and: