Extract listen-button and sound-indicator into UI package#1515
Extract listen-button and sound-indicator into UI package#1515
Conversation
|
Note Reviews pausedUse the following commands to manage reviews:
📝 WalkthroughWalkthroughRemoves desktop FloatingButton and SoundIndicator components, refactors the desktop ListenButton to delegate to a new UI block component, adds new ListenButton and SoundIndicator components under packages/ui, adjusts Tailwind scan paths, and downgrades the motion dependency version. Locale files show no functional changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant ListenButton as UI Block: ListenButton
participant Session as Recording Session API
participant Devices as Device Manager
rect rgb(245,248,255)
note right of ListenButton: Inactive states (meeting not ended/ended)
User->>ListenButton: Click
alt Start allowed
ListenButton->>Session: startSession(templateId?)
Session-->>ListenButton: running state + amplitudes
else Disabled
ListenButton-->>User: No action
end
end
rect rgb(242,255,245)
note right of ListenButton: Active (this session)
User->>ListenButton: Open controls popover
ListenButton->>Devices: listMicrophones()
Devices-->>ListenButton: devices | loading | none
User->>ListenButton: Toggle mic/speaker or change device
ListenButton->>Session: updateSettings(mic/speaker/device)
User->>ListenButton: Stop
ListenButton->>Session: stopSession()
Session-->>ListenButton: inactive state
end
rect rgb(255,248,240)
note right of ListenButton: Active in other session
ListenButton-->>User: Render nothing
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
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 |
|
@CodeRabbit ignore |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/ui/src/components/block/sound-indicator.tsx (1)
4-16: Move constant outside component.The
u16maxconstant is recreated on every render. Since it's a pure constant, declare it outside the component function for better performance.Apply this diff:
+const U16_MAX = 65535; + export function SoundIndicator({ value, color }: { value: number | Array<number>; color?: string }) { const [amplitude, setAmplitude] = useState(0); - const u16max = 65535; useEffect(() => { const sample = Array.isArray(value) - ? (value.reduce((sum, v) => sum + v, 0) / value.length) / u16max - : value / u16max; + ? (value.reduce((sum, v) => sum + v, 0) / value.length) / U16_MAX + : value / U16_MAX; setAmplitude(Math.min(sample, 1)); }, [value]);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
apps/desktop/src/locales/en/messages.pois excluded by!**/*.poapps/desktop/src/locales/ko/messages.pois excluded by!**/*.popnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
apps/desktop/src/components/editor-area/floating-button.tsx(0 hunks)apps/desktop/src/components/editor-area/note-header/listen-button.tsx(5 hunks)apps/desktop/src/components/sound-indicator.tsx(0 hunks)apps/desktop/src/locales/en/messages.ts(1 hunks)apps/desktop/src/locales/ko/messages.ts(1 hunks)packages/ui/package.json(1 hunks)packages/ui/src/components/block/listen-button.tsx(1 hunks)packages/ui/src/components/block/sound-indicator.tsx(1 hunks)packages/ui/tailwind.config.ts(1 hunks)
💤 Files with no reviewable changes (2)
- apps/desktop/src/components/sound-indicator.tsx
- apps/desktop/src/components/editor-area/floating-button.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit configuration file
**/*.{js,ts,tsx,rs}: 1. Do not add any error handling. Keep the existing one.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
Files:
packages/ui/src/components/block/listen-button.tsxpackages/ui/tailwind.config.tsapps/desktop/src/locales/ko/messages.tsapps/desktop/src/locales/en/messages.tsapps/desktop/src/components/editor-area/note-header/listen-button.tsxpackages/ui/src/components/block/sound-indicator.tsx
🧬 Code graph analysis (4)
packages/ui/src/components/block/listen-button.tsx (4)
packages/ui/src/lib/utils.ts (1)
cn(4-6)packages/ui/src/components/ui/shiny-button.tsx (1)
ShinyButton(23-57)packages/ui/src/components/block/sound-indicator.tsx (1)
SoundIndicator(4-16)packages/ui/src/components/ui/button.tsx (1)
Button(37-89)
apps/desktop/src/locales/ko/messages.ts (1)
apps/desktop/src/locales/en/messages.ts (1)
messages(1-1)
apps/desktop/src/locales/en/messages.ts (1)
apps/desktop/src/locales/ko/messages.ts (1)
messages(1-1)
packages/ui/src/components/block/sound-indicator.tsx (1)
packages/ui/src/components/ui/dancing-sticks.tsx (1)
DancingSticks(49-74)
⏰ 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). (2)
- GitHub Check: ci (windows, windows-latest)
- GitHub Check: ci (macos, macos-14)
🔇 Additional comments (14)
apps/desktop/src/components/editor-area/note-header/listen-button.tsx (1)
50-50: Review the disabled flag computation logic.The
disabledflag is computed as!modelDownloaded || (meetingEnded && isEnhancePending), but the state computation at lines 57-58 usesmeetingEndedalone to determine if the state is"inactive_meeting_ended".This creates a scenario where:
- When
meetingEndedis true butisEnhancePendingis false, the button state will be"inactive_meeting_ended"butdisabledwill befalse- This may be intentional (allowing restart after meeting ended but before enhancement begins), but please verify this is the desired behavior
packages/ui/tailwind.config.ts (1)
147-150: Tailwind content globs sufficiently cover all component files.
No.tsxfiles exist in nested subdirectories undersrc/components/uiorsrc/components/block, so the direct-child patterns (*.tsx) will scan all components.packages/ui/package.json (1)
42-42: Approve Motion v11 downgrade. Code uses only core APIs—motioncomponents (motion.dev),AnimatePresence(motion.dev),motion.create(motion.dev), andvariants(motion.dev)—all supported in v11.18.2.packages/ui/src/components/block/sound-indicator.tsx (1)
1-2: LGTM!Imports are clean and all are used in the component.
packages/ui/src/components/block/listen-button.tsx (10)
1-19: LGTM!All imports are utilized within the component and its subcomponents.
21-26: LGTM!The type definition clearly captures all possible button states and aligns with the switch cases in the main component.
28-98: LGTM!The switch-based state machine cleanly delegates to appropriate subcomponents based on the session state.
100-119: LGTM!The button correctly applies conditional styling and provides clear user feedback via the tooltip.
148-168: LGTM!The onboarding variant correctly uses ShinyButton with appropriate styling and conditional states.
170-188: LGTM!The onboarding ended state appropriately displays "Play again" with consistent styling.
190-252: LGTM!The active recording state correctly manages the popover and visualizes combined mic/speaker amplitude via SoundIndicator.
254-303: LGTM!The recording controls correctly compose microphone, speaker, and stop button components.
321-436: LGTM!The microphone selector correctly handles device enumeration, selection, and mute toggle with proper loading and empty states.
438-475: LGTM!The speaker button follows the same pattern as the microphone button with appropriate amplitude visualization.
✅ Actions performedReviews paused. |
1 participant
No description provided.