Skip to content
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

Fix modals being unregistered inside command menu #9155

Merged
merged 3 commits into from
Dec 20, 2024

Conversation

bosiraphael
Copy link
Contributor

Fix modals being unregistered inside command menu

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

Added a new shouldCloseCommandMenuOnClick property across command menu components to control menu closing behavior after item clicks, fixing issues with premature modal unregistration.

  • Added shouldCloseCommandMenuOnClick to Command type in types/Command.ts and implemented in CommandMenuItem.tsx
  • Set shouldCloseCommandMenuOnClick: true for all navigation commands in CommandMenuNavigateCommands.ts
  • Added ActionMenuContext provider with onActionExecutedCallback in DefaultLayout.tsx to handle menu closing on action execution
  • Updated useCommandMenu hook to conditionally close menu based on shouldCloseCommandMenuOnClick parameter
  • Added test coverage for new shouldCloseCommandMenuOnClick behavior in useCommandMenu.test.tsx

7 file(s) reviewed, 3 comment(s)
Edit PR Review Bot Settings | Greptile

@charlesBochet charlesBochet merged commit a0b5720 into main Dec 20, 2024
18 of 22 checks passed
@charlesBochet charlesBochet deleted the r-fix-command-menu-actions branch December 20, 2024 09:45
mdrazak2001 pushed a commit to mdrazak2001/twenty that referenced this pull request Dec 20, 2024
Fix modals being unregistered inside command menu
samyakpiya pushed a commit to samyakpiya/twenty that referenced this pull request Dec 28, 2024
Fix modals being unregistered inside command menu
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants