-
Notifications
You must be signed in to change notification settings - Fork 139
chore(shadcn): cleanup microphoneTrack #1269
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
base: main
Are you sure you want to change the base?
chore(shadcn): cleanup microphoneTrack #1269
Conversation
|
📝 WalkthroughWalkthroughMigrates microphone track sourcing from individual Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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
Comment |
size-limit report 📦
|
2dd163e to
0a78249
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/storybook/stories/agents-ui/AgentAudioVisualizerGrid.stories.tsx (1)
17-24: Typo:defaultshould besize.Line 18 uses
default: 'lg'which is not a valid prop forAgentAudioVisualizerGrid. Based on theargTypesconfiguration and the component's props interface, this should besize: 'lg'.args: { - default: 'lg', + size: 'lg', state: 'connecting', radius: 5, interval: 100, rowCount: 10, columnCount: 10, },docs/storybook/stories/agents-ui/AgentTrackControl.stories.tsx (1)
101-107: Bug: Microphone control won't receiveaudioTrackdue to incorrect condition.Line 106 checks
args.source === Track.Source.Microphone, butargs.sourceis undefined (nosourcein the story'sargs), while line 104 hardcodessource={Track.Source.Microphone}. This means the condition will always be false andaudioTrackwill beundefined.This is inconsistent with the Default story (line 65) which correctly passes
audioTrack={microphoneTrack}unconditionally for the Microphone control.<AgentTrackControl {...args} kind="audioinput" source={Track.Source.Microphone} pressed={isMicrophonePressed} - audioTrack={args.source === Track.Source.Microphone ? microphoneTrack : undefined} + audioTrack={microphoneTrack} onPressedChange={(pressed: boolean) => setIsMicrophonePressed(pressed)} />
🧹 Nitpick comments (1)
docs/storybook/.storybook/lk-decorators/AgentSessionProvider.tsx (1)
13-16: Missingsessionin useEffect dependency array.The
sessionobject is used inside the effect but not listed in the dependency array. This will trigger an ESLint exhaustive-deps warning. Ifsessionis guaranteed to be stable across renders (memoized byuseSession), the current pattern is functionally correct but consider addingsessionto the array or suppressing the lint rule with a comment explaining the intentional omission.useEffect(() => { session.start(); return () => session.end(); - }, []); + }, [session]);
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
docs/storybook/.storybook/lk-decorators/AgentSessionProvider.tsxdocs/storybook/stories/agents-ui/AgentAudioVisualizerBar.stories.tsxdocs/storybook/stories/agents-ui/AgentAudioVisualizerGrid.stories.tsxdocs/storybook/stories/agents-ui/AgentAudioVisualizerRadial.stories.tsxdocs/storybook/stories/agents-ui/AgentControlBar.stories.tsxdocs/storybook/stories/agents-ui/AgentTrackControl.stories.tsxpackages/shadcn/components/agents-ui/agent-audio-visualizer-radial.tsxpackages/shadcn/components/agents-ui/agent-control-bar.tsxpackages/shadcn/components/agents-ui/agent-track-control.tsxpackages/shadcn/hooks/agents-ui/use-agent-control-bar.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- docs/storybook/stories/agents-ui/AgentAudioVisualizerRadial.stories.tsx
- packages/shadcn/components/agents-ui/agent-control-bar.tsx
- packages/shadcn/hooks/agents-ui/use-agent-control-bar.ts
- packages/shadcn/components/agents-ui/agent-track-control.tsx
- docs/storybook/stories/agents-ui/AgentControlBar.stories.tsx
🧰 Additional context used
🧬 Code graph analysis (3)
docs/storybook/stories/agents-ui/AgentAudioVisualizerGrid.stories.tsx (2)
packages/shadcn/components/agents-ui/agent-audio-visualizer-grid.tsx (2)
AgentAudioVisualizerGrid(239-290)AgentAudioVisualizerGridProps(195-219)docs/storybook/.storybook/lk-decorators/AgentSessionProvider.tsx (1)
AgentSessionProvider(10-23)
docs/storybook/stories/agents-ui/AgentTrackControl.stories.tsx (3)
packages/shadcn/components/agents-ui/agent-track-control.tsx (2)
AgentTrackControl(259-324)AgentTrackControlProps(199-240)docs/storybook/.storybook/lk-decorators/AgentSessionProvider.tsx (1)
AgentSessionProvider(10-23)packages/shadcn/components/agents-ui/agent-session-provider.tsx (1)
AgentSessionProvider(50-61)
docs/storybook/stories/agents-ui/AgentAudioVisualizerBar.stories.tsx (2)
packages/shadcn/components/agents-ui/agent-audio-visualizer-bar.tsx (2)
AgentAudioVisualizerBar(118-196)AgentAudioVisualizerBarProps(71-100)docs/storybook/.storybook/lk-decorators/AgentSessionProvider.tsx (1)
AgentSessionProvider(10-23)
⏰ 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)
docs/storybook/stories/agents-ui/AgentAudioVisualizerBar.stories.tsx (1)
10-15: LGTM!The migration from
useMicrophonetouseSessionContextis clean. The destructuring oflocal.microphoneTrackand passing it asaudioTrackprop aligns with the PR objectives and matches the expected prop types.docs/storybook/stories/agents-ui/AgentAudioVisualizerGrid.stories.tsx (1)
10-15: LGTM!The migration to
useSessionContextfollows the same clean pattern as the other stories.docs/storybook/stories/agents-ui/AgentTrackControl.stories.tsx (1)
11-26: LGTM!The default render function cleanly integrates
useSessionContextformicrophoneTrackanduseTrackTogglefor toggle state management. The conditionalaudioTrackprop based on source type is appropriate here since the source can vary via args.packages/shadcn/components/agents-ui/agent-audio-visualizer-radial.tsx (1)
17-22: Tailwind v4 natively supports these selectors — no issue.The project uses Tailwind CSS v4, which has built-in support for both the
**:descendant combinator variant anddata-*:attribute variants. The selectors**:data-lk-index:...andhas-data-[lk-state=...]:**:data-lk-index:...are valid Tailwind v4 syntax and will not be stripped. Other components in the codebase (tooltip, select, alert, etc.) already usedata-[...]variants successfully. No custom variant configuration or fallback syntax is needed.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Summary
useMicrophonewithuseSessionContextacross Agent storiesuseInputControls/AgentControlBarto useuseSessionContextto retrieve microphoneTrackSummary by CodeRabbit
Refactor
Style
✏️ Tip: You can customize this high-level summary in your review settings.