-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
7499 refactor right drawer to have contextual actions #7954
7499 refactor right drawer to have contextual actions #7954
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
This pull request refactors the right drawer to implement contextual actions and modifies the context store states to be component-specific. The changes introduce a new mainContextStore
concept and add support for contextual actions in various components.
- Introduced
RecordIndexActionMenu
andRecordShowActionMenu
components to handle different action menu contexts - Refactored context store states to use
createComponentStateV2
for component-specific state management - Added
ActionMenuType
to differentiate between 'recordIndex' and 'recordShow' action menus - Implemented
SetMainContextStoreComponentInstanceIdEffect
to manage the main context store component instance ID - Updated action menu components to use new context store states and support contextual actions
30 file(s) reviewed, 16 comment(s)
Edit PR Review Bot Settings | Greptile
...les/action-menu/actions/record-actions/components/MultipleRecordsActionMenuEntriesSetter.tsx
Show resolved
Hide resolved
.../src/modules/action-menu/actions/record-actions/components/RecordActionMenuEntriesSetter.tsx
Show resolved
Hide resolved
packages/twenty-front/src/modules/action-menu/components/ActionMenu.tsx
Outdated
Show resolved
Hide resolved
packages/twenty-front/src/modules/action-menu/components/RecordIndexActionMenu.tsx
Show resolved
Hide resolved
packages/twenty-front/src/modules/action-menu/components/RecordIndexActionMenuBarEntry.tsx
Outdated
Show resolved
Hide resolved
packages/twenty-front/src/modules/action-menu/components/RecordShowActionMenuBar.tsx
Show resolved
Hide resolved
...-front/src/modules/context-store/states/contextStoreNumberOfSelectedRecordsComponentState.ts
Show resolved
Hide resolved
...nty-front/src/modules/context-store/states/contexts/ContextStoreComponentInstanceContext.tsx
Show resolved
Hide resolved
packages/twenty-front/src/modules/context-store/states/mainContextStoreComponentInstanceId.ts
Show resolved
Hide resolved
@@ -0,0 +1 @@ | |||
export type ActionMenuType = 'recordIndex' | 'recordShow'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need this? I think it's a bit broken from a dependency perspective action-menu is a component that should not have to know about where it's used
const contextStoreNumberOfSelectedRecords = useRecoilValue( | ||
contextStoreNumberOfSelectedRecordsState, | ||
export const RecordActionMenuEntriesSetter = ({ | ||
actionMenuType, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also seems a bit painful to drill down
|
||
// For now, this component is the same as RecordIndexActionMenuBarEntry but they | ||
// will probably diverge in the future | ||
export const RecordShowActionMenuBarEntry = ({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where is this bar displayed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Closes #7499
mainContextStore
which will dictate the available actions inside the command K