-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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 block editor not changing content when switching to a new activity #9107
base: main
Are you sure you want to change the base?
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 PR introduces a new hook useReplaceBlockEditorContent
to manage block editor content synchronization, replacing the previous state management approach using activityBodyFamilyState
.
- Unsafe
JSON.parse
inuseReplaceBlockEditorContent.tsx
lacks error handling, risking runtime exceptions - Editor parameter typed as
any
inuseReplaceBlockEditorContent
reduces type safety - Potential race condition in
RichTextEditor.tsx
between content updates and editor state synchronization replaceBlocks
call inuseReplaceBlockEditorContent
appears to have incorrect parameter order- Removal of debounced state updates could impact drag-and-drop performance
2 file(s) reviewed, 3 comment(s)
Edit PR Review Bot Settings | Greptile
|
||
export const useReplaceBlockEditorContent = ( | ||
activityId: string, | ||
editor: any, |
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.
style: editor type should be properly typed rather than 'any' to ensure type safety
? JSON.parse(activityInStore?.body) | ||
: [{ type: 'paragraph', content: '' }]; |
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.
logic: JSON.parse could throw if body contains invalid JSON. Wrap in try/catch and provide fallback
editor, | ||
); | ||
|
||
replaceBlockEditorContent(); |
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.
logic: replaceBlockEditorContent() called directly in render function could cause infinite re-renders or race conditions. Should be in useEffect.
WIP