-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Refacto views #10272
Refacto views #10272
Changes from 3 commits
9ad87d4
2c95605
ec415ef
3f29d16
355035a
b32c205
e081da6
16309f1
6b4928e
13b5a5e
42daab9
f247fc0
07d8335
f45b377
2ea1e18
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,8 +4,7 @@ import { RunWorkflowRecordAgnosticActionMenuEntriesSetter } from '@/action-menu/ | |
import { ActionMenuConfirmationModals } from '@/action-menu/components/ActionMenuConfirmationModals'; | ||
import { RecordShowActionMenuButtons } from '@/action-menu/components/RecordShowActionMenuButtons'; | ||
import { ActionMenuContext } from '@/action-menu/contexts/ActionMenuContext'; | ||
|
||
import { contextStoreCurrentObjectMetadataIdComponentState } from '@/context-store/states/contextStoreCurrentObjectMetadataIdComponentState'; | ||
import { contextStoreCurrentObjectMetadataItemComponentState } from '@/context-store/states/contextStoreCurrentObjectMetadataItemComponentState'; | ||
import { ObjectMetadataItem } from '@/object-metadata/types/ObjectMetadataItem'; | ||
import { ObjectRecord } from '@/object-record/types/ObjectRecord'; | ||
import { useRecoilComponentValueV2 } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentValueV2'; | ||
|
@@ -26,8 +25,8 @@ export const RecordShowActionMenu = ({ | |
objectNameSingular: string; | ||
handleFavoriteButtonClick: () => void; | ||
}) => { | ||
const contextStoreCurrentObjectMetadataId = useRecoilComponentValueV2( | ||
contextStoreCurrentObjectMetadataIdComponentState, | ||
const contextStoreCurrentObjectMetadataItem = useRecoilComponentValueV2( | ||
contextStoreCurrentObjectMetadataItemComponentState, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question: Went from an atomic ID listener to the whole objectMetadataItem ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, because the DevXP was painful (always have to find the objectMetadataItem from the Id). Another possibility is to create a hook or a selector on top of that. Which would actually be better I believe |
||
); | ||
|
||
const isCommandMenuV2Enabled = useIsFeatureEnabled( | ||
|
@@ -42,7 +41,7 @@ export const RecordShowActionMenu = ({ | |
|
||
return ( | ||
<> | ||
{contextStoreCurrentObjectMetadataId && ( | ||
{contextStoreCurrentObjectMetadataItem && ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: The conditional check here may be redundant since objectMetadataItem is already passed as a required prop to this component There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. False positive |
||
<ActionMenuContext.Provider | ||
value={{ | ||
isInRightDrawer: false, | ||
|
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.
Is there any risk that this state get desynchronized from the objectMetadataItems state ? Could this be a component selector instead ? Based on the id ?
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.
good idea!