feat(listen-button): add scrolling waveform for audio amplitude#2234
feat(listen-button): add scrolling waveform for audio amplitude#2234
Conversation
✅ Deploy Preview for hyprnote-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for hyprnote ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughReplaces the exported Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/desktop/src/components/main/body/sessions/outer-header/listen.tsx (1)
111-147: Consider using className and theme colors for better maintainability.The component uses inline styles and hardcoded RGB values. Consider refactoring to use Tailwind classes or CSS variables for better maintainability and consistency with the rest of the codebase.
Example refactor:
return ( - <div - style={{ - position: "relative", - width, - height, - minWidth: width, - minHeight: height, - }} - > + <div className="relative" style={{ width, height, minWidth: width, minHeight: height }}> <canvas ref={canvasRef} style={{ width, height }} /> - <div - style={{ - position: "absolute", - top: 0, - left: 0, - width: 12, - height: "100%", - background: - "linear-gradient(to right, rgb(254 242 242), transparent)", - pointerEvents: "none", - }} - /> + <div + className="absolute top-0 left-0 h-full pointer-events-none" + style={{ + width: 12, + background: "linear-gradient(to right, rgb(254 242 242 / 1), transparent)" + }} + /> - <div - style={{ - position: "absolute", - top: 0, - right: 0, - width: 12, - height: "100%", - background: "linear-gradient(to left, rgb(254 242 242), transparent)", - pointerEvents: "none", - }} - /> + <div + className="absolute top-0 right-0 h-full pointer-events-none" + style={{ + width: 12, + background: "linear-gradient(to left, rgb(254 242 242 / 1), transparent)" + }} + /> </div> );Note: The hardcoded
rgb(254 242 242)appears to match the red-50 background. You might want to extract this to a CSS variable or use a Tailwind color reference to maintain consistency if the theme changes.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/desktop/src/components/main/body/sessions/outer-header/listen.tsx(3 hunks)apps/desktop/src/components/main/body/shared.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
If there are many classNames with conditional logic, usecn(import from@hypr/utils). It is similar toclsx. Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/desktop/src/components/main/body/sessions/outer-header/listen.tsxapps/desktop/src/components/main/body/shared.tsx
🧬 Code graph analysis (1)
apps/desktop/src/components/main/body/sessions/outer-header/listen.tsx (1)
packages/utils/src/cn.ts (1)
cn(20-22)
⏰ 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). (7)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
- GitHub Check: desktop_ci (linux, depot-ubuntu-24.04-8)
- GitHub Check: fmt
- GitHub Check: desktop_ci (linux, depot-ubuntu-22.04-8)
- GitHub Check: desktop_ci (macos, depot-macos-14)
🔇 Additional comments (4)
apps/desktop/src/components/main/body/shared.tsx (1)
2-2: LGTM!Clean import update following the removal of SoundIndicator component.
apps/desktop/src/components/main/body/sessions/outer-header/listen.tsx (3)
3-3: LGTM!Import additions are necessary for the new ScrollingWaveform component's canvas rendering and animation logic.
23-41: LGTM!Props are appropriately inlined for this internal component, following the coding guidelines for non-shared types.
263-290: LGTM!The updated two-panel layout (waveform when not hovered, stop label when hovered) provides good UX. The
cnusage follows the coding guidelines by using array syntax with logical grouping.
apps/desktop/src/components/main/body/sessions/outer-header/listen.tsx
Outdated
Show resolved
Hide resolved
ComputelessComputer
force-pushed
the
feat/scrolling-waveform-audio-amplitude
branch
from
265407a to
7e8b457
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/desktop/src/components/main/body/shared.tsx (2)
48-49: Hover-only UI can be inaccessible for keyboard/touch users (close affordance never appears).Consider also revealing the close button on focus (
:focus-visible/group-focus-within) and/or always rendering it with visually-hidden text.Also applies to: 72-74, 101-121
122-140: Addtype="button"+aria-labelon the icon-only close button; verify no nested interactive elements.
- Without
type="button", this can accidentally submit if used inside a<form>.- Icon-only button needs an accessible name (e.g.
aria-label="Close tab").- With
InteractiveButton asChild, double-check we’re not ending up with<button><button/></button>(invalid HTML) depending on howInteractiveButtonrenders.<button + type="button" + aria-label="Close tab" onClick={(e) => { e.stopPropagation(); handleCloseThis(); }}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/desktop/src/components/main/body/sessions/outer-header/listen.tsx(3 hunks)apps/desktop/src/components/main/body/shared.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/components/main/body/sessions/outer-header/listen.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
If there are many classNames with conditional logic, usecn(import from@hypr/utils). It is similar toclsx. Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/desktop/src/components/main/body/shared.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). (10)
- GitHub Check: Redirect rules - hyprnote-storybook
- GitHub Check: Header rules - hyprnote-storybook
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Pages changed - hyprnote-storybook
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
- GitHub Check: fmt
- GitHub Check: desktop_ci (macos, depot-macos-14)
- GitHub Check: desktop_ci (linux, depot-ubuntu-24.04-8)
- GitHub Check: desktop_ci (linux, depot-ubuntu-22.04-8)
🔇 Additional comments (1)
apps/desktop/src/components/main/body/shared.tsx (1)
2-2: Preferimport * as React from "react"(or explicitly importReact) if relying onReact.ReactNodetype.This file uses
React.ReactNode/React.MouseEventbut only importsuseState; ensure the project’s TS config provides global React types (otherwise this becomes a type error).
2 participants
No description provided.