-
Notifications
You must be signed in to change notification settings - Fork 139
fix(shadcn): agents ui updates #1273
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
📝 WalkthroughWalkthroughDefaults and prop names updated across agents-ui visualizers and controls: visualizer size defaults changed from 'md' → 'lg', Wave's smoothing prop renamed to blur (with shader uniform updated), storybook/docs adjusted, Grid/Radial styles tweaked, AgentControlBar send flow refactored, and AgentTrackToggle gained size variants. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🧬 Code graph analysis (1)packages/shadcn/components/agents-ui/agent-control-bar.tsx (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (9)
✏️ Tip: You can disable this entire section by setting Comment |
size-limit report 📦
|
| interval: 100, | ||
| rowCount: 10, | ||
| columnCount: 10, | ||
| rowCount: 9, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
odd numbers ensure s we can have a center row/column
| export const AgentAudioVisualizerRadialVariants = cva( | ||
| [ | ||
| 'relative flex items-center justify-center', | ||
| '[&_[data-lk-index]]:bg-current/10', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ensures that dots are visible in every agent state
71d6426 to
87138ac
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/shadcn/components/agents-ui/agent-control-bar.tsx (1)
74-99: Prevent empty/duplicate sends from the Enter key path.
handleKeyDownalways callshandleSend, andhandleSenddoesn’t guard against empty or in‑flight sends. This bypassesisDisabled, so pressing Enter can send empty messages or double‑send whileisSendingis true.🐛 Proposed fix
- const handleSend = async () => { + const handleSend = async () => { + const trimmed = message.trim(); + if (isSending || trimmed.length === 0) return; try { setIsSending(true); - await onSend(message); + await onSend(trimmed); setMessage(''); } catch (error) { console.error(error); } finally { setIsSending(false); } };packages/shadcn/components/agents-ui/agent-audio-visualizer-wave.tsx (1)
267-275: Update the JSDoc example to useblur.The example still uses
smoothing, which no longer exists.✏️ Suggested doc fix
* <AgentAudioVisualizerWave * size="lg" * state="speaking" * color="#1FD5F9" * lineWidth={2} - * smoothing={0.5} + * blur={0.5} * audioTrack={audioTrack} * />
🤖 Fix all issues with AI agents
In `@packages/shadcn/components/agents-ui/agent-audio-visualizer-wave.tsx`:
- Around line 246-249: Update the JSDoc example for the AgentAudioVisualizerWave
component to use the renamed prop "blur" instead of the old "smoothing" prop:
locate the JSDoc block that shows the example usage of AgentAudioVisualizerWave
and replace smoothing={0.5} with blur={0.5} so the example matches the
component's current API (AgentAudioVisualizerWave, prop blur).
🧹 Nitpick comments (2)
packages/shadcn/components/agents-ui/agent-audio-visualizer-radial.tsx (1)
19-22: Remove the duplicatedduration-300utility.Line 22 repeats the same class token twice, which is redundant. Consider removing the duplicate for clarity.
♻️ Suggested cleanup
- 'has-data-[lk-state=listening]:[&_[data-lk-index]]:duration-300 has-data-[lk-state=listening]:[&_[data-lk-index]]:duration-300', + 'has-data-[lk-state=listening]:[&_[data-lk-index]]:duration-300',packages/shadcn/components/agents-ui/agent-audio-visualizer-wave.tsx (1)
224-230: Align variant defaults with the new documented default.The JSDoc now says the default is
'lg', butAgentAudioVisualizerWaveVariantsstill defaults to'md'(Line 220). Consider aligning these to avoid confusion for consumers using the variants directly.♻️ Suggested update
defaultVariants: { - size: 'md', + size: 'lg', },
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
README.mddocs/storybook/stories/agents-ui/AgentAudioVisualizerGrid.stories.tsxdocs/storybook/stories/agents-ui/AgentAudioVisualizerWave.stories.tsxpackages/shadcn/README.mdpackages/shadcn/components/agents-ui/agent-audio-visualizer-grid.tsxpackages/shadcn/components/agents-ui/agent-audio-visualizer-radial.tsxpackages/shadcn/components/agents-ui/agent-audio-visualizer-wave.tsxpackages/shadcn/components/agents-ui/agent-control-bar.tsxpackages/shadcn/components/agents-ui/agent-track-toggle.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
docs/storybook/stories/agents-ui/AgentAudioVisualizerGrid.stories.tsx (3)
docs/storybook/stories/agents-ui/AgentAudioVisualizerWave.stories.tsx (1)
Default(61-63)docs/storybook/stories/agents-ui/AgentAudioVisualizerRadial.stories.tsx (1)
Default(57-59)packages/shadcn/components/agents-ui/agent-audio-visualizer-grid.tsx (1)
AgentAudioVisualizerGridProps(195-219)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (4)
packages/shadcn/components/agents-ui/agent-audio-visualizer-wave.tsx (1)
167-193: Blur prop wiring looks consistent.Defaulting
blurto 0.5 and mapping it into the shader uniforms is clear and consistent with the new prop name.docs/storybook/stories/agents-ui/AgentAudioVisualizerGrid.stories.tsx (2)
17-24: Story args update looks good.Switching to
size: 'lg'and using a 9x9 grid is consistent with the updated visuals.
66-70: Default story export looks good.Matches the pattern used by other visualizer stories.
docs/storybook/stories/agents-ui/AgentAudioVisualizerWave.stories.tsx (1)
17-48: Storybook args now match theblurAPI.The args and controls line up with the renamed prop.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
packages/shadcn/components/agents-ui/agent-audio-visualizer-wave.tsx (1)
219-221: CVAdefaultVariantsis inconsistent with the component's actual default.The CVA definition at line 220 specifies
size: 'md'as the default variant, but the component function (line 279) defaults tosize = 'lg', and the JSDoc (line 227) documents'lg'as the default.While the component works correctly because the function parameter default takes precedence, this inconsistency could confuse consumers who use the CVA variants directly.
🔧 Suggested fix
defaultVariants: { - size: 'md', + size: 'lg', },Also applies to: 227-229
♻️ Duplicate comments (1)
packages/shadcn/components/agents-ui/agent-audio-visualizer-wave.tsx (1)
246-249: LGTM! Past review feedback addressed.The JSDoc example at line 273 now correctly uses
blur={0.5}instead of the oldsmoothingprop, resolving the issue flagged in the previous review.Also applies to: 273-273
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/shadcn/components/agents-ui/agent-audio-visualizer-wave.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (3)
packages/shadcn/components/agents-ui/agent-audio-visualizer-wave.tsx (3)
161-164: LGTM!The
smoothing→blurrename is correctly applied in both the interface definition and the destructuring with the preserved default value.Also applies to: 174-174
192-192: LGTM!The uniform binding correctly passes the renamed
blurprop value to the shader's internaluSmoothinguniform.
283-283: LGTM!The
blurprop is correctly destructured and passed through to theWaveShadercomponent.Also applies to: 318-318
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/shadcn/components/agents-ui/agent-control-bar.tsx (1)
74-90: Prevent draft loss while send is in-flight.
WhileisSendingis true, the textarea remains editable butsetMessage('')clears the input on success. That can wipe new text typed during the send. Easiest fix: disable the textarea while sending (or preserve draft on change).✅ Suggested fix (disable input while sending)
- disabled={!chatOpen} + disabled={!chatOpen || isSending}Also applies to: 114-123
🤖 Fix all issues with AI agents
In `@packages/shadcn/components/agents-ui/agent-control-bar.tsx`:
- Around line 92-97: The Enter-to-send handler (handleKeyDown) needs to ignore
Enter events during IME composition; update handleKeyDown to first check the
native event's isComposing flag (e.nativeEvent.isComposing) and return early if
true, so that when e.key === 'Enter' and !e.shiftKey you only call
e.preventDefault() and handleSend() when not composing. Ensure you reference the
handleKeyDown function and the React.KeyboardEvent<HTMLTextAreaElement>
nativeEvent.isComposing property in your change.
🧹 Nitpick comments (1)
packages/shadcn/components/agents-ui/agent-audio-visualizer-wave.tsx (1)
161-164: Confirm public API break forsmoothing→blur.If this component is public, the rename is a breaking change. Consider a temporary
smoothingalias (with deprecation) or confirm a major version bump/migration note.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/shadcn/components/agents-ui/agent-audio-visualizer-wave.tsxpackages/shadcn/components/agents-ui/agent-control-bar.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
packages/shadcn/components/agents-ui/agent-control-bar.tsx (1)
packages/shadcn/components/ui/button.tsx (1)
Button(57-57)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (6)
packages/shadcn/components/agents-ui/agent-audio-visualizer-wave.tsx (5)
167-193: Blur defaults are wired consistently.Defaulting
blurand passing it into the shader uniforms looks coherent.
209-221: Size variants & default updated cleanly.Variant map and the new default size align well.
224-249: Prop docs match new defaults and blur naming.JSDoc updates reflect the new default size and blur prop correctly.
260-276: Example updated toblur.Usage snippet is now consistent with the renamed prop.
278-318: Blur is threaded through to the shader.Prop wiring from
AgentAudioVisualizerWaveintoWaveShaderlooks correct.packages/shadcn/components/agents-ui/agent-control-bar.tsx (1)
99-102: LGTM — explicit send handler +type="button"wiring.
Cleaner flow and avoids implicit form submit behavior.Also applies to: 124-135
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| const handleKeyDown = async (e: React.KeyboardEvent<HTMLTextAreaElement>) => { | ||
| if (e.key === 'Enter' && !e.shiftKey) { | ||
| e.preventDefault(); | ||
| handleSend(); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Skip Enter-to-send during IME composition.
Keydown should ignore Enter while the user is composing (CJK/IME), otherwise partial text can be sent.
✅ Suggested fix
- if (e.key === 'Enter' && !e.shiftKey) {
+ if (e.key === 'Enter' && !e.shiftKey && !e.nativeEvent.isComposing) {
e.preventDefault();
handleSend();
}🤖 Prompt for AI Agents
In `@packages/shadcn/components/agents-ui/agent-control-bar.tsx` around lines 92 -
97, The Enter-to-send handler (handleKeyDown) needs to ignore Enter events
during IME composition; update handleKeyDown to first check the native event's
isComposing flag (e.nativeEvent.isComposing) and return early if true, so that
when e.key === 'Enter' and !e.shiftKey you only call e.preventDefault() and
handleSend() when not composing. Ensure you reference the handleKeyDown function
and the React.KeyboardEvent<HTMLTextAreaElement> nativeEvent.isComposing
property in your change.
a314873 to
6608457
Compare
Summary by CodeRabbit
Features
Bug Fixes
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.