bugs: disable requiring something to be selected to chat #2814
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
WalkthroughThe disabled condition for the ChatInput and its child components was simplified to depend only on isWaiting, removing the previous dependency on an empty chat context. No other logic for input handling, message sending, or state management was changed. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ChatInput
participant UI as Child Components<br/>(Suggestions, Textarea, Toggle, Actions)
Note over ChatInput: Previous: disabled if isWaiting OR context empty
Note over ChatInput: Now: disabled only if isWaiting
User->>ChatInput: Focus/type
ChatInput->>UI: set disabled = isWaiting
alt isWaiting = true
UI-->>User: Inputs disabled
else isWaiting = false
UI-->>User: Inputs enabled (even with empty context)
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks (2 warnings, 1 inconclusive)❌ Failed checks (2 warnings, 1 inconclusive)
Poem
✨ Finishing Touches
🧪 Generate unit tests
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx (1)
69-84: Enter handling calls handleEnterSelection twice (duplicate action)handleEnterSelection() is invoked in the if condition and again inside the block, potentially applying the suggestion twice.
Apply:
- const handleGlobalKeyDown = (e: KeyboardEvent) => { - if (e.key === 'Enter' && suggestionRef.current?.handleEnterSelection()) { - e.preventDefault(); - e.stopPropagation(); - // Stop the event from bubbling to the canvas - e.stopImmediatePropagation(); - // Handle the suggestion selection - suggestionRef.current.handleEnterSelection(); - } - }; + const handleGlobalKeyDown = (e: KeyboardEvent) => { + if (e.key === 'Enter') { + const handled = suggestionRef.current?.handleEnterSelection?.(); + if (handled) { + e.preventDefault(); + e.stopPropagation(); + e.stopImmediatePropagation(); + } + } + };
🧹 Nitpick comments (2)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx (2)
58-67: Avoid stale isWaiting in focus handlerThe listener closes over isWaiting from mount. Re-register on change to ensure correct gating.
- }, []); + }, [isWaiting]);
135-141: Remove unused variable for claritymessage is assigned but never used.
- const message = chatMode === ChatType.ASK - ? await editorEngine.chat.addAskMessage(savedInput) - : await editorEngine.chat.addEditMessage(savedInput); + if (chatMode === ChatType.ASK) { + await editorEngine.chat.addAskMessage(savedInput); + } else { + await editorEngine.chat.addEditMessage(savedInput); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.tsx: In React providers managing MobX stores, create the observable store with useState(() => new Store()) to ensure a stable instance
Keep a ref (e.g., storeRef.current) to the MobX store to avoid stale closures in effects
Use setTimeout(() => store.clear(), 0) for delayed cleanup of MobX stores to avoid race conditions
Separate project changes from branch updates by using proper effect dependency arrays
Do not use useMemo to hold MobX observable instances; React may drop memoized values causing data loss
Do not clean up MobX stores synchronously during navigation; perform delayed cleanup instead
Do not include the MobX store instance in effect dependency arrays when it causes infinite loops
Files:
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx
🔇 Additional comments (2)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx (2)
86-86: Good change: input no longer blocked by empty contextSetting disabled = isWaiting aligns with the PR goal and keeps UX consistent; the Send button remains guarded by inputEmpty.
327-345: No action needed: Suggestions, ChatModeToggle, and ActionButtons correctly handle empty chat context
All three components use thedisabledflag (decoupled from context) to hide or disable UI as intended, without blocking typing, mode switching, or sending when the chat context is empty.
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Important
Removes requirement for chat context selection to enable chat input in
ChatInputcomponent.ChatInputcomponent.disabledcondition inChatInputnow only checksisWaitingstate.index.tsxto simplifydisabledlogic.This description was created by
for 9fae513. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit