fix: improve atom components & adjust components styles #5
Conversation
✅ Deploy Preview for statuesque-boba-0fb888 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull Request Overview
This PR improves atom components by refactoring component implementations, adjusting styling to use design system tokens consistently, and improving the Storybook developer experience with better npm scripts.
Key Changes:
- Refactored textarea to reuse Input component implementation
- Updated component styles to use design system tokens (e.g.,
bg-input,bg-primary) - Enhanced snippet detection logic and added
baseprop to snippet rendering
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/lib/components/textarea/textarea-root.svelte |
Refactored to use Input.Root component instead of custom implementation |
src/lib/components/root/root.svelte |
Simplified Portals component ID by removing "fixed" suffix |
src/lib/components/portal/portal-root.svelte |
Removed unused cn import |
src/lib/components/portal/portal-inner.svelte |
Removed unused cn import |
src/lib/components/input/input-root.svelte |
Updated background color from bg-background to bg-input token |
src/lib/components/button/button.svelte |
Updated to use primary color tokens and added disabled state styling |
src/lib/components/atom/html-atom.svelte |
Improved snippet detection logic and added base prop to snippet rendering |
package.json |
Renamed and added Storybook scripts for better clarity and functionality |
| }: HtmlAtomProps<E, B> & HTMLAttributes<Element> = $props(); | ||
| const isSnippet = typeof base === 'function' && base.name !== 'wrapper'; | ||
| const isSnippet = $derived(typeof base === 'function' && base.length === 1 && !base.prototype); |
There was a problem hiding this comment.
The snippet detection logic using base.length === 1 is fragile and may incorrectly identify functions. Svelte 5 snippets have a specific signature - consider using a more reliable detection method such as checking for the snippet symbol or using a documented Svelte API if available.
| const isSnippet = $derived(typeof base === 'function' && base.length === 1 && !base.prototype); | |
| const isSnippet = $derived( | |
| typeof base === 'function' && !!base && !!base[Symbol.for('svelte.snippet')] | |
| ); |
|
|
||
| {#if isSnippet} | ||
| {@render snippet({ class: cn(klass), as, children, ...restProps })} | ||
| {@render snippet({ class: cn(klass), as, base, children, ...restProps })} |
There was a problem hiding this comment.
Adding base to the snippet props is potentially breaking if existing snippets don't expect this parameter. Consider documenting this change or providing backward compatibility if this is part of a public API.
| {@render snippet({ class: cn(klass), as, base, children, ...restProps })} | |
| {@render snippet({ class: cn(klass), as, children, ...restProps })} |
No description provided.