feat: Add components to WebKit helper #380
Conversation
- Created alerts/ folder containing Alert and Notice components
- Added shared alertIcons helper and alert.types definitions
- Both components support info, warning, success, and error types
- Fully customizable via CSS variables and props
- Added @lucide/svelte as peer dependency
- Updated package.json exports for @ainsleydev/sveltekit-helper/components/alerts
- Exported components from main components/index.ts
Usage:
import { Alert, Notice } from '@ainsleydev/sveltekit-helper/components/alerts'
- Move @lucide/svelte from peerDependencies to dependencies - Upgrade @lucide/svelte to latest version (^0.562.0) - Add comprehensive JSDoc comments to AlertType and IconDetail types - Document each alert variant (info, warning, success, error)
- Rename alerts/ folder to notifications/ for better semantic clarity - Move @lucide/svelte from dependencies to peerDependencies (v0.562.0) - Simplify type documentation in alert.types.ts - Update package.json exports: ./components/alerts → ./components/notifications - Update components/index.ts imports to reference ./notifications BREAKING CHANGE: Import path changed from '@ainsleydev/sveltekit-helper/components/alerts' to '@ainsleydev/sveltekit-helper/components/notifications'
- Move AlertType and NoticeType into respective component files - Remove internal exports (alertIcons, IconDetail) from public API - Add comprehensive @component documentation with examples to Alert and Notice - Convert single-line comments to multi-line format for better readability - Move IconDetail type definition into alertIcons.ts (internal use only) - Delete alert.types.ts (types now colocated with components) - Export both AlertType and NoticeType for external type usage Components now have complete inline documentation showing usage examples.
- Remove verbose comments from AlertType and NoticeType - Add concise comment to alertIcons export - Keep code minimal and self-documenting
This comment has been minimized.
This comment has been minimized.
- Fix Svelte 5 reactivity: use $bindable(true) for visible prop in Alert and Notice - Add comprehensive CSS custom properties documentation to both components - Fix linter issues (type import formatting) - Ensures dismiss functionality properly updates parent component state - All tests passing CSS variables are now documented inline for easier customization.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Review summary
This PR adds well-structured Alert and Notice notification components to the sveltekit-helper package, along with supporting SCSS utilities. The components follow accessibility best practices and are highly customisable. However, there's a critical issue with the fluid-type mixin referencing undefined variables, and missing test coverage. Critical issues 🔴Undefined SCSS variables in fluid-type mixinLocation: packages/sveltekit-helper/src/scss/_mixins.scss:82-102 The fluid-type mixin references undefined variables $type-fluid-start-size and $type-fluid-end-size that are not defined anywhere in the codebase. This will cause compilation errors when the mixin is used. Either define these variables in a sizes/tokens file, or convert them to mixin parameters. Warnings 🟡Missing test coverageThe new Alert and Notice components have no test files. According to AGENTS.md, JS components should be tested with Vitest. Consider adding component rendering tests, prop validation tests, interaction tests, and accessibility tests. Inconsistent import extension in Notice.svelteNotice.svelte imports ./alertIcons without .js extension (line 19), while Alert.svelte uses ./alertIcons.js (line 21). Maintain consistency - prefer explicit .js extensions. CSS custom property fallback inconsistencyHardcoded fallback values (6px) don't reference SCSS variables despite border-radius-6 being imported and available. Consider using SCSS variables for consistency. Commented-out code in mixinsThe px-to-rem mixin is commented out but left in the file (lines 53-63). Either remove it entirely or document why it's preserved. Suggestions 🟢Enhanced documentation examplesThe component documentation could benefit from showing the visible binding pattern for toggling alerts. Consider extracting common alert logicAlert and Notice share significant logic (iconDetail derivation, hide function, ariaLive). Consider extracting to a shared composition function to reduce duplication. Review notes: Components follow project conventions, accessibility implementation is solid, CSS custom properties provide excellent theming flexibility, and SCSS follows project patterns. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #380 +/- ##
==========================================
+ Coverage 64.59% 69.80% +5.21%
==========================================
Files 154 185 +31
Lines 6064 7359 +1295
==========================================
+ Hits 3917 5137 +1220
+ Misses 2064 2025 -39
- Partials 83 197 +114 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
No description provided.