fix: stabilize real-time search during IME input#235
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR updates SearchBar to pause real-time search during IME composition and adds tests for composition and whitespace transitions. It also refactors MarkdownRenderer image wrappers, adjusts a paragraph renderer return path, fixes one ForkTimeline test, and removes a VectorSearchSettings callback dependency. ChangesSearchBar IME Composition Handling
MarkdownRenderer Image Markup Refactor
Miscellaneous Fixes
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant SearchBarInput
participant SearchBar
User->>SearchBarInput: compositionStart
SearchBarInput->>SearchBar: set isComposing = true
User->>SearchBarInput: type preedit text
SearchBar->>SearchBar: skip performRealTimeSearch (isComposing)
User->>SearchBarInput: compositionEnd
SearchBarInput->>SearchBar: set isComposing = false
SearchBar->>SearchBar: updateRealTimeSearchState(value)
SearchBar->>SearchBar: performRealTimeSearch (if armed)
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces several key improvements: it enhances the SearchBar component with IME composition support (pausing real-time search during active input) and better whitespace handling; it updates MarkdownRenderer to use valid HTML nesting (replacing block-level divs with inline spans and using a div instead of a p tag when rendering only images); it updates tests to cover these new behaviors; and it cleans up a dependency array in VectorSearchSettings. As there are no review comments, no additional feedback is provided.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/MarkdownRenderer.tsx (1)
838-863: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep the
<p>wrapper for image-only paragraphs
Switching this branch to a<div>bypasses the.prose p > ...image selectors insrc/index.css, so standalone images lose the custom hover/max-width styling. Render the flex classes on<p>instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/MarkdownRenderer.tsx` around lines 838 - 863, The image-only paragraph branch in MarkdownRenderer should keep using the existing p wrapper instead of switching to a div, since the .prose p > ... selectors in src/index.css depend on that structure for image styling. Update the hasImagesOnly render path in MarkdownRenderer so the flex/layout classes are applied directly to the p element while preserving the current children rendering.
🧹 Nitpick comments (1)
src/components/SearchBar.test.tsx (1)
182-193: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMisleading
currentTargetoverride infireEvent.compositionEnd.
@testing-library/dom'sfireEventonly special-cases thetargetproperty to assign onto the real DOM node before dispatch;currentTargetis not overridable this way. When an event is dispatched on an element, the event has the subjected element on a property called target, and if you provide a target property in the eventProperties, those properties will be assigned to the node which is receiving the event. ThecurrentTarget: { value: 'rain' } }passed here is a no-op — the test only passes because the priorfireEvent.changealready set the real input's DOM value to'rain', which is whate.currentTarget.valueactually reads inhandleCompositionEnd.This makes the test's intent misleading and fragile to future edits (e.g., if the preceding
changevalue diverges from this one).♻️ Proposed fix: drop the no-op override, rely on real DOM state instead
- fireEvent.compositionEnd(input, { currentTarget: { value: 'rain' } }); + fireEvent.compositionEnd(input);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/SearchBar.test.tsx` around lines 182 - 193, The composition-end test in SearchBar.test.tsx is relying on a misleading no-op `currentTarget` override in `fireEvent.compositionEnd`, since only the real DOM input value is used by `handleCompositionEnd`. Update the test near the composition event flow to remove the `currentTarget` override and let the event read from the actual input state set by the prior `fireEvent.change`, keeping the assertion aligned with real DOM behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/components/MarkdownRenderer.tsx`:
- Around line 838-863: The image-only paragraph branch in MarkdownRenderer
should keep using the existing p wrapper instead of switching to a div, since
the .prose p > ... selectors in src/index.css depend on that structure for image
styling. Update the hasImagesOnly render path in MarkdownRenderer so the
flex/layout classes are applied directly to the p element while preserving the
current children rendering.
---
Nitpick comments:
In `@src/components/SearchBar.test.tsx`:
- Around line 182-193: The composition-end test in SearchBar.test.tsx is relying
on a misleading no-op `currentTarget` override in `fireEvent.compositionEnd`,
since only the real DOM input value is used by `handleCompositionEnd`. Update
the test near the composition event flow to remove the `currentTarget` override
and let the event read from the actual input state set by the prior
`fireEvent.change`, keeping the assertion aligned with real DOM behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dc337bce-3d20-4a7d-96a9-34875b18ba9e
📒 Files selected for processing (5)
src/components/ForkTimeline.test.tsxsrc/components/MarkdownRenderer.tsxsrc/components/SearchBar.test.tsxsrc/components/SearchBar.tsxsrc/components/settings/VectorSearchSettings.tsx
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 participant
Summary
Testing
Notes
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Fixes #232