enhance: migrate design-system package to vite bundler and integrate shadcn #76
Conversation
# Conflicts: # package-lock.json
|
Important Review skippedMore than 25% of the files skipped due to max files limit. The review is being skipped to prevent a low-quality review. 28 files out of 87 files are above the max files limit of 50. Please upgrade to Pro plan to get higher limits. You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
* start vite migration * chore: upgrade pnpm * add prettier * add constants and copy, format * updates * initialize shadcn * delete design system v3 * upgrade prettier etc * install tailwind plugins * add ladle * ts for ladle * working ladle with our design system theme * fix package manager entry in package.json * unknown instead of any * update deps * format
rschlaefli
changed the title
| import Countdown from './Countdown' | ||
|
|
||
| export const Default = () => { | ||
| let time = new Date() | ||
| const time = new Date() |
There was a problem hiding this comment.
Even though it should work technically, does it make sense to declare a constant time, which is then changed in the following lines using the setSeconds method? Simply out of interest: What is the best practice in such cases - to declare a variable to be constant whenever possible or when it remains semantically constant?
| import CycleCountdown from './CycleCountdown' | ||
|
|
||
| export const Default = () => { | ||
| let time = new Date() | ||
| const time = new Date() |
There was a problem hiding this comment.
Same question as above for Countdown stories
| export * from './Switch' | ||
| export * from './Table' | ||
| export * from './Tabs' | ||
| export { default as Button } from './Button' |
There was a problem hiding this comment.
If we only export the default under the corresponding name here, will the corresponding exported interface props still be available (not tested)? In some cases, e.g. for the select component, the corresponding types have probably been used in KlickerUZH
| import { twMerge } from 'tailwind-merge' | ||
|
|
||
| export function cn(...inputs: ClassValue[]) { | ||
| return twMerge(clsx(inputs)) |
There was a problem hiding this comment.
This seems like a very good idea 👍 , especially in the case of "conflicting styles" (according to https://stackoverflow.com/questions/69390216/how-to-properly-join-tailwind-css-classes-using-clsx). It would probably make sense to replace the corresponding use of tailwind-merge through this combined function in all components soon, but we probably need to be careful to sudden changes in appearance in the corresponding apps...
| }, | ||
| }, | ||
| }, | ||
| plugins: [ | ||
| require('tailwindcss-radix')({ variantPrefix: 'rdx' }), |
There was a problem hiding this comment.
Can we remove these radix styles without loosing functionality here? Corresponding tailwind styles are used in the Tabs and Navigation components.
No description provided.