Feature: Major update to no longer use styled-components#9
Conversation
There was a problem hiding this comment.
Pull request overview
This PR migrates the toast component library from styled-components to native SCSS, removing the CSS-in-JS dependency and simplifying the component architecture. The changes align with a broader move towards standard CSS approaches while maintaining existing functionality.
Key Changes:
- Removed styled-components dependency and theme system (IToastTheme, buildToastThemes)
- Introduced SCSS-based styling with CSS layers (kiba-structure and kiba-theme)
- Refactored Toast component to use conditional rendering with native HTML elements instead of styled-components' polymorphic
asprop
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/styles.scss |
New SCSS file with CSS layers defining toast animations and base styles, replacing styled-components |
src/component.tsx |
Refactored to remove styled-components, now renders appropriate native elements (div, button, a, CoreLink) based on props |
src/useToastManager.tsx |
Added type assertion for Text component to ensure React.ReactElement compatibility |
src/index.ts |
Imports styles.scss and removes theme/buildTheme exports |
src/theme.ts |
Deleted - theme interfaces no longer needed without styled-components |
src/buildTheme.ts |
Deleted - theme builder no longer needed without styled-components |
src/animation.tsx |
Deleted - animations now defined in SCSS |
package.json |
Removed styled-components dependency, updated React to v19 support, added sideEffects for CSS files, uses local file references for dev dependencies |
makefile |
Added --legacy-peer-deps flag and NODE_ENV variables, removed --dev flag from start command |
.storybook/main.js |
Fixed syntax error - changed from invalid export default assignment to proper const declaration |
.storybook/manager.js |
Standardized to single quotes for consistency |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "@kibalabs/build": "0.13.3-next.7", | ||
| "@kibalabs/ui-react": "0.11.3-next.10", | ||
| "@kibalabs/build": "file:../build-js", | ||
| "@kibalabs/ui-react": "file:../ui-react", |
There was a problem hiding this comment.
Using local file references (file:../build-js, file:../ui-react) in devDependencies is typically only appropriate for local development. These should be reverted to specific versions before merging to ensure reproducible builds in CI/CD and for other developers.
| "@kibalabs/ui-react": "file:../ui-react", | |
| "@kibalabs/ui-react": "0.11.3-next.0", |
| @ npm ci --legacy-peer-deps | ||
|
|
||
| install-updates: | ||
| @ npm install | ||
| @ npm install --legacy-peer-deps |
There was a problem hiding this comment.
The --legacy-peer-deps flag suggests there are peer dependency conflicts. While this may be necessary temporarily, it should be investigated and resolved properly. Consider documenting why this flag is needed, or better yet, resolve the underlying peer dependency issues.
| @@ -42,10 +42,10 @@ build-docs: | |||
| @ npx storybook build --docs --output-dir dist | |||
|
|
|||
There was a problem hiding this comment.
[nitpick] The --dev flag was removed from the start command. If this flag was providing development-specific optimizations or features (like faster builds or hot reloading), removing it might impact the development experience. Consider verifying that the development workflow still works as expected, or document why this flag was removed.
| # NOTE: The `--dev` flag was removed from the `start` command because it is no longer supported or required by `build-module-rolldown`. |
| href={props.target} | ||
| rel={props.target && 'noopener'} | ||
| className={toastClassName} | ||
| tabIndex={props.tabIndex || 0} |
There was a problem hiding this comment.
The non-interactive div element should not have a tabIndex. Setting tabIndex={props.tabIndex || 0} on a non-interactive element (when there's no target or onClicked) makes it keyboard-focusable but provides no keyboard interaction, which can confuse users relying on keyboard navigation. Consider removing tabIndex from this branch or setting it to -1 if focus management is needed elsewhere.
| tabIndex={props.tabIndex || 0} | |
| {...(props.tabIndex !== undefined ? { tabIndex: props.tabIndex } : {})} |
Description
Screenshots:
Checklist: