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 block editor not changing content when switching to a new activity #9107

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Weiko
Copy link
Member

@Weiko Weiko commented Dec 17, 2024

WIP

@Weiko Weiko added the -PR: wip label Dec 17, 2024
@Weiko Weiko changed the title fix block editor changed content fix block editor not changing content when switching to a new activity Dec 17, 2024
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

This PR introduces a new hook useReplaceBlockEditorContent to manage block editor content synchronization, replacing the previous state management approach using activityBodyFamilyState.

  • Unsafe JSON.parse in useReplaceBlockEditorContent.tsx lacks error handling, risking runtime exceptions
  • Editor parameter typed as any in useReplaceBlockEditorContent reduces type safety
  • Potential race condition in RichTextEditor.tsx between content updates and editor state synchronization
  • replaceBlocks call in useReplaceBlockEditorContent 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,
Copy link
Contributor

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

Comment on lines +20 to +21
? JSON.parse(activityInStore?.body)
: [{ type: 'paragraph', content: '' }];
Copy link
Contributor

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();
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants