Feat/1958 ai left panel UI elements#1971
Open
DSanich wants to merge 9 commits intofeat/1957-ai-left-panelfrom
Open
Feat/1958 ai left panel UI elements#1971DSanich wants to merge 9 commits intofeat/1957-ai-left-panelfrom
DSanich wants to merge 9 commits intofeat/1957-ai-left-panelfrom
Conversation
Contributor
📝 WalkthroughWalkthroughReplaces the top-level layout with a new AiLeftPanelLayout wrapper that embeds a resizable desktop left AI panel and a Drawer-based mobile panel, moves Footer inside the new layout, adds multiple AI panel subcomponents and mock data, introduces a useIsMobile hook, and adds a Vaul-based Drawer primitive and exports. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AppLayout
participant AiLeftPanelLayout
participant useIsMobile
participant DesktopPanel
participant Drawer
User->>AppLayout: Request page
AppLayout->>AiLeftPanelLayout: Render wrapper
AiLeftPanelLayout->>useIsMobile: evaluate viewport
useIsMobile-->>AiLeftPanelLayout: isMobile
alt Mobile
AiLeftPanelLayout->>Drawer: render AiLeftPanel in Drawer
User->>Drawer: open panel
Drawer->>AiLeftPanelLayout: show panel
else Desktop
AiLeftPanelLayout->>DesktopPanel: render persistent left panel
User->>DesktopPanel: drag resize handle
DesktopPanel->>DesktopPanel: update panelWidth (clamped)
end
User->>AiLeftPanelLayout: interact with content / footer renders inside layout
sequenceDiagram
participant User
participant AiLeftPanel
participant AiPanelHeader
participant AiPanelMessages
participant AiPanelChatBar
User->>AiLeftPanel: mount
AiLeftPanel->>AiPanelHeader: pass selectedModel, handlers
AiLeftPanel->>AiPanelMessages: pass messages, suggestions
AiLeftPanel->>AiPanelChatBar: pass input, onChange, onSend
User->>AiPanelHeader: select model
AiPanelHeader->>AiLeftPanel: onModelSelect
AiLeftPanel->>AiLeftPanel: update selectedModel
User->>AiPanelChatBar: submit message
AiPanelChatBar->>AiLeftPanel: onSend
AiLeftPanel->>AiPanelMessages: append message / re-render
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
packages/epics/src/common/ai-panel/mock-data.ts (1)
23-29: Move sharedMessagetype out ofmock-data.ts.
Messageis now part of component contracts across files, so keeping it in a mock fixture module creates unnecessary coupling. Please extract it into a shared type module (e.g.,ai-panel.types.ts) and import it from both UI components and mock data.Based on learnings: DSanich prefers extracting repeated type declarations into shared modules for reusability and maintainability, particularly for token types that are used across multiple files in the codebase.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/epics/src/common/ai-panel/mock-data.ts` around lines 23 - 29, Extract the Message type from the mock module into a new shared types module (e.g., ai-panel.types.ts): declare and export "Message" there, remove the local declaration from the mock fixture (mock-data.ts), and replace it with an import of Message from the new module; then update all UI components and any other files that previously duplicated the type to import Message from ai-panel.types.ts so there is a single source of truth.apps/web/src/app/layout.tsx (1)
137-142: Remove unnecessary fragment wrapper.The
<>...</>fragment wrapping thedivandFooteris redundant sinceAiLeftPanelLayoutacceptschildrenasReact.ReactNode, which can be multiple elements.♻️ Suggested simplification
<AiLeftPanelLayout> - <> - <div className="w-full shrink-0 pb-8">{children}</div> - <Footer /> - </> + <div className="w-full shrink-0 pb-8">{children}</div> + <Footer /> </AiLeftPanelLayout>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/layout.tsx` around lines 137 - 142, Remove the redundant fragment wrapping the child elements passed to AiLeftPanelLayout: instead of passing <> <div className="w-full shrink-0 pb-8">{children}</div> <Footer /> </>, pass the two elements directly as children to AiLeftPanelLayout (the div and Footer). Update the JSX where AiLeftPanelLayout is used so it contains the div with {children} and the Footer as sibling children without the <>...</> fragment.packages/epics/src/common/ai-left-panel.tsx (1)
23-23: Consider defaulting safely instead of using non-null assertion.Using
!assumesMOCK_MODEL_OPTIONSis never empty. A safer pattern would handle the potential undefined case, though for mock data this is low risk.♻️ Optional safer approach
- const [selectedModel, setSelectedModel] = useState(MOCK_MODEL_OPTIONS[0]!); + const [selectedModel, setSelectedModel] = useState(MOCK_MODEL_OPTIONS[0] ?? MOCK_MODEL_OPTIONS[0]);Or if TypeScript config allows, trust the mock data array will always have at least one element and leave as-is.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/epics/src/common/ai-left-panel.tsx` at line 23, The initialization for selectedModel uses a non-null assertion on MOCK_MODEL_OPTIONS[0]; change it to safely handle an empty array by providing a fallback or allowing undefined: initialize selectedModel with MOCK_MODEL_OPTIONS[0] ?? a sensible default model (or initialize as undefined and update the state type accordingly), and update any code using selectedModel to handle the possible undefined case; refer to the useState call that declares selectedModel and setSelectedModel and the MOCK_MODEL_OPTIONS constant when making the change.packages/epics/src/common/ai-left-panel-layout.tsx (1)
57-96: Remove unnecessary fragment wrapper.The
<>...</>fragment wrapping the desktop panel content is unnecessary since it's the only child in that conditional branch.♻️ Suggested simplification
{!isMobile && ( - <> - <div - className={cn( - 'relative flex h-full flex-shrink-0 flex-col transition-all duration-300', - panelOpen ? '' : 'w-0 overflow-hidden', - )} - style={panelOpen ? { width: panelWidth } : undefined} - > - <AiLeftPanel onClose={() => setPanelOpen(false)} /> - - {panelOpen && ( - <div - role="separator" - ... - > - ... - </div> - )} - </div> - </> + <div + className={cn( + 'relative flex h-full flex-shrink-0 flex-col transition-all duration-300', + panelOpen ? '' : 'w-0 overflow-hidden', + )} + style={panelOpen ? { width: panelWidth } : undefined} + > + <AiLeftPanel onClose={() => setPanelOpen(false)} /> + + {panelOpen && ( + <div + role="separator" + ... + > + ... + </div> + )} + </div> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/epics/src/common/ai-left-panel-layout.tsx` around lines 57 - 96, Remove the unnecessary React fragment wrapping the desktop panel branch: replace the fragment that encloses the top-level div (the element with className built via cn and style using panelWidth) by directly returning that div when !isMobile is true; ensure AiLeftPanel, the conditional panelOpen block (with the separator div using onResizeMouseDown and aria attributes referencing panelWidth), and the isDragging-based classes remain unchanged.packages/epics/src/common/ai-panel/ai-panel-header.tsx (2)
27-39: Consider adding listener only when menu is open.The mousedown listener is added unconditionally and checks
showModelMenuinside the handler. For better performance, the listener could be added/removed based on menu state.♻️ Optional optimization
useEffect(() => { + if (!showModelMenu) return; + const handleClickOutside = (e: MouseEvent) => { - if ( - showModelMenu && - menuRef.current && - !menuRef.current.contains(e.target as Node) - ) { + if (menuRef.current && !menuRef.current.contains(e.target as Node)) { setShowModelMenu(false); } }; document.addEventListener('mousedown', handleClickOutside); return () => document.removeEventListener('mousedown', handleClickOutside); }, [showModelMenu]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/epics/src/common/ai-panel/ai-panel-header.tsx` around lines 27 - 39, Currently the mousedown listener is always attached inside the useEffect and only checks showModelMenu inside the handler; update the effect (the useEffect containing handleClickOutside) to add the document.addEventListener('mousedown', handleClickOutside) only when showModelMenu is true and remove it in the cleanup (and also when showModelMenu flips to false) so the listener is not attached unnecessarily; keep using menuRef and setShowModelMenu inside the handler but ensure the effect's dependency array includes showModelMenu so the listener lifecycle is tied to the menu state.
89-96: Refresh button has no handler.The Refresh button renders but has no
onClickhandler. If this is intentional placeholder behavior, consider adding a TODO comment or disabling the button visually.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/epics/src/common/ai-panel/ai-panel-header.tsx` around lines 89 - 96, The Refresh button in ai-panel-header.tsx renders without an onClick handler (the <button> with title/aria-label "Refresh" and the RefreshCw icon), so either wire it to a real handler or mark it as intentionally inactive: add an onClick that calls a refresh function (e.g., handleRefresh defined in the component or passed as an onRefresh prop) to perform the refresh logic, or if it's a placeholder, add a clear TODO comment and set the button to disabled (and update aria-disabled/visual styles) so users and screen readers know it’s inactive; ensure you update any prop types/interfaces if you add an onRefresh prop and reference the button element and the handler name (handleRefresh or onRefresh) when making the change.
🤖 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/epics/src/common/ai-left-panel-layout.tsx`:
- Around line 74-79: The conditional class string in ai-left-panel-layout.tsx
contains a duplicate Tailwind class ('hover:bg-primary/20') inside the
isDragging false branch; update the ternary in the className (the expression
with isDragging ? 'bg-primary' : 'hover:bg-primary/20 group
hover:bg-primary/20') to remove the duplicate so it only includes each utility
once (e.g., use 'group hover:bg-primary/20' as the false branch).
In `@packages/epics/src/common/ai-panel/ai-panel-chat-bar.tsx`:
- Around line 41-45: The Enter key handler (handleKeyDown) currently calls
onSend even when the input is empty and the Send/Stop button disabling logic
prevents clicking Stop while streaming; update handleKeyDown to only call onSend
when e.key === 'Enter', !e.shiftKey, and the current input (e.g., inputValue or
message) has non-empty trimmed content; change the Send button disabled
expression so it is disabled only when not streaming and the input is empty
(e.g., disabled = !isStreaming && input.trim() === ''), and ensure the Stop
action/button (which calls onStop) is enabled when isStreaming (do not include
isStreaming in the disabled clause for the Stop button).
In `@packages/epics/src/common/ai-panel/ai-panel-message-bubble.tsx`:
- Around line 53-80: The icon-only action buttons in ai-panel-message-bubble.tsx
(the <button> elements that render <Copy />, <ThumbsUp />, <ThumbsDown />, and
<RefreshCw />) currently rely only on title for accessibility; add explicit
aria-label attributes to each button (e.g., aria-label="Copy",
aria-label="Thumbs up", aria-label="Thumbs down", aria-label="Refresh" or more
descriptive variants like "Copy message") to match the visible title and provide
reliable screen-reader support.
In `@packages/ui/package.json`:
- Around line 19-21: Update the vaul dependency entry in package.json: replace
the current "vaul": "^0.9.9" with "vaul": "^1.1.2" so the project can pick up
the 1.x releases; after updating the dependency string run your package manager
install (npm/yarn/pnpm) to refresh lockfiles and verify no breaking changes
affect imports or APIs that reference vaul.
---
Nitpick comments:
In `@apps/web/src/app/layout.tsx`:
- Around line 137-142: Remove the redundant fragment wrapping the child elements
passed to AiLeftPanelLayout: instead of passing <> <div className="w-full
shrink-0 pb-8">{children}</div> <Footer /> </>, pass the two elements directly
as children to AiLeftPanelLayout (the div and Footer). Update the JSX where
AiLeftPanelLayout is used so it contains the div with {children} and the Footer
as sibling children without the <>...</> fragment.
In `@packages/epics/src/common/ai-left-panel-layout.tsx`:
- Around line 57-96: Remove the unnecessary React fragment wrapping the desktop
panel branch: replace the fragment that encloses the top-level div (the element
with className built via cn and style using panelWidth) by directly returning
that div when !isMobile is true; ensure AiLeftPanel, the conditional panelOpen
block (with the separator div using onResizeMouseDown and aria attributes
referencing panelWidth), and the isDragging-based classes remain unchanged.
In `@packages/epics/src/common/ai-left-panel.tsx`:
- Line 23: The initialization for selectedModel uses a non-null assertion on
MOCK_MODEL_OPTIONS[0]; change it to safely handle an empty array by providing a
fallback or allowing undefined: initialize selectedModel with
MOCK_MODEL_OPTIONS[0] ?? a sensible default model (or initialize as undefined
and update the state type accordingly), and update any code using selectedModel
to handle the possible undefined case; refer to the useState call that declares
selectedModel and setSelectedModel and the MOCK_MODEL_OPTIONS constant when
making the change.
In `@packages/epics/src/common/ai-panel/ai-panel-header.tsx`:
- Around line 27-39: Currently the mousedown listener is always attached inside
the useEffect and only checks showModelMenu inside the handler; update the
effect (the useEffect containing handleClickOutside) to add the
document.addEventListener('mousedown', handleClickOutside) only when
showModelMenu is true and remove it in the cleanup (and also when showModelMenu
flips to false) so the listener is not attached unnecessarily; keep using
menuRef and setShowModelMenu inside the handler but ensure the effect's
dependency array includes showModelMenu so the listener lifecycle is tied to the
menu state.
- Around line 89-96: The Refresh button in ai-panel-header.tsx renders without
an onClick handler (the <button> with title/aria-label "Refresh" and the
RefreshCw icon), so either wire it to a real handler or mark it as intentionally
inactive: add an onClick that calls a refresh function (e.g., handleRefresh
defined in the component or passed as an onRefresh prop) to perform the refresh
logic, or if it's a placeholder, add a clear TODO comment and set the button to
disabled (and update aria-disabled/visual styles) so users and screen readers
know it’s inactive; ensure you update any prop types/interfaces if you add an
onRefresh prop and reference the button element and the handler name
(handleRefresh or onRefresh) when making the change.
In `@packages/epics/src/common/ai-panel/mock-data.ts`:
- Around line 23-29: Extract the Message type from the mock module into a new
shared types module (e.g., ai-panel.types.ts): declare and export "Message"
there, remove the local declaration from the mock fixture (mock-data.ts), and
replace it with an import of Message from the new module; then update all UI
components and any other files that previously duplicated the type to import
Message from ai-panel.types.ts so there is a single source of truth.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (17)
apps/web/src/app/layout.tsxpackages/epics/src/common/ai-left-panel-layout.tsxpackages/epics/src/common/ai-left-panel.tsxpackages/epics/src/common/ai-panel/ai-panel-chat-bar.tsxpackages/epics/src/common/ai-panel/ai-panel-header.tsxpackages/epics/src/common/ai-panel/ai-panel-message-bubble.tsxpackages/epics/src/common/ai-panel/ai-panel-messages.tsxpackages/epics/src/common/ai-panel/ai-panel-suggestions.tsxpackages/epics/src/common/ai-panel/index.tspackages/epics/src/common/ai-panel/mock-data.tspackages/epics/src/common/index.tspackages/epics/src/hooks/index.tspackages/epics/src/hooks/use-is-mobile.tsxpackages/ui/package.jsonpackages/ui/src/drawer.tsxpackages/ui/src/index.tspackages/ui/src/organisms/footer.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
packages/epics/src/common/ai-panel/ai-panel-messages.tsx (2)
22-23: Remove placeholder scroll anchor until scroll behavior is implemented.Line 22 and Line 38 keep an
endRefanchor, but there is no effect using it. Either wire auto-scroll behavior now or remove this placeholder to keep the component minimal.Minimal cleanup
-import { useRef } from 'react'; @@ - const endRef = useRef<HTMLDivElement>(null); @@ - <div ref={endRef} />Also applies to: 38-38
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/epics/src/common/ai-panel/ai-panel-messages.tsx` around lines 22 - 23, Remove the unused scroll anchor: delete the const endRef = useRef<HTMLDivElement>(null) declaration and the JSX anchor element that uses ref={endRef} inside the AiPanelMessages component, and also remove the now-unused useRef import to avoid linter warnings; leave a short TODO comment if you want to reintroduce auto-scroll later and run the build to ensure no unused-symbol errors remain.
7-7: Decouple UI typing from the mock-data module.Line 7 imports
Messagefrom./mock-data, which makes this production UI component depend on a mock-focused module boundary. MoveMessageto a sharedtypesmodule and import it from there.Proposed refactor
- import type { Message } from './mock-data'; + import type { Message } from './types';// packages/epics/src/common/ai-panel/types.ts export type Message = { id: string; role: 'user' | 'assistant'; content: string; timestamp: Date; isStreaming?: boolean; };Based on learnings: DSanich prefers extracting repeated type declarations into shared modules for reusability and maintainability, particularly for token types that are used across multiple files in the codebase.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/epics/src/common/ai-panel/ai-panel-messages.tsx` at line 7, The UI component imports the Message type from the mock-data module which couples production UI to mock fixtures; extract the Message type into a shared types module (e.g., export type Message = { id: string; role: 'user'|'assistant'; content: string; timestamp: Date; isStreaming?: boolean }) and update ai-panel-messages.tsx to import Message from that new types module instead of ./mock-data; ensure any other files currently importing Message from mock-data are updated to the new shared types export to keep typings consistent.packages/epics/src/common/ai-panel/ai-panel-header.tsx (2)
51-61: Add explicit menu semantics for assistive tech.The model dropdown works visually, but it is missing ARIA menu semantics (
aria-expanded,aria-haspopup, menu/menuitem roles), which hurts screen-reader clarity.Suggested accessibility refinement
<button type="button" onClick={() => setShowModelMenu(!showModelMenu)} + aria-haspopup="menu" + aria-expanded={showModelMenu} className="flex items-center gap-1.5 rounded-lg border border-border bg-secondary px-2.5 py-1.5 text-xs text-muted-foreground transition-colors hover:bg-muted hover:text-foreground" > @@ - <div className="absolute left-0 top-full z-50 mt-1 w-44 animate-in fade-in slide-in-from-top-2 rounded-xl border border-border bg-popover py-1 shadow-2xl duration-200"> + <div + role="menu" + aria-label="Select model" + className="absolute left-0 top-full z-50 mt-1 w-44 animate-in fade-in slide-in-from-top-2 rounded-xl border border-border bg-popover py-1 shadow-2xl duration-200" + > @@ <button key={m.id} type="button" + role="menuitemradio" + aria-checked={m.id === selectedModel.id} onClick={() => {Also applies to: 65-82
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/epics/src/common/ai-panel/ai-panel-header.tsx` around lines 51 - 61, The model dropdown currently lacks ARIA menu semantics; update the toggle button (the element using onClick and setShowModelMenu, currently rendering SelectedIcon and ChevronDown) to include aria-haspopup="menu" and aria-expanded={showModelMenu} and an id; wrap the conditional menu container (the div rendered when showModelMenu is true) with role="menu" and aria-labelledby pointing to the button id; ensure each selectable entry inside the menu uses role="menuitem" (or menuitemradio/menuitemcheckbox as appropriate) and keyboard focusability so assistive tech can announce and navigate the menu.
27-39: Avoid attaching a document listener when the menu is closed.The outside-click listener can be registered only while
showModelMenuis true to reduce global event work.Suggested optimization
useEffect(() => { + if (!showModelMenu) return; const handleClickOutside = (e: MouseEvent) => { - if ( - showModelMenu && - menuRef.current && - !menuRef.current.contains(e.target as Node) - ) { + if (menuRef.current && !menuRef.current.contains(e.target as Node)) { setShowModelMenu(false); } }; document.addEventListener('mousedown', handleClickOutside); return () => document.removeEventListener('mousedown', handleClickOutside); }, [showModelMenu]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/epics/src/common/ai-panel/ai-panel-header.tsx` around lines 27 - 39, The effect currently always registers the document 'mousedown' listener even when the menu is closed; modify the useEffect (the hook that defines handleClickOutside) to only add the listener when showModelMenu is true (e.g., wrap the addEventListener/removeEventListener logic in an if (showModelMenu) block or return early when false), keep the same cleanup that removes the listener, and continue to reference menuRef and setShowModelMenu inside handleClickOutside to close the menu when a click occurs outside.
🤖 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/epics/src/common/ai-panel/ai-panel-header.tsx`:
- Around line 87-94: The Refresh button in ai-panel-header.tsx is an interactive
control with no handler (the <button> containing <RefreshCw />), so either wire
it to the component's refresh logic or make it non-interactive; add an onClick
prop that calls the existing refresh/refreshState function or a passed-in prop
like onRefresh (or if none exists, expose a new prop and call the parent
handler), ensure accessibility by keeping aria-label and optionally add disabled
when refresh is unavailable, or replace the element with a non-button if it
should be purely decorative.
---
Nitpick comments:
In `@packages/epics/src/common/ai-panel/ai-panel-header.tsx`:
- Around line 51-61: The model dropdown currently lacks ARIA menu semantics;
update the toggle button (the element using onClick and setShowModelMenu,
currently rendering SelectedIcon and ChevronDown) to include
aria-haspopup="menu" and aria-expanded={showModelMenu} and an id; wrap the
conditional menu container (the div rendered when showModelMenu is true) with
role="menu" and aria-labelledby pointing to the button id; ensure each
selectable entry inside the menu uses role="menuitem" (or
menuitemradio/menuitemcheckbox as appropriate) and keyboard focusability so
assistive tech can announce and navigate the menu.
- Around line 27-39: The effect currently always registers the document
'mousedown' listener even when the menu is closed; modify the useEffect (the
hook that defines handleClickOutside) to only add the listener when
showModelMenu is true (e.g., wrap the addEventListener/removeEventListener logic
in an if (showModelMenu) block or return early when false), keep the same
cleanup that removes the listener, and continue to reference menuRef and
setShowModelMenu inside handleClickOutside to close the menu when a click occurs
outside.
In `@packages/epics/src/common/ai-panel/ai-panel-messages.tsx`:
- Around line 22-23: Remove the unused scroll anchor: delete the const endRef =
useRef<HTMLDivElement>(null) declaration and the JSX anchor element that uses
ref={endRef} inside the AiPanelMessages component, and also remove the
now-unused useRef import to avoid linter warnings; leave a short TODO comment if
you want to reintroduce auto-scroll later and run the build to ensure no
unused-symbol errors remain.
- Line 7: The UI component imports the Message type from the mock-data module
which couples production UI to mock fixtures; extract the Message type into a
shared types module (e.g., export type Message = { id: string; role:
'user'|'assistant'; content: string; timestamp: Date; isStreaming?: boolean })
and update ai-panel-messages.tsx to import Message from that new types module
instead of ./mock-data; ensure any other files currently importing Message from
mock-data are updated to the new shared types export to keep typings consistent.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/epics/src/common/ai-panel/ai-panel-header.tsxpackages/epics/src/common/ai-panel/ai-panel-message-bubble.tsxpackages/epics/src/common/ai-panel/ai-panel-messages.tsxpackages/epics/src/common/ai-panel/mock-data.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/epics/src/common/ai-panel/mock-data.ts
- packages/epics/src/common/ai-panel/ai-panel-message-bubble.tsx
Comment on lines
+87
to
+94
| <button | ||
| type="button" | ||
| className="flex h-7 w-7 items-center justify-center rounded-lg text-muted-foreground transition-colors hover:bg-muted hover:text-foreground" | ||
| title="Refresh" | ||
| aria-label="Refresh" | ||
| > | ||
| <RefreshCw className="h-3.5 w-3.5" /> | ||
| </button> |
Contributor
There was a problem hiding this comment.
Refresh is a no-op clickable control.
Line 90-92 presents an actionable refresh button, but there is no handler. This is confusing UX unless intentionally disabled.
Suggested fix
type AiPanelHeaderProps = {
onClose: () => void;
+ onRefresh?: () => void;
modelOptions: ModelOption[];
selectedModel: ModelOption;
onModelSelect: (model: ModelOption) => void;
};
export function AiPanelHeader({
onClose,
+ onRefresh,
modelOptions,
selectedModel,
onModelSelect,
}: AiPanelHeaderProps) {
@@
<button
type="button"
+ onClick={onRefresh}
+ disabled={!onRefresh}
className="flex h-7 w-7 items-center justify-center rounded-lg text-muted-foreground transition-colors hover:bg-muted hover:text-foreground"
title="Refresh"
aria-label="Refresh"
>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/epics/src/common/ai-panel/ai-panel-header.tsx` around lines 87 - 94,
The Refresh button in ai-panel-header.tsx is an interactive control with no
handler (the <button> containing <RefreshCw />), so either wire it to the
component's refresh logic or make it non-interactive; add an onClick prop that
calls the existing refresh/refreshState function or a passed-in prop like
onRefresh (or if none exists, expose a new prop and call the parent handler),
ensure accessibility by keeping aria-label and optionally add disabled when
refresh is unavailable, or replace the element with a non-button if it should be
purely decorative.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
packages/epics/src/common/ai-panel/ai-panel-chat-bar.tsx (1)
41-45:⚠️ Potential issue | 🔴 CriticalStop/send gating is still broken (Stop disabled while streaming; Enter can send empty input).
Line 44 triggers
onSend()even with empty input, and Line 130 disables the same button whileisStreamingis true, so the visible Stop action (Lines 138-142) cannot be clicked.Proposed fix
- const handleKeyDown = (e: React.KeyboardEvent) => { + const handleKeyDown = (e: React.KeyboardEvent) => { if (e.key === 'Enter' && !e.shiftKey) { e.preventDefault(); - onSend(); + if (canSubmit) onSend(); } }; - const canSend = value.trim().length > 0 && !isStreaming; + const hasInput = value.trim().length > 0; + const canSubmit = isStreaming || hasInput; @@ <button type="button" onClick={onSend} - disabled={!canSend} + disabled={!canSubmit} className={cn( 'flex items-center gap-1.5 rounded-xl px-3 py-1.5 text-xs font-medium transition-all duration-200', - canSend + canSubmit ? 'bg-primary text-primary-foreground hover:opacity-90' : 'cursor-not-allowed bg-muted text-muted-foreground', )} >Also applies to: 48-50, 127-149
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/epics/src/common/ai-panel/ai-panel-chat-bar.tsx` around lines 41 - 45, handleKeyDown currently calls onSend() on Enter without validating input and the Stop button is disabled while isStreaming, preventing stopping; update handleKeyDown to check the current input value (same signal/state used by the Send button) and only call onSend() when the trimmed input is non-empty, and ensure the Stop action/button component (the control that checks isStreaming) remains enabled while isStreaming so users can cancel streaming—verify logic in handleKeyDown, onSend, and the button rendering/disabled prop around isStreaming and empty-input checks to keep Send gated by non-empty input and Stop always clickable during streaming.packages/epics/src/common/ai-panel/ai-panel-header.tsx (1)
87-94:⚠️ Potential issue | 🟡 MinorRefresh button is still an interactive no-op.
Line 87 renders a clickable button with no handler, which is misleading unless intentionally disabled/presentational.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/epics/src/common/ai-panel/ai-panel-header.tsx` around lines 87 - 94, The Refresh button rendered in ai-panel-header.tsx is an interactive element with no handler; either wire it to the actual refresh action (add an onClick that calls the existing refresh function/prop—e.g., invoke a passed-in onRefresh prop or call the component's refresh method/dispatcher where the button with the RefreshCw icon is defined) or make it explicitly non-interactive (set disabled and aria-disabled="true" and remove pointer events) so it’s not a misleading clickable control; update the button element accordingly to use onClick={onRefresh} or disabled+aria-disabled as appropriate.packages/epics/src/common/ai-left-panel-layout.tsx (1)
75-79:⚠️ Potential issue | 🟡 MinorDuplicate Tailwind utility in resizer class branch.
Line 78 repeats
hover:bg-primary/20in the same conditional branch.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/epics/src/common/ai-left-panel-layout.tsx` around lines 75 - 79, The resizer class conditional for the left panel contains a duplicated Tailwind utility; in the ternary that uses isDragging (inside the AiLeftPanelLayout component's resizer element) remove the repeated 'hover:bg-primary/20' so the false branch is e.g. 'group hover:bg-primary/20' (or 'hover:bg-primary/20 group') instead of repeating the same utility twice.
🧹 Nitpick comments (2)
packages/epics/src/common/ai-panel/ai-panel-header.tsx (1)
51-61: Add dropdown ARIA state to the model selector trigger.Please expose
aria-haspopup="menu"andaria-expanded={showModelMenu}on the trigger button for better screen-reader context.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/epics/src/common/ai-panel/ai-panel-header.tsx` around lines 51 - 61, The model selector trigger button lacks ARIA attributes; update the button in ai-panel-header.tsx (the element using onClick={() => setShowModelMenu(!showModelMenu)} and rendering SelectedIcon, selectedModel.label, and ChevronDown) to include aria-haspopup="menu" and aria-expanded={showModelMenu} so screen readers know it opens a menu; keep the existing onClick toggle and className unchanged and ensure aria-expanded uses the showModelMenu state variable.packages/epics/src/common/ai-left-panel-layout.tsx (1)
70-73: Resizable separator is mouse-only; add keyboard interaction metadata.For accessibility parity, make the separator focusable and support keyboard resizing (
tabIndex, arrow-key handlers, andaria-valuemin/aria-valuemax).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/epics/src/common/ai-left-panel-layout.tsx` around lines 70 - 73, The separator element currently only supports mouse resizing via onResizeMouseDown and exposes aria-valuenow; make it keyboard-accessible by adding tabIndex={0}, aria-valuemin and aria-valuemax (use the same min/max values used for resizing logic), and a keydown handler (e.g., onResizeKeyDown) that listens for ArrowLeft/ArrowRight (and ArrowUp/ArrowDown if vertical) to adjust panelWidth incrementally and call the same resize logic used by onResizeMouseDown. Locate the separator element (the JSX with role="separator" and aria-valuenow={panelWidth}) and implement a companion onResizeKeyDown function that updates panelWidth within bounds and invokes any existing resize callbacks so keyboard users get parity with mouse interactions.
🤖 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/epics/src/common/ai-left-panel-layout.tsx`:
- Around line 99-104: The open-panel button lacks an accessible name; update the
button element that calls setPanelOpen(true) (the icon-only control in
ai-left-panel-layout's Open AI panel button) to include an explicit accessible
name by adding an aria-label (e.g., aria-label="Open AI panel") or
aria-labelledby pointing to visible text so screen readers can announce its
purpose; ensure the label is concise and matches the existing title if you want
consistency.
In `@packages/epics/src/common/ai-panel/ai-panel-chat-bar.tsx`:
- Around line 31-39: The textarea height can become stale because autoResize
only runs on local typing; add a useEffect that calls the existing autoResize
function whenever the controlled value prop changes (the prop/variable named
value used to render the textarea) so that height shrinks/expands after parent
updates (e.g., after send); ensure you reference the textareaRef and autoResize
functions inside that useEffect and also call autoResize after any programmatic
value resets (where send/clear logic runs) to keep behavior consistent.
---
Duplicate comments:
In `@packages/epics/src/common/ai-left-panel-layout.tsx`:
- Around line 75-79: The resizer class conditional for the left panel contains a
duplicated Tailwind utility; in the ternary that uses isDragging (inside the
AiLeftPanelLayout component's resizer element) remove the repeated
'hover:bg-primary/20' so the false branch is e.g. 'group hover:bg-primary/20'
(or 'hover:bg-primary/20 group') instead of repeating the same utility twice.
In `@packages/epics/src/common/ai-panel/ai-panel-chat-bar.tsx`:
- Around line 41-45: handleKeyDown currently calls onSend() on Enter without
validating input and the Stop button is disabled while isStreaming, preventing
stopping; update handleKeyDown to check the current input value (same
signal/state used by the Send button) and only call onSend() when the trimmed
input is non-empty, and ensure the Stop action/button component (the control
that checks isStreaming) remains enabled while isStreaming so users can cancel
streaming—verify logic in handleKeyDown, onSend, and the button
rendering/disabled prop around isStreaming and empty-input checks to keep Send
gated by non-empty input and Stop always clickable during streaming.
In `@packages/epics/src/common/ai-panel/ai-panel-header.tsx`:
- Around line 87-94: The Refresh button rendered in ai-panel-header.tsx is an
interactive element with no handler; either wire it to the actual refresh action
(add an onClick that calls the existing refresh function/prop—e.g., invoke a
passed-in onRefresh prop or call the component's refresh method/dispatcher where
the button with the RefreshCw icon is defined) or make it explicitly
non-interactive (set disabled and aria-disabled="true" and remove pointer
events) so it’s not a misleading clickable control; update the button element
accordingly to use onClick={onRefresh} or disabled+aria-disabled as appropriate.
---
Nitpick comments:
In `@packages/epics/src/common/ai-left-panel-layout.tsx`:
- Around line 70-73: The separator element currently only supports mouse
resizing via onResizeMouseDown and exposes aria-valuenow; make it
keyboard-accessible by adding tabIndex={0}, aria-valuemin and aria-valuemax (use
the same min/max values used for resizing logic), and a keydown handler (e.g.,
onResizeKeyDown) that listens for ArrowLeft/ArrowRight (and ArrowUp/ArrowDown if
vertical) to adjust panelWidth incrementally and call the same resize logic used
by onResizeMouseDown. Locate the separator element (the JSX with
role="separator" and aria-valuenow={panelWidth}) and implement a companion
onResizeKeyDown function that updates panelWidth within bounds and invokes any
existing resize callbacks so keyboard users get parity with mouse interactions.
In `@packages/epics/src/common/ai-panel/ai-panel-header.tsx`:
- Around line 51-61: The model selector trigger button lacks ARIA attributes;
update the button in ai-panel-header.tsx (the element using onClick={() =>
setShowModelMenu(!showModelMenu)} and rendering SelectedIcon,
selectedModel.label, and ChevronDown) to include aria-haspopup="menu" and
aria-expanded={showModelMenu} so screen readers know it opens a menu; keep the
existing onClick toggle and className unchanged and ensure aria-expanded uses
the showModelMenu state variable.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/epics/src/common/ai-left-panel-layout.tsxpackages/epics/src/common/ai-left-panel.tsxpackages/epics/src/common/ai-panel/ai-panel-chat-bar.tsxpackages/epics/src/common/ai-panel/ai-panel-header.tsxpackages/epics/src/common/ai-panel/ai-panel-messages.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/epics/src/common/ai-panel/ai-panel-messages.tsx
- packages/epics/src/common/ai-left-panel.tsx
Comment on lines
+99
to
+104
| <button | ||
| type="button" | ||
| onClick={() => setPanelOpen(true)} | ||
| className="fixed left-0 top-[4.5rem] z-30 flex items-center gap-1.5 rounded-r-xl border border-l-0 border-border bg-card px-2 py-1.5 text-xs text-muted-foreground transition-all duration-200 hover:bg-muted hover:text-foreground" | ||
| title="Open AI panel" | ||
| > |
Contributor
There was a problem hiding this comment.
Reopen button should have an explicit accessible name.
This icon-only control lacks aria-label, so assistive tech may not announce intent reliably.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/epics/src/common/ai-left-panel-layout.tsx` around lines 99 - 104,
The open-panel button lacks an accessible name; update the button element that
calls setPanelOpen(true) (the icon-only control in ai-left-panel-layout's Open
AI panel button) to include an explicit accessible name by adding an aria-label
(e.g., aria-label="Open AI panel") or aria-labelledby pointing to visible text
so screen readers can announce its purpose; ensure the label is concise and
matches the existing title if you want consistency.
Comment on lines
+31
to
+39
| const textareaRef = useRef<HTMLTextAreaElement>(null); | ||
|
|
||
| const autoResize = () => { | ||
| if (textareaRef.current) { | ||
| textareaRef.current.style.height = 'auto'; | ||
| textareaRef.current.style.height = | ||
| Math.min(textareaRef.current.scrollHeight, 160) + 'px'; | ||
| } | ||
| }; |
Contributor
There was a problem hiding this comment.
Textarea height can get stale after controlled value updates.
autoResize() only runs on local typing. If parent state clears or replaces value (e.g., after send), height may not shrink until next keystroke.
Proposed fix
-import { useRef } from 'react';
+import { useEffect, useRef } from 'react';
@@
const autoResize = () => {
@@
};
+
+ useEffect(() => {
+ autoResize();
+ }, [value]);Also applies to: 107-110, 119-120
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/epics/src/common/ai-panel/ai-panel-chat-bar.tsx` around lines 31 -
39, The textarea height can become stale because autoResize only runs on local
typing; add a useEffect that calls the existing autoResize function whenever the
controlled value prop changes (the prop/variable named value used to render the
textarea) so that height shrinks/expands after parent updates (e.g., after
send); ensure you reference the textareaRef and autoResize functions inside that
useEffect and also call autoResize after any programmatic value resets (where
send/clear logic runs) to keep behavior consistent.
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
1 participant
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.
Summary by CodeRabbit