Skip to content
This repository was archived by the owner on Jul 20, 2025. It is now read-only.

Conversation

jeremy-babylonlabs
Copy link
Contributor

@jeremy-babylonlabs jeremy-babylonlabs commented Jul 17, 2025

Screenshot 2025-07-17 at 8 32 55 PM

@Copilot Copilot AI review requested due to automatic review settings July 17, 2025 08:50
Copilot

This comment was marked as outdated.

Copilot

This comment was marked as outdated.

Copy link
Collaborator

@jrwbabylonlab jrwbabylonlab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved, but should this component being put under components? it looks like a higher-level components to me.

@jeremy-babylonlabs
Copy link
Contributor Author

the implementation of the menu should be put in components

I think we can add a PR to put the Setting Menu in the widget

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Adds a fully-featured, responsive Menu component with nested submenus, mobile drawer support, and a viewport detection hook.

  • Introduces useIsMobile hook for mobile breakpoint detection.
  • Implements Menu, SubMenu, MenuItem, SubMenuItem, MenuDrawer, and MenuContext for flexible menu structures.
  • Wires up responsive behavior: Popover on desktop, sliding drawer on mobile.

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/index.tsx Export the new Menu entry point
src/hooks/useIsMobile.ts Add hook to detect mobile viewport size
src/components/Menu/index.ts Aggregate exports for Menu subcomponents
src/components/Menu/SubMenuItem.tsx Render submenu items with labels, description, and suffix
src/components/Menu/SubMenu.tsx Implement nested submenu with sliding drawer logic
src/components/Menu/MenuItem.tsx Implement menu item with click handling and context-based close
src/components/Menu/MenuDrawer.tsx Sliding drawer component with header and backdrop
src/components/Menu/MenuContext.tsx Create React context for menu state management
src/components/Menu/Menu.tsx Main Menu component combining Popover and mobile drawer
src/components/Menu/Menu.stories.tsx Add Storybook example demonstrating default menu
Comments suppressed due to low confidence (2)

src/components/Menu/SubMenu.tsx:10

  • Prop naming is inconsistent: SubMenu uses name while SubMenuItem uses label for primary text. Consider unifying these prop names across Menu components for API consistency.
  name?: string; // This is the recommended property for the primary text in menu items. Use this instead of older alternatives.

src/hooks/useIsMobile.ts:29

  • The useIsMobile hook currently lacks unit tests. Consider adding tests to verify correct behavior across different window widths and resize events.
  return isMobile;

@jrwbabylonlab
Copy link
Collaborator

@jeremy-babylonlabs it's quite hard to review what's been changed since last approval
image

Could u squash later commits or just push a single commit once ready?

@jrwbabylonlab
Copy link
Collaborator

@gbarkhatov @jonybur plz double check if there are more comments.

@jeremy-babylonlabs jeremy-babylonlabs merged commit 357e6f6 into main Jul 18, 2025
4 checks passed
@jeremy-babylonlabs jeremy-babylonlabs deleted the feat/menu-component branch July 18, 2025 16:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants