-
Notifications
You must be signed in to change notification settings - Fork 137
feat: LiveKit OSS Shadcn Components 2 #1250
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
|
size-limit report 📦
|
5579440 to
a0e85fb
Compare
a0e85fb to
2117571
Compare
| @@ -0,0 +1,201 @@ | |||
| // forked from https://github.com/rysana-ai/react-shaders | |||
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.
thought: wondering if it's worth forking this as it's own package instead of having it live within the registry.
@thomasyuill-livekit I know we've discussed this on another PR already, but would be great to get the necessary fix upstreamed to the react-shaders repo
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.
I'm not against that, but I'd rather contribute back to https://github.com/rysana-ai/react-shaders but my concern is it hasn't been updated in 6 months, so it might be unmaintained. I'm not sure I want to hold up this PR to shepherd this through seeing as it's a significant update (I migrated it from a class component to a functional component).
If we forked it, would there be react-shader and livekit-react-shader?
If I can get this into https://github.com/rysana-ai/react-shaders, I believe we can remove the react-shader component at a later date and update our component dependencies. Old installs will already have the react-shader component, and old installs will get the new dependency.
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.
Just out of curiosity, what is actually the cause for needing to vendor react-shader and not depend on the published package? Is it that it's implemented as a class component? Or maybe that you are worried that it seems unmaintained?
I think out of the options proposed, I agree that I think a fork would probably be better than including the modified code as a shadcn component, since it is unlikely a user would need to further modify the code and it would be good for a user to be able to easily update to new versions.
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.
Just out of curiosity, what is actually the cause for needing to vendor react-shader
The shader fails when rendered dynamically in agent-starter-react. Though it works fine in storybook 🤷.
More interestingly, it seems to work when rendered after SSR, but throws a shader compilation error when rendered only on the client.
I've submitted a PR, let's see what happens
rysana-ai/react-shaders#3
Initially I converted the component to a functional component, but that didn't resolve the issue. Then I added a requestAnimationFrame around the initial draw call and the issue went away.
My initial PR just introduced the requestAnimationFrame (commit rysana-ai/react-shaders@20fa4d9), but further testing revealed that it did not resolve the issue. I brought over the migrated functional component and the issue was resolved.
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.
I don't see the harm in publishing our shadcn react-shader to unblock ourselves and later removing it for a reliable dependency (rysana/livekit).
Maybe I'm misunderstanding the nature of shadcn components
2117571 to
61e0e31
Compare
| options: [ | ||
| 'idle', | ||
| 'disconnected', | ||
| 'pre-connect-buffering', | ||
| 'connecting', | ||
| 'initializing', | ||
| 'listening', | ||
| 'thinking', | ||
| 'speaking', | ||
| 'failed', | ||
| ], |
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.
nitpick(non-blocking): Maybe it's worth making a constant somewhere with all these values so it's easy to update across everything if we added a new one?
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.
Yeah, we should explore that in a future update. We should export it from livekit/components-js
packages/shadcn/hooks/agents-ui/use-agent-audio-visualizer-grid.ts
Outdated
Show resolved
Hide resolved
packages/shadcn/components/agents-ui/audio-visualizer-wave/audio-visualizer-wave.tsx
Outdated
Show resolved
Hide resolved
| @@ -0,0 +1,201 @@ | |||
| // forked from https://github.com/rysana-ai/react-shaders | |||
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.
Just out of curiosity, what is actually the cause for needing to vendor react-shader and not depend on the published package? Is it that it's implemented as a class component? Or maybe that you are worried that it seems unmaintained?
I think out of the options proposed, I agree that I think a fork would probably be better than including the modified code as a shadcn component, since it is unlikely a user would need to further modify the code and it would be good for a user to be able to easily update to new versions.
e9fff9b to
6b47aad
Compare
6b47aad to
01723d8
Compare
Overview
This PR introduces a suite of new audio visualizer components to the
@livekit/components-shadcnpackage, providing rich visual feedback for agent states in voice AI applications.New Components
🎨 Audio Visualizers
AudioVisualizerRadialAudioVisualizerGrid🔧 Supporting Infrastructure
useRadialAnimatoranduseGridAnimatorfor state-driven animationFeatures
connecting,initializing,listening,thinking,speaking,disconnecteduseMultibandTrackVolumeanduseTrackVolumehooksicon,sm,md,lg,xlmotion/reactlibraryStorybook
All new components include Storybook stories with interactive controls for testing different states, sizes, and configurations.