Skip to content

feat(compass-components): add context menu COMPASS-9386 #6956

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 26 commits into
base: gagik/headless-context-menu
Choose a base branch
from

Conversation

gagik
Copy link
Contributor

@gagik gagik commented May 23, 2025

Ties #6937's context menu with leafygreen components and exposes that.

gagik added 14 commits May 22, 2025 09:38
…p menu state consistent

The refactor is meant to make the Leafygreen integration more straightforward:
1. We stick to item groups and instead have a single wrapper to handle any rendering differences between groups. This allows the wrapper to always have context of all items when rendering which is useful when inserting menu seperators in Leafygreen. Also encourages consistent UI (while allowing per-case customization if needed at wrapper-level). We could introduce itemWrappers instead of itemGroups but having one wrapper handling all seems cleaner to me.
2. More of the responsibility is moved to a parent wrapper component that will house the context menu. This allows us to standardize the right click menu and make better use of Leafygreen's menu component including its click handling (which has been removed from the context menu library).
3. Menu state (i.e. position) is now preserved even closed; this is useful for leafygreen's menu to animate in the same position instead of losing the position all together.
@github-actions github-actions bot added the feat label May 23, 2025
@gagik gagik changed the title feat(compass-components): add context menu feat(compass-components): add context menu COMPASS-9386 May 23, 2025
@gagik gagik added the no release notes Fix or feature not for release notes label May 23, 2025
gagik added 10 commits May 23, 2025 17:19
…p menu state consistent

The refactor is meant to make the Leafygreen integration more straightforward:
1. We stick to item groups and instead have a single wrapper to handle any rendering differences between groups. This allows the wrapper to always have context of all items when rendering which is useful when inserting menu seperators in Leafygreen. Also encourages consistent UI (while allowing per-case customization if needed at wrapper-level). We could introduce itemWrappers instead of itemGroups but having one wrapper handling all seems cleaner to me.
2. More of the responsibility is moved to a parent wrapper component that will house the context menu. This allows us to standardize the right click menu and make better use of Leafygreen's menu component including its click handling (which has been removed from the context menu library).
3. Menu state (i.e. position) is now preserved even closed; this is useful for leafygreen's menu to animate in the same position instead of losing the position all together.
@gagik gagik force-pushed the gagik/headless-context-menu branch from 82a10c5 to ca1fb86 Compare May 23, 2025 15:20
children: React.ReactNode;
}) {
return (
<ContextMenuProviderBase wrapper={ContextMenu}>
Copy link
Contributor Author

@gagik gagik May 23, 2025

Choose a reason for hiding this comment

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

maybe there's a better way to structure this, including the wrapper passing in general

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat no release notes Fix or feature not for release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant