chore(shadcn): cleanup microphoneTrack #1269
Conversation
|
📝 WalkthroughWalkthroughMoves microphone sourcing from story-level Changes
Sequence Diagram(s)(Skipped — changes are refactors and story/hook updates without new multi-component control-flow features.) 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
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (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)
✏️ Tip: You can disable this entire section by setting Comment |
size-limit report 📦
|
thomasyuill-livekit
force-pushed
the
ty/shadcn-cleanup-microphoneTrack-creation
branch
from
2dd163e to
0a78249
Compare
![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 (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.
thomasyuill-livekit
force-pushed
the
ty/shadcn-cleanup-microphoneTrack-creation
branch
from
0a78249 to
53669a3
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/shadcn/components/agents-ui/agent-control-bar.tsx`:
- Around line 24-32: The LK_TOGGLE_VARIANT_1 constant contains malformed
Tailwind arbitrary-variant strings (patterns like `data-[state=off]_~_button]`,
`dark:data-[state=off]_~_button]:bg-accent`,
`dark:data-[state=off]_~_button:hover]`) which Tailwind will ignore; fix by
replacing those entries in LK_TOGGLE_VARIANT_1 with valid arbitrary-variant
syntax that targets sibling buttons (for example use `[&_~_button]:bg-accent`,
`[&_~_button]:border-border`, `[&_~_button]:hover:bg-foreground/10` and their
dark/hover/focus variants as needed) so all selectors are balanced and conform
to Tailwind v4 arbitrary-variant rules.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/storybook/stories/agents-ui/AgentAudioVisualizerAura.stories.tsxdocs/storybook/stories/agents-ui/AgentAudioVisualizerWave.stories.tsxpackages/shadcn/components/agents-ui/agent-audio-visualizer-grid.tsxpackages/shadcn/components/agents-ui/agent-control-bar.tsxpackages/shadcn/components/agents-ui/react-shader-toy.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
docs/storybook/stories/agents-ui/AgentAudioVisualizerWave.stories.tsx (1)
packages/shadcn/components/agents-ui/agent-audio-visualizer-wave.tsx (2)
AgentAudioVisualizerWave(278-329)AgentAudioVisualizerWaveProps(224-258)
packages/shadcn/components/agents-ui/agent-control-bar.tsx (1)
packages/shadcn/lib/utils.ts (1)
cn(4-6)
⏰ 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 (19)
docs/storybook/stories/agents-ui/AgentAudioVisualizerAura.stories.tsx (2)
3-6: LGTM!The import changes correctly replace
useMicrophonewithuseSessionContextfrom@livekit/components-react, aligning with the PR objective to centralize microphone track retrieval via session context.
12-19: LGTM!The migration from
useMicrophonetouseSessionContextis correctly implemented. The nested destructuring to extractmicrophoneTrackfromlocalworks well here since theAgentSessionProviderdecorator ensures the session context is properly initialized before the render function executes.docs/storybook/stories/agents-ui/AgentAudioVisualizerWave.stories.tsx (1)
10-15: Guard against missinglocalin session context.If
useSessionContext()can ever returnlocalasundefined(e.g., before connection or if a provider is missing), the destructuring on Line 11 will throw. Consider a null-safe access unless the hook explicitly guaranteeslocalis always defined.✅ Suggested safe access
- const { - local: { microphoneTrack }, - } = useSessionContext(); + const { local } = useSessionContext(); + const microphoneTrack = local?.microphoneTrack;packages/shadcn/components/agents-ui/react-shader-toy.tsx (9)
1-25: LGTM!License block reformatted cleanly with updated copyright year.
260-262: LGTM!Good API simplification—
loadnow accepts configuration params directly rather than requiring a pre-constructed object.
364-370: LGTM!Clean normalization of pointer coordinates with proper null safety for touch events.
492-492: LGTM!Correct type—the ref stores
Textureinstances, not rawWebGLTexturehandles.
511-522: LGTM!Good use of type discrimination via
'width' in textureto differentiate betweenTextureParams(pre-load, no dimensions) andTexture(post-load, has dimensions). The early return guard on line 515 is a nice defensive addition.
561-567: LGTM!Good defensive guard—prevents potential runtime errors when
iMouseuniform isn't configured.
700-702: LGTM!Proper two-phase dimension setup: initial call with placeholder dimensions, then post-load call with actual texture dimensions.
820-820: LGTM!
gl.TEXTURE0 + indexis the correct and idiomatic WebGL pattern for computing texture unit constants, replacing the fragile dynamic property lookup.
840-844: LGTM!Consistent defensive guard pattern for the mouse lerp operation, matching the other iMouse handlers.
packages/shadcn/components/agents-ui/agent-control-bar.tsx (6)
3-17: LGTM!Imports are well-organized and the
motion/reactimport path is correct for motion v12+.
111-122: LGTM!The className composition using
cn()is appropriate, and thefield-sizing-contentutility for intrinsic textarea sizing is a nice modern CSS approach.
138-242: LGTM!The JSDoc documentation improvements are clear and well-structured. The
@defaultValueannotations provide useful guidance for consumers of this component.
258-267: LGTM!The
microphoneTrackdestructuring aligns with the PR objective of retrieving the microphone track viauseSessionContext. The rename frommicTrackReftomicrophoneTrackimproves clarity and consistency with the session context API.
314-332: LGTM on the microphone control wiring.The
audioTrack={microphoneTrack}prop correctly passes the track from the session context toAgentTrackControl, consistent with the centralized microphone sourcing approach described in the PR objectives.
291-295: Verify Tailwind v4 opacity modifier syntax.The
drop-shadow-md/3syntax uses Tailwind v4's opacity modifier format. Ensure the project is configured for Tailwind v4, as this syntax would be invalid in Tailwind v3.packages/shadcn/components/agents-ui/agent-audio-visualizer-grid.tsx (1)
52-55: The code is using valid Tailwind CSS 4.x syntax. The*:modifier is fully supported as a composable variant in Tailwind v4 and can be stacked with arbitrarydata-[...]selectors. No compatibility issues or fallbacks are 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.
thomasyuill-livekit
force-pushed
the
ty/shadcn-cleanup-microphoneTrack-creation
branch
from
a56a8da to
1861835
Compare
thomasyuill-livekit
deleted the
ty/shadcn-cleanup-microphoneTrack-creation
branch
Summary
useMicrophonewithuseSessionContextacross Agent storiesuseInputControls/AgentControlBarto useuseSessionContextto retrieve microphoneTrackSummary by CodeRabbit
Refactor
Style
Docs
✏️ Tip: You can customize this high-level summary in your review settings.