fix(chat): enable scrolling in input field and fix height reset#1143
Merged
fbricon merged 1 commit intokortex-hub:mainfrom Mar 23, 2026
Merged
fix(chat): enable scrolling in input field and fix height reset#1143fbricon merged 1 commit intokortex-hub:mainfrom
fbricon merged 1 commit intokortex-hub:mainfrom
Conversation
fbricon
requested review from
benoitf and
gastoner
and removed request for
a team
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughTextarea autosize and edit/submit/cancel flows were refactored: height now uses scrollHeight capped to 25% viewport; cancel/submit assign Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/renderer/src/lib/chat/components/multimodal-input.svelte (1)
62-79:⚠️ Potential issue | 🟠 MajorUnify the submit guard across keyboard and click paths.
Right now the disabled state only exists on the send button, but
Enterstill callssubmitForm()directly. That lets one path submit drafts the other path blocks. Please move the predicate into a shared helper and reuse it in both places.🔧 Suggested fix
+function canSubmit(): boolean { + return input.trim().length > 0; +} + async function submitForm(): Promise<void> { + if (!canSubmit()) { + return; + } + const text = input; // Clear input directly instead of using setInput('') to avoid calling adjustHeight() before DOM updates input = ''; @@ if (loading) { toast.error('Please wait for the model to finish its response!'); - } else { + } else if (canSubmit()) { await submitForm(); } } @@ - disabled={input.trim().length === 0} + disabled={!canSubmit()}Also applies to: 153-160, 196-204
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/renderer/src/lib/chat/components/multimodal-input.svelte` around lines 62 - 79, Create a single shared predicate (e.g., canSubmit or isSubmitDisabled) and use it everywhere the UI currently decides whether to send: call it at the top of submitForm(), use it to set the send button's disabled attribute, and call it in the Enter/keyboard handler before invoking submitForm(); ensure the predicate checks the same conditions used by the button (empty input, ongoing send state, or invalid attachments) so both click and keyboard paths behave identically — update references to submitForm, the send button disabled binding, and the keyboard handler (e.g., onKeyDown/onKeyPress) to reuse this helper; also apply the same change to the other submit locations noted (the blocks around lines 153-160 and 196-204).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/renderer/src/lib/chat/components/multimodal-input.svelte`:
- Around line 168-178: The attachment Button (the Button component wrapping
PaperclipIcon) is an icon-only control and needs an accessible name; add an
aria-label (e.g., "Attach file") to the Button element that invokes handleFile
to ensure screen readers announce it; update the Button props next to
onclick/disabled/variant (the same element that renders PaperclipIcon and calls
handleFile) to include aria-label="Attach file" (or a localized string) so
accessibility is satisfied.
- Around line 183-191: The handler calls a nonexistent standalone stop()
function; replace that call with the chatClient.stop() method so the generation
abort uses the chat client API (update the onclick callback where stop() is
invoked to call chatClient.stop() and keep the surrounding
event.preventDefault() and chatClient.messages reassignment intact).
---
Outside diff comments:
In `@packages/renderer/src/lib/chat/components/multimodal-input.svelte`:
- Around line 62-79: Create a single shared predicate (e.g., canSubmit or
isSubmitDisabled) and use it everywhere the UI currently decides whether to
send: call it at the top of submitForm(), use it to set the send button's
disabled attribute, and call it in the Enter/keyboard handler before invoking
submitForm(); ensure the predicate checks the same conditions used by the button
(empty input, ongoing send state, or invalid attachments) so both click and
keyboard paths behave identically — update references to submitForm, the send
button disabled binding, and the keyboard handler (e.g., onKeyDown/onKeyPress)
to reuse this helper; also apply the same change to the other submit locations
noted (the blocks around lines 153-160 and 196-204).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d746d38d-e5da-4494-9e92-154e1f329107
📒 Files selected for processing (1)
packages/renderer/src/lib/chat/components/multimodal-input.svelte
packages/renderer/src/lib/chat/components/multimodal-input.svelte
Outdated
Show resolved
Hide resolved
packages/renderer/src/lib/chat/components/multimodal-input.svelte
Outdated
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
benoitf
approved these changes
Mar 23, 2026
fbricon
force-pushed
the
GH-662
branch
2 times, most recently
from
f95909a to
ef28038
Compare
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)
packages/renderer/src/lib/chat/components/multimodal-input.svelte (1)
94-145:⚠️ Potential issue | 🟠 MajorMirror the disabled-state check inside
submitForm().Line 233 and Line 285 both invoke
submitForm()directly, but the "cannot submit empty content" rule only exists on Line 287. Pressing Enter with an empty or whitespace-only draft still reaches this function. In edit mode that also meanswindow.inferenceDeleteTrailingMessages(...)can run even though the send button is disabled.🛠️ Minimal fix
async function submitForm(): Promise<void> { const text = input; + if (text.trim().length === 0) { + return; + } if (editState.editingMessage) {If attachment-only sends are meant to be valid, extract a shared
canSubmitpredicate and use it both here and on Line 287 instead of hardcodingtrim()in only one path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/renderer/src/lib/chat/components/multimodal-input.svelte` around lines 94 - 145, The submitForm() function currently accepts empty/whitespace input even though the UI disables the send button; extract a shared predicate (e.g., canSubmit(input, attachments)) and use it both where the send button is disabled and at the top of submitForm(); ensure canSubmit mirrors the UI rule (allow attachment-only sends if intended) and return early from submitForm() before doing editState.editingMessage work or calling window.inferenceDeleteTrailingMessages, chatClient.sendMessage, or modifying attachments when canSubmit is false.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/renderer/src/lib/chat/components/multimodal-input.svelte`:
- Around line 94-145: The submitForm() function currently accepts
empty/whitespace input even though the UI disables the send button; extract a
shared predicate (e.g., canSubmit(input, attachments)) and use it both where the
send button is disabled and at the top of submitForm(); ensure canSubmit mirrors
the UI rule (allow attachment-only sends if intended) and return early from
submitForm() before doing editState.editingMessage work or calling
window.inferenceDeleteTrailingMessages, chatClient.sendMessage, or modifying
attachments when canSubmit is false.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6c97346c-d4ba-4539-bf4b-55f796e4858b
📒 Files selected for processing (2)
packages/renderer/src/lib/chat/components/messages/preview-message.sveltepackages/renderer/src/lib/chat/components/multimodal-input.svelte
fbricon
force-pushed
the
GH-662
branch
3 times, most recently
from
cb479e5 to
ffe2128
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/renderer/src/lib/chat/components/multimodal-input.svelte`:
- Around line 94-96: canSubmit() currently allows submission when attachments
exist even during an edit, but edits discard attachments and can therefore
enable an empty edit submit; update canSubmit() to respect editState.isEditing
by requiring non-empty trimmed input when editState.isEditing is true (i.e.,
only allow attachments to enable submit when not editing). Apply the same change
to other similar checks referenced around the 104-123 and 284-293 blocks so any
place that gates Send uses attachments, ensure those branches check
editState.isEditing and require input.trim().length > 0 when editing.
- Around line 194-208: The Escape handler is currently bound to window so any
Escape anywhere cancels editing; scope it to the active composer by either
attaching the keydown listener to the composer root element or guarding the
handler to only act when the event target is inside the composer. Update the
$effect block that registers handleKeyDown (and references editState.isEditing
and cancelEditing) so the listener is added to the composer DOM node (e.g.,
composerEl.addEventListener('keydown', handleKeyDown)) or, if keeping window,
start the handler with a check like if (!composerEl.contains(event.target as
Node)) return; then keep the existing Escape logic and cleanup via
removeEventListener.
- Line 236: Replace the Tailwind v3 important syntax in the textarea class
inside the multimodal-input.svelte component: locate the class attribute that
currently contains "!text-base" and change it to the Tailwind v4 trailing form
"text-base!" so the utility takes precedence (keep the rest of the class string
intact, e.g., the max-h, min-h, resize-none, overflow-y-auto, border-0,
bg-transparent, shadow-none, focus-visible:ring-0 utilities).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d1af52db-a695-463c-bec1-fa4ec28b407e
📒 Files selected for processing (2)
packages/renderer/src/lib/chat/components/messages/preview-message.sveltepackages/renderer/src/lib/chat/components/multimodal-input.svelte
Comment on lines
+194
to
+208
| $effect((): (() => void) | void => { | ||
| if (editState.isEditing) { | ||
| const handleKeyDown = (event: KeyboardEvent): void => { | ||
| if (event.key === 'Escape') { | ||
| event.preventDefault(); | ||
| cancelEditing().catch(console.error); | ||
| } | ||
| }; | ||
|
|
||
| window.addEventListener('keydown', handleKeyDown); | ||
| return (): void => { | ||
| window.removeEventListener('keydown', handleKeyDown); | ||
| }; | ||
| } | ||
| }); |
There was a problem hiding this comment.
Scope the Escape shortcut to the active composer.
Binding this on window means every Escape press cancels the edit while edit mode is open. If the user hits Escape to dismiss some other popover/dialog, the current edit is discarded too.
🔧 Possible guard
if (editState.isEditing) {
const handleKeyDown = (event: KeyboardEvent): void => {
- if (event.key === 'Escape') {
+ if (
+ event.key === 'Escape' &&
+ !event.defaultPrevented &&
+ document.activeElement === textareaRef
+ ) {
event.preventDefault();
cancelEditing().catch(console.error);
}
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| $effect((): (() => void) | void => { | |
| if (editState.isEditing) { | |
| const handleKeyDown = (event: KeyboardEvent): void => { | |
| if (event.key === 'Escape') { | |
| event.preventDefault(); | |
| cancelEditing().catch(console.error); | |
| } | |
| }; | |
| window.addEventListener('keydown', handleKeyDown); | |
| return (): void => { | |
| window.removeEventListener('keydown', handleKeyDown); | |
| }; | |
| } | |
| }); | |
| $effect((): (() => void) | void => { | |
| if (editState.isEditing) { | |
| const handleKeyDown = (event: KeyboardEvent): void => { | |
| if ( | |
| event.key === 'Escape' && | |
| !event.defaultPrevented && | |
| document.activeElement === textareaRef | |
| ) { | |
| event.preventDefault(); | |
| cancelEditing().catch(console.error); | |
| } | |
| }; | |
| window.addEventListener('keydown', handleKeyDown); | |
| return (): void => { | |
| window.removeEventListener('keydown', handleKeyDown); | |
| }; | |
| } | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/renderer/src/lib/chat/components/multimodal-input.svelte` around
lines 194 - 208, The Escape handler is currently bound to window so any Escape
anywhere cancels editing; scope it to the active composer by either attaching
the keydown listener to the composer root element or guarding the handler to
only act when the event target is inside the composer. Update the $effect block
that registers handleKeyDown (and references editState.isEditing and
cancelEditing) so the listener is added to the composer DOM node (e.g.,
composerEl.addEventListener('keydown', handleKeyDown)) or, if keeping window,
start the handler with a check like if (!composerEl.contains(event.target as
Node)) return; then keep the existing Escape logic and cleanup via
removeEventListener.
packages/renderer/src/lib/chat/components/multimodal-input.svelte
Outdated
Show resolved
Hide resolved
Resolves issue where long messages couldn't be scrolled or reviewed. The input field now becomes scrollable when text exceeds 25% of viewport height. Restructured layout to keep buttons in a separate row within the chat box container, preventing text overlap when scrolling. Fixes kortex-hub#662 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Fred Bricon <fbricon@gmail.com>
benoitf
approved these changes
Mar 23, 2026
This was referenced Mar 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves issue where long messages couldn't be scrolled or reviewed.
The input field now becomes scrollable when text exceeds 25% of viewport
height. Restructured layout to keep buttons in a separate row within the
chat box container, preventing text overlap when scrolling.
limit-message-box-height.mp4
Fixes #662