Fix intermittent image display issue during Image Upload #410
Conversation
WalkthroughThe changes in this pull request focus on the Changes
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
frontend/src/components/editor/Preview.tsx (2)
70-70: Consider optimizing throttle configurationWhile removing
cmViewfrom the dependencies is correct and aligns with the content retrieval changes, the current throttle delay of 500ms might need fine-tuning:
- For image uploads, 500ms might be noticeable to users
- During collaborative editing, this delay could lead to perceived lag
Consider:
- Reducing DELAY for image uploads specifically
- Using different throttle delays for different update types
- Adding a debounce for text changes while keeping immediate updates for images
Example approach:
const DELAYS = { IMAGE_UPLOAD: 100, TEXT_CHANGE: 500 }; // Determine delay based on content type const getDelay = (content: string) => { return content.includes('![') ? DELAYS.IMAGE_UPLOAD : DELAYS.TEXT_CHANGE; };
Line range hint
1-108: Consider architectural improvements for better maintainabilityWhile the current changes fix the immediate issue, consider these architectural improvements:
- Extract the content synchronization logic into a custom hook:
const useDocumentSync = (doc: Doc, delay: number) => { const [content, setContent] = useState(""); // ... synchronization logic return content; };
- Add error boundaries to handle markdown rendering failures gracefully
- Memoize markdown parser configuration to prevent unnecessary recreations
Would you like me to provide a detailed implementation for any of these suggestions?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
frontend/src/components/editor/Preview.tsx(2 hunks)
🔇 Additional comments (1)
frontend/src/components/editor/Preview.tsx (1)
61-64: Simplified content retrieval addresses race condition
The change to use only editorStore.doc?.getRoot().content as the single source of truth is a good approach to fix the intermittent image display issues. This eliminates potential race conditions that could occur when using both CodeMirror's view state and the document content.
Please verify that:
- Images appear immediately after upload across different network conditions
- No content flickers occur during collaborative editing
What this PR does / why we need it:
This PR addresses a bug where images uploaded intermittently do not display immediately. The issue arises due to the rendering of the editor using data that precedes the CodeMirror editor application.
By ensuring that the image rendering is correctly synced with the latest data input, we improve the reliability of image uploads.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?:
Additional documentation:
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes