chore: initialize storybook using nx command #97
Conversation
📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThis pull request introduces significant updates to the project's configuration and styling, particularly enhancing the Storybook integration. It adds new files for global styles, theme configurations, TypeScript settings, and Babel configurations. Key changes include the introduction of Tailwind CSS for styling, support for dark mode, and the establishment of a structured environment for developing and testing UI components. Additionally, new utility functions and components for button styling are introduced, improving the overall user interface consistency and flexibility. Changes
Poem
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
❌ Deploy Preview for cuhacking-portal-dev failed. Why did it fail? →
|
MFarabi619
force-pushed
the
mfarabi/feat/96-add-storybook-with-tailwindcss-shadcn-ui-and-chromatic-visual-test-server
branch
2 times, most recently
from
bc63fbe
to
1ff9815
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
libs/storybook-host/src/lib/storybook-host.tsx (1)
1-9: LGTM!The
StorybookHostcomponent provides a solid foundation for the Storybook host interface. The implementation is correct and follows the React functional component syntax.Consider the following suggestions for further customization:
- Add more JSX elements and components to create a more comprehensive and visually appealing Storybook host interface.
- Utilize CSS or a styling library to enhance the visual presentation of the component.
- Incorporate interactive elements, such as buttons or links, to navigate between different stories or sections within Storybook.
- Integrate with the Storybook API to dynamically render the selected story components within the host interface.
- Implement responsive design techniques to ensure the component adapts well to different screen sizes and devices.
Feel free to explore and expand upon this component to create a rich and engaging Storybook experience tailored to your project's needs.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
Files selected for processing (15)
- libs/storybook-host/.babelrc (1 hunks)
- libs/storybook-host/.storybook/main.ts (1 hunks)
- libs/storybook-host/.storybook/preview.ts (1 hunks)
- libs/storybook-host/README.md (1 hunks)
- libs/storybook-host/eslint.config.js (1 hunks)
- libs/storybook-host/package.json (1 hunks)
- libs/storybook-host/project.json (1 hunks)
- libs/storybook-host/src/index.ts (1 hunks)
- libs/storybook-host/src/lib/storybook-host.tsx (1 hunks)
- libs/storybook-host/tsconfig.json (1 hunks)
- libs/storybook-host/tsconfig.lib.json (1 hunks)
- libs/storybook-host/tsconfig.storybook.json (1 hunks)
- nx.json (3 hunks)
- package.json (3 hunks)
- tsconfig.base.json (2 hunks)
Files skipped from review due to trivial changes (6)
- libs/storybook-host/.storybook/preview.ts
- libs/storybook-host/README.md
- libs/storybook-host/package.json
- libs/storybook-host/project.json
- libs/storybook-host/src/index.ts
- tsconfig.base.json
Files skipped from review as they are similar to previous changes (1)
- nx.json
Additional comments not posted (7)
libs/storybook-host/.babelrc (1)
1-12: Babel configuration follows best practices for React applications.The Babel configuration in this file is well-structured and follows best practices for React applications:
- The
@nx/react/babelpreset is used, which is tailored for React applications and enables the automatic JSX runtime. This ensures compatibility with modern React features and simplifies the JSX usage.- The
useBuiltInsoption is set to "usage," allowing Babel to include only the necessary polyfills based on the code's usage. This helps optimize the bundle size by avoiding the inclusion of unnecessary polyfills.- The empty plugins array indicates that no additional plugins are being used, keeping the configuration simple and focused on the preset's default behavior.
Overall, this Babel configuration provides a solid foundation for transpiling React code in the project.
libs/storybook-host/tsconfig.json (1)
1-20: LGTM!The new TypeScript configuration file for the
libs/storybook-hostdirectory is well-structured and follows best practices. It extends a base configuration, specifies appropriate compiler options for a React application, and references additional configuration files for further customization.The configuration promotes type safety, module management, and interoperability, which will contribute to code quality and maintainability in the Storybook host library.
libs/storybook-host/tsconfig.lib.json (1)
1-27: LGTM!The TypeScript configuration for the library looks good:
- Extending the base
tsconfig.jsonpromotes consistency across the project.- Setting
outDirto../../dist/out-tsckeeps the compiled output separate from the source files.- Including types for Node.js and specific React typings enhances type safety.
- Explicitly including JavaScript, JSX, TypeScript, and TSX files ensures all relevant source files are compiled.
- Excluding test files, stories, and configuration files keeps the compilation focused on the library code.
The configuration follows best practices and provides a solid foundation for the library.
libs/storybook-host/tsconfig.storybook.json (1)
1-31: LGTM!The TypeScript configuration for Storybook looks good and follows the recommended setup for integrating Storybook with TypeScript in an Nx workspace. It extends the base
tsconfig.json, enables necessary compiler options, includes relevant type declaration files, and specifies appropriate include/exclude patterns for Storybook files.libs/storybook-host/.storybook/main.ts (1)
1-35: LGTM!The Storybook configuration follows the recommended practices and includes essential addons for an enhanced development experience. The custom Vite configuration ensures compatibility with the Nx workspace, and the use of
react-docgen-typescriptensures accurate documentation of React components.libs/storybook-host/eslint.config.js (1)
1-41: LGTM! The ESLint configuration file is well-structured and modular.The file follows a modular approach by importing the base configuration from a separate file and extending it with additional rules and settings. The use of an asynchronous function allows for proper resolution of the base configuration promise.
The commented-out sections provide a clear structure for extending the configuration in the future, making it easy to add specific rules for different file types or frameworks.
A few suggestions for future improvements:
- Consider adding comments to explain the purpose of each section and the specific rules being applied.
- If the commented-out sections are not needed, consider removing them to keep the file clean and focused.
- Ensure that the
TODOcomment regarding merging with theantfuESLint config is addressed in a timely manner.Overall, the file sets up a solid foundation for the ESLint configuration of the
storybook-hostlibrary.package.json (1)
38-40: LGTM! The added Babel and Storybook dependencies are essential for the Storybook integration.The added dependencies in the
devDependenciessection are crucial for integrating Storybook into the project and enabling the development and testing of UI components in isolation. The Babel dependencies (@babel/coreand@babel/preset-react) are required for transpiling modern JavaScript and React code, while the Storybook packages provide the necessary tools and addons for building and showcasing UI components.The versions of the dependencies appear to be consistent and up to date, aligning with the project's requirements.
Also applies to: 64-70
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
libs/storybook-host/src/button.tsx (1)
1-3: Consider enhancing the Button component for better reusability and customization.The Button component is a good start for a reusable UI component. However, consider the following improvements:
Rename the component to a more descriptive name, such as
PrimaryButtonorDefaultButton, to better convey its purpose and appearance.Make the button text configurable through props to allow for different labels to be used when the component is reused in different contexts. For example:
export function Button({ label }: { label: string }) { return <button type="button">{label}</button>; }
- Make the button type configurable through props to support other types like "submit" or "reset". For example:
export function Button({ label, type }: { label: string; type?: "button" | "submit" | "reset" }) { return <button type={type || "button"}>{label}</button>; }These enhancements will make the Button component more flexible and reusable across different scenarios in the application.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
Files selected for processing (4)
- libs/storybook-host/.storybook/main.ts (1 hunks)
- libs/storybook-host/src/button.stories.tsx (1 hunks)
- libs/storybook-host/src/button.tsx (1 hunks)
- package.json (4 hunks)
Files skipped from review as they are similar to previous changes (2)
- libs/storybook-host/.storybook/main.ts
- package.json
Additional comments not posted (5)
libs/storybook-host/src/button.stories.tsx (5)
6-21: LGTM!The
metaconfiguration object is correctly set up and follows the Storybook conventions. The use offnfor theonClickargument is a good practice for tracking user interactions in the actions panel.
27-32: LGTM!The
Primarystory definition is correctly set up and provides a clear example of the primary variant of theButtoncomponent.
34-38: LGTM!The
Secondarystory definition is correctly set up and provides a clear example of the secondary variant of theButtoncomponent.
40-45: LGTM!The
Largestory definition is correctly set up and provides a clear example of the large variant of theButtoncomponent.
47-52: LGTM!The
Smallstory definition is correctly set up and provides a clear example of the small variant of theButtoncomponent.
MFarabi619
force-pushed
the
mfarabi/feat/96-add-storybook-with-tailwindcss-shadcn-ui-and-chromatic-visual-test-server
branch
from
1a2f848
to
12dfc41
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
apps/website/app/components/button.tsx (1)
1-3: LGTM! Consider enhancing the component by accepting props.The
Buttoncomponent is correctly implemented and provides a reusable UI element. Setting the button type to "button" is a good practice to prevent form submission on click.To make the component more reusable and customizable, consider accepting props to:
- Customize the button text.
- Apply custom styles or CSS classes.
- Handle click events.
Here's an example of how the component can be enhanced:
type ButtonProps = { text: string; onClick: () => void; className?: string; }; export function Button({ text, onClick, className }: ButtonProps) { return ( <button type="button" onClick={onClick} className={className}> {text} </button> ); }
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
Files selected for processing (9)
- apps/website/app/components/button.tsx (1 hunks)
- libs/storybook-host/.storybook/custom-viewports.ts (1 hunks)
- libs/storybook-host/.storybook/main.ts (1 hunks)
- libs/storybook-host/.storybook/preview.tsx (1 hunks)
- libs/storybook-host/package.json (1 hunks)
- libs/storybook-host/src/button.stories.tsx (1 hunks)
- libs/storybook-host/src/button.tsx (1 hunks)
- libs/storybook-host/tsconfig.json (1 hunks)
- package.json (4 hunks)
Files skipped from review due to trivial changes (2)
- libs/storybook-host/package.json
- libs/storybook-host/src/button.tsx
Files skipped from review as they are similar to previous changes (3)
- libs/storybook-host/.storybook/main.ts
- libs/storybook-host/tsconfig.json
- package.json
Additional comments not posted (7)
libs/storybook-host/.storybook/custom-viewports.ts (1)
1-40: LGTM!The custom viewport configurations are well-defined, covering a good range of common display resolutions and aspect ratios. The code follows a consistent and readable structure, making it easy to understand and maintain.
Extending the default Storybook viewports with these custom configurations will provide greater flexibility for testing and showcasing UI components across various screen sizes, enhancing the overall development experience.
libs/storybook-host/src/button.stories.tsx (5)
6-21: LGTM!The
metaconfiguration object is correctly set up and follows the Storybook documentation guidelines. The inclusion of theonClickspy function is a good practice for tracking user interactions in the actions panel.
27-32: LGTM!The
Primarystory definition is correctly set up and provides a clear example of the primary button variant.
34-38: LGTM!The
Secondarystory definition is correctly set up and provides a clear example of the secondary button variant.
40-45: LGTM!The
Largestory definition is correctly set up and provides a clear example of the large button variant.
47-52: LGTM!The
Smallstory definition is correctly set up and provides a clear example of the small button variant.libs/storybook-host/.storybook/preview.tsx (1)
1-58: LGTM!The Storybook configuration in this file is comprehensive and well-structured. It covers essential aspects such as layout, actions, controls, sorting, viewports, and backgrounds. The use of custom viewports enhances the responsiveness of the components being showcased, and the predefined background colors provide a visually appealing presentation.
The code segment aligns with the PR objective of initializing Storybook using the Nx command. The configurations are set up correctly and follow best practices.
Great job on setting up a solid foundation for Storybook in this project!
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
libs/storybook-host/.storybook/themes-storybook-ui.ts (1)
82-116: Complete the light theme configuration.The
lightUIStorybookobject is a good starting point for providing a light theme option in Storybook. However, the majority of the properties are currently commented out, indicating that the configuration is incomplete.To ensure a fully functional light theme:
- Uncomment the relevant properties and provide appropriate values for each.
- Customize the color scheme, background colors, and other visual properties to create a cohesive and visually appealing light theme.
- Test the light theme thoroughly to ensure proper contrast, readability, and overall user experience.
By completing the light theme configuration, users will have the flexibility to switch between dark and light themes based on their preferences.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
Files selected for processing (5)
- libs/storybook-host/.storybook/globals.css (1 hunks)
- libs/storybook-host/.storybook/preview.tsx (1 hunks)
- libs/storybook-host/.storybook/themes-storybook-ui.ts (1 hunks)
- libs/storybook-host/tsconfig.storybook.json (1 hunks)
- package.json (4 hunks)
Files skipped from review as they are similar to previous changes (2)
- libs/storybook-host/tsconfig.storybook.json
- package.json
Additional comments not posted (11)
libs/storybook-host/.storybook/themes-storybook-ui.ts (2)
3-80: LGTM!The
darkUIStorybookobject is well-structured and provides a comprehensive dark theme configuration for Storybook's UI. The use of HSL color values allows for precise color management, and the included properties cover various aspects of the UI, ensuring a cohesive dark theme experience.The commented out properties provide additional context and alternative values, which can be helpful for future customization or reference.
118-123: LGTM!The
commonThemeobject effectively encapsulates the common branding properties for the Storybook UI. The included properties, such asbrandTitle,brandUrl,brandTarget, andbrandImage, provide relevant information for consistent branding across different theme configurations.The naming of the properties is clear and self-explanatory, making it easy to understand their purpose. The
brandImageproperty allows for convenient customization of the logo displayed in the Storybook UI.This approach promotes reusability and maintainability by centralizing the branding information in a single object.
libs/storybook-host/.storybook/preview.tsx (5)
1-19: LGTM!The imports are correctly set up and the necessary dependencies are included. The custom configurations are also imported correctly.
20-105: LGTM!The Storybook configurations are correctly set up in the
previewobject. The configurations include:
- Layout set to 'fullscreen'
- Action handlers defined via a regex pattern
- Control matchers for color and date types
- Sorting method set to alphabetical order
- Custom viewports for responsiveness
- Dark mode with custom UI themes
- Background colors defined
No issues found in the configurations.
106-129: LGTM!The custom container for documentation is correctly set up. It listens to the dark mode event and updates the theme accordingly, ensuring consistency across the Storybook interface. No issues found in the implementation.
130-143: LGTM!The HTML addon for Storybook is correctly configured with Prettier settings and highlighter options. The configurations are set up correctly and no issues are found.
145-169: LGTM!The decorators for the Storybook preview are correctly set up. The decorators include:
- A theme provider with the default theme and transition options
- A theme switcher with the default theme and class names for light and dark themes
No issues found in the decorator setup.
libs/storybook-host/.storybook/globals.css (4)
1-4: LGTM!The Tailwind configuration is correctly imported and applied.
31-197: LGTM!The CSS variables for theming and dark mode are correctly defined using the HSL color format. The variables cover a comprehensive set of UI elements, providing a solid foundation for consistent styling throughout the application.
199-207: LGTM!The code segment correctly applies the border color to all elements and sets the background color and text color for the
bodyelement using the appropriate utility classes.
209-242: LGTM!The utility class for backdrop blur and the keyframe animations for the "bobbing" effect are correctly defined. The utility classes for applying the "bobbing" animations with varying durations provide a nice visual enhancement that can be easily reused across different components.
MFarabi619
force-pushed
the
mfarabi/feat/96-add-storybook-with-tailwindcss-shadcn-ui-and-chromatic-visual-test-server
branch
from
328c8a5
to
6a38a85
Compare
|
Regarding Babel, all Storybook needs to work is to see a I'm surprised you got it to work with Babel in the first place, that genuinely deserves an award 😭 |
Handed off to you and Jeremy as per discord messages, feel free to take it out. I don't see us needing Babel anywhere else, and this was scaffolded by the nx storybook command I used lmao. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
libs/storybook-host/.storybook/main.ts (2)
49-63: Consider removing unused commented-out code for better maintainabilityThe configuration for
@storybook/addon-storysourceis currently commented out. If this addon is not going to be used, consider removing the commented-out code to enhance readability and maintainability of the configuration file.
65-88: Clean up commented-out addons to streamline the configurationSeveral addons, such as
@storybook/addon-onboarding,@storybook/addon-toolbars, and@whitespace/storybook-addon-html, are commented out in theaddonsarray. If these addons are no longer needed, removing the commented code can help keep the configuration file clean and easier to navigate.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- libs/storybook-host/.storybook/main.ts (1 hunks)
🔇 Additional comments (1)
libs/storybook-host/.storybook/main.ts (1)
6-104: Configuration is comprehensive and well-structuredThe Storybook configuration is well-organized, and the selection of addons and custom plugins appears appropriate for the project's needs. The custom ordering and inclusion of specific addons provide a tailored development environment.
MFarabi619
force-pushed
the
mfarabi/feat/96-add-storybook-with-tailwindcss-shadcn-ui-and-chromatic-visual-test-server
branch
from
5a71141
to
f79dbae
Compare
❌ Deploy Preview for website-cuhacking failed. Why did it fail? →
|
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Outside diff range and nitpick comments (29)
libs/ui/src/shadcn/cn.ts (2)
4-6: LGTM: Efficient implementation of class name utility function.The
cnfunction is well-implemented, combiningclsxandtwMergeto efficiently handle dynamic class names and resolve conflicts in Tailwind CSS classes. This approach is ideal for React components using Tailwind CSS.Consider renaming the function to something more descriptive, like
combineClassNamesormergeClasses, to improve readability and self-documentation of the code. However, ifcnis a widely accepted convention in your team or the broader community, it's fine to keep it as is.
1-6: Overall assessment: Well-implemented utility function for class name management.This file introduces a concise and efficient utility function for managing class names in a React project using Tailwind CSS. The implementation follows best practices by leveraging
clsxfor conditional class names andtwMergefor resolving Tailwind class conflicts. The function is exported, making it easily accessible throughout the project.Consider documenting the usage of this utility function in your project's README or developer guidelines to ensure consistent usage across the codebase and to help onboard new developers quickly.
libs/ui/src/lib/ui.tsx (2)
1-3: Consider using a more descriptive name for the component.The component name
Uiis quite generic. In React, it's a best practice to use more descriptive, PascalCase names for components. Consider renaming it to something that better describes its purpose or content, such asWelcomeMessageorUiDemo.
11-11: Consider removing the default export.Having both a named export and a default export for the same component can lead to inconsistent import styles across the project. It's generally better to stick to one export style consistently. Since you're already using a named export, consider removing the default export.
You can simply remove line 11:
-export default UiThis will encourage consistent use of named imports throughout your project.
libs/ui/tsconfig.storybook.json (2)
6-14: LGTM: Comprehensive include patterns for Storybook files.The include patterns cover all common story file extensions and Storybook configuration files, ensuring that TypeScript processes all relevant files for Storybook.
Consider simplifying the include patterns for better maintainability:
"include": [ - "src/**/*.stories.ts", - "src/**/*.stories.js", - "src/**/*.stories.jsx", - "src/**/*.stories.tsx", - "src/**/*.stories.mdx", + "src/**/*.stories.@(ts|js|jsx|tsx|mdx)", ".storybook/*.js", ".storybook/*.ts" ],This change uses a more concise glob pattern to match all story file types.
15-15: LGTM: Appropriate exclusion of test files.Excluding test files from the Storybook TypeScript configuration is a good practice. It prevents potential conflicts and keeps the Storybook build process focused on story files.
Consider expanding the exclude pattern to cover JavaScript test files as well:
- "exclude": ["src/**/*.spec.ts", "src/**/*.test.ts"] + "exclude": ["src/**/*.@(spec|test).@(ts|js)"]This change will exclude both TypeScript and JavaScript test files, ensuring consistency across your project.
libs/ui/postcss.config.js (2)
3-6: Informative comment, consider adding version informationThe comment block provides valuable guidance on configuration usage and potential conflicts. It's great that you've included a link to the Nx documentation for further information.
Consider adding the Nx version number for which this configuration is valid. This can help future-proof the configuration and make it easier for developers to troubleshoot if the recommended setup changes in future Nx versions.
8-15: LGTM: Well-structured PostCSS configurationThe configuration object is well-structured and includes both Tailwind CSS and Autoprefixer, which is a good combination for modern CSS processing.
For improved robustness, consider adding error handling for the case where the Tailwind config file doesn't exist. Here's a suggested modification:
const { join } = require('node:path') +const { existsSync } = require('node:fs') + +const tailwindConfigPath = join(__dirname, 'tailwind.config.js') + +if (!existsSync(tailwindConfigPath)) { + console.warn(`Warning: Tailwind config file not found at ${tailwindConfigPath}`) +} module.exports = { plugins: { tailwindcss: { - config: join(__dirname, 'tailwind.config.js'), + config: tailwindConfigPath, }, autoprefixer: {}, }, }This change will provide a warning if the Tailwind config file is missing, which can help developers quickly identify configuration issues.
libs/ui/.storybook/custom-viewports.ts (1)
5-39: LGTM with suggestions: Comprehensive custom viewport configurationsThe custom viewport configurations cover a good range of common resolutions, including ultrawide (21:9) displays. The structure is consistent and well-organized.
Suggestions for improvement:
Consider using more descriptive names for the viewports. For example:
- '720p' -> 'hdReady'
- '1080p' -> 'fullHd'
- '2k' -> 'quadHd'
- '4k' -> 'ultraHd'
- '21/9' -> 'ultraWide'
Add a
typeproperty to each viewport for better categorization. For example:'1080p': { name: 'Full HD', type: 'desktop', styles: { width: '1920px', height: '1080px', }, },Consider adding more common mobile and tablet viewports if they're not already included in INITIAL_VIEWPORTS.
Would you like me to provide a refactored version of the custom viewports with these improvements?
libs/ui/eslint.config.js (3)
1-6: Address TODO comment and clean up importsThere are a few points to address in this section:
- The TODO comment suggests merging with antfu eslint config. Consider creating an issue to track this task if it hasn't been done already.
- There are several commented-out import statements. If these are no longer needed, consider removing them to keep the code clean.
- The active import uses ES module syntax (
import ... from) while the commented-out ones use CommonJS (require()). Ensure consistency in the import style across the project.Would you like me to create an issue for the TODO item or help clean up the unused imports?
7-12: Approve structure with minor suggestionThe structure of the exported configuration is good. It allows for asynchronous loading of the base configuration and easy extension. However, consider this minor improvement:
Instead of using an immediately invoked async function, you could use a named async function and export its invocation. This might improve readability:
async function createConfig() { const baseConfig = await baseConfigPromise; return [ ...baseConfig, // Additional configurations... ]; } export default createConfig();This change would make the code slightly more explicit about its intentions.
Also applies to: 41-42
13-40: Consider removing or documenting commented configurationsThe file contains large blocks of commented-out configurations. While these might serve as references for future additions, they can make the file harder to read and maintain.
Consider the following options:
- If these configurations are intended for future use, document them in a separate file or in the project's documentation, then remove them from this file.
- If some of these configurations are ready to be used, uncomment and activate them.
- If these are just examples or no longer relevant, remove them entirely.
This will help keep the configuration file clean and focused on active rules.
Would you like assistance in creating a separate documentation file for these configuration options?
libs/ui/src/shadcn/button.stories.tsx (2)
11-45: LGTM: Well-structured Storybook metadata with room for enhancement.The metadata configuration is comprehensive and well-organized. It provides good control over the Button component's props in the Storybook UI. The use of the 'autodocs' tag is excellent for automatic documentation generation.
Consider adding a description field to the metadata to provide more context about the Button component. For example:
description: 'A versatile button component with various styles and states.',This would enhance the auto-generated documentation.
57-97: LGTM: Well-implemented advanced button stories with room for enhancement.The Icon, WithIcon, and Loading stories are well-implemented, showcasing different advanced uses of the Button component. The use of React fragments and proper className assignments for icons is commendable.
Consider adding an accessibility enhancement to the Icon story:
export const Icon: Story = { args: { variant: 'outline', size: 'icon', children: <ChevronRightIcon className="size-4" />, 'aria-label': 'Next', // Add this line }, // ... rest of the story configuration }This
aria-labeladdition would improve the accessibility of the icon-only button by providing a text alternative for screen readers.libs/ui/.storybook/main.ts (2)
6-85: LGTM: Comprehensive addon configuration with room for optimizationThe addon configuration is well-structured and includes a wide range of useful tools for UI development and testing. The comments provide helpful context for each addon's purpose.
Consider reviewing the commented-out addons (@storybook/addon-onboarding, @storybook/addon-toolbars, @whitespace/storybook-addon-html) to determine if they would be beneficial to include in the current setup.
Would you like assistance in evaluating these additional addons for potential inclusion?
92-96: LGTM: Proper export and helpful comments for future customizationThe configuration object is correctly exported as default. The comments provide valuable information about customizing webpack configuration in the future.
Consider adding a TODO comment to remind the team to review and potentially implement webpack customizations when needed in the future.
Would you like me to suggest a TODO comment to add?
libs/ui/.storybook/themes-storybook-ui.ts (1)
3-80: Comprehensive dark theme implementation with suggestions for improvementThe
darkUIStorybookconstant provides a thorough dark theme configuration for the Storybook UI. However, there are a few areas that could be improved:
There are several commented-out lines with alternative color values using HSL notation. Consider cleaning up the code by removing unused comments or standardizing the color notation (either use HSL or hexadecimal consistently).
If the alternative color values are intended for future use or comparison, consider moving them to a separate constant or documenting their purpose clearly.
Some properties are duplicated with different values (commented and uncommented). Resolve these duplications to avoid confusion and potential bugs.
Would you like assistance in cleaning up the commented code and standardizing the color notation?
libs/ui/.storybook/globals.css (5)
1-7: LGTM! Consider adding a brief description of the file's purpose.The Tailwind CSS setup and imports are correct. The comments provide useful resources for theme customization and exploring the Shadcn/ui ecosystem.
Consider adding a brief comment at the top of the file describing its purpose and importance in the project's styling architecture.
9-29: Evaluate the necessity of commented-out color definitions.These commented-out color definitions might be useful for reference, but they could also clutter the file. Consider one of the following options:
- Remove them if they are no longer needed.
- Move them to a separate file for future reference.
- If they are actively being used for comparison or future changes, add a comment explaining their purpose.
31-113: LGTM! Consider standardizing HSL value formatting for consistency.The CSS variables for the light mode are well-defined and comprehensive. The use of HSL colors allows for easy theming and manipulation.
For consistency, consider standardizing the format of HSL values. Some are written with spaces (e.g.,
196 100% 95%), while others use commas (e.g.,203, 100%, 37%). Adopting a single format throughout the file would improve readability.
115-196: LGTM! Address commented-out color definitions in dark mode.The dark mode color scheme is well-defined and complements the light mode. The structure and naming conventions are consistent, which is excellent for maintainability.
Similar to the light mode section, there are commented-out color definitions (lines 145-149). Consider removing or documenting these as suggested in the previous comment about commented-out colors.
199-222: LGTM! Consider parameterizing the bobbing animation.The additional styles and animation are well-defined and add useful functionality to the project.
Consider parameterizing the bobbing animation to make it more flexible. You could use CSS custom properties to allow easy customization of the animation distance and duration. For example:
@keyframes bobbing { 0%, 100% { transform: translateY(calc(var(--bobbing-distance, -2px) * -1)); } 50% { transform: translateY(var(--bobbing-distance, 2px)); } } .bobbing { animation: bobbing var(--bobbing-duration, 2s) infinite; }This way, you can easily adjust the animation for different elements by setting
--bobbing-distanceand--bobbing-durationon the element or its parent.apps/website/app/nx-welcome.tsx (2)
438-442: Button demonstrates UI library usageThe addition of the
Buttoncomponent effectively demonstrates the usage of the newly imported UI library. This is a good way to showcase the Storybook integration.Consider the following suggestions:
- Add an
onClickhandler to the button to demonstrate its functionality.- Consider using a more descriptive label for the button, related to your application's purpose.
- You might want to add a Storybook story for this
Buttoncomponent to fully leverage the Storybook integration.Would you like me to provide an example of how to implement these suggestions?
Line range hint
1-1023: Consider refactoring for improved maintainabilityWhile the additions related to Storybook are good, the overall structure of this component could be improved:
Separate Concerns: Consider moving the inline CSS to a separate stylesheet. This will improve readability and maintainability.
Component Breakdown: The component contains a large amount of static JSX. Consider breaking it down into smaller, reusable components. This aligns well with the component-driven development approach that Storybook encourages.
Data Abstraction: Static content like learning materials and links could be moved to a separate data file and mapped over in the JSX.
Accessibility: Ensure that all interactive elements are keyboard accessible and have appropriate ARIA attributes.
Styling Consistency: With the introduction of a UI library, consider using its styling system consistently throughout this component.
These changes will make the codebase more maintainable, easier to test, and more aligned with React and Storybook best practices.
Would you like me to provide an example of how to implement any of these suggestions?
libs/ui/tailwind.config.js (1)
60-62: Use consistent quotation marks for string valuesIn the
borderRadiusconfiguration (lines 60-62), there is an inconsistency in the use of quotation marks. Lines 60 and 61 use backticks (` `), while line 62 uses single quotes (' '). For consistency and readability, consider using single quotes for all string values unless template literals are necessary.Apply this diff to make the changes:
60 - lg: `var(--radius)`, 61 - md: `calc(var(--radius) - 2px)`, 62 sm: 'calc(var(--radius) - 4px)', + lg: 'var(--radius)', + md: 'calc(var(--radius) - 2px)',libs/ui/.storybook/preview.tsx (4)
5-5: Clarify or remove the inline comment for better readabilityThe inline comment on line 5 may be unclear or unnecessary. Consider clarifying its purpose or removing it to improve code readability.
46-60: Consider removing commented-out code to reduce clutterThere's a large block of commented-out code related to
globalTypes. If this code is no longer needed, removing it can help keep the codebase clean. If you plan to use it later, consider adding a comment explaining its purpose.
115-115: Remove unnecessary ESLint disable commentAfter wrapping the event listener in a
useEffecthook, the ESLint rulereact-hooks/rules-of-hooksis no longer violated. You can remove the// eslint-disable-next-line react-hooks/rules-of-hookscomment on line 115.
123-126: Remove commented-out debugging statementsThe commented-out
console.logstatements from lines 123 to 126 can be removed to clean up the code. If you need them for future debugging, consider adding a note explaining their purpose.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
libs/ui/.storybook/cuhacking-logo.pngis excluded by!**/*.pngpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (27)
- .gitignore (1 hunks)
- apps/website/app/nx-welcome.tsx (2 hunks)
- apps/website/app/tailwind.css (1 hunks)
- libs/ui/.babelrc (1 hunks)
- libs/ui/.storybook/custom-viewports.ts (1 hunks)
- libs/ui/.storybook/globals.css (1 hunks)
- libs/ui/.storybook/main.ts (1 hunks)
- libs/ui/.storybook/preview.tsx (1 hunks)
- libs/ui/.storybook/themes-storybook-ui.ts (1 hunks)
- libs/ui/README.md (1 hunks)
- libs/ui/eslint.config.js (1 hunks)
- libs/ui/package.json (1 hunks)
- libs/ui/postcss.config.js (1 hunks)
- libs/ui/project.json (1 hunks)
- libs/ui/src/index.ts (1 hunks)
- libs/ui/src/lib/ui.module.css (1 hunks)
- libs/ui/src/lib/ui.tsx (1 hunks)
- libs/ui/src/shadcn/button.stories.tsx (1 hunks)
- libs/ui/src/shadcn/button.tsx (1 hunks)
- libs/ui/src/shadcn/cn.ts (1 hunks)
- libs/ui/tailwind.config.js (1 hunks)
- libs/ui/tsconfig.json (1 hunks)
- libs/ui/tsconfig.lib.json (1 hunks)
- libs/ui/tsconfig.storybook.json (1 hunks)
- nx.json (3 hunks)
- package.json (4 hunks)
- tsconfig.base.json (1 hunks)
✅ Files skipped from review due to trivial changes (10)
- .gitignore
- apps/website/app/tailwind.css
- libs/ui/.babelrc
- libs/ui/README.md
- libs/ui/package.json
- libs/ui/project.json
- libs/ui/src/index.ts
- libs/ui/src/lib/ui.module.css
- libs/ui/tsconfig.json
- libs/ui/tsconfig.lib.json
🧰 Additional context used
🔇 Additional comments (28)
libs/ui/src/shadcn/cn.ts (1)
1-2: LGTM: Imports are correctly structured and necessary.The import statements are well-organized and import the required utilities from 'clsx' and 'tailwind-merge' libraries. These imports are essential for the
cnfunction implementation.libs/ui/tsconfig.storybook.json (3)
1-2: LGTM: Proper extension of base TypeScript configuration.The configuration correctly extends the base
tsconfig.json, which is a good practice for maintaining consistent TypeScript settings across the project while allowing Storybook-specific overrides.
1-16: Overall, the Storybook TypeScript configuration is well-structured.The
tsconfig.storybook.jsonfile is properly set up to extend the base TypeScript configuration and includes appropriate patterns for Storybook files. The suggestions provided in the review are minor optimizations to improve maintainability and consistency. Once these small adjustments are made, the configuration will be in excellent shape for your Storybook setup.
3-5: Verify the necessity ofemitDecoratorMetadata.The
emitDecoratorMetadataoption is enabled, which is typically used in Angular projects or with certain dependency injection systems. For a React-based Storybook setup, this option might not be necessary and could potentially increase bundle size.Please confirm if this option is required for your project. If not, consider removing it to optimize performance and bundle size. You can use the following script to check for decorator usage in your project:
✅ Verification successful
emitDecoratorMetadatais not required and can be removed.No decorator usages were found in the TypeScript files, indicating that the
emitDecoratorMetadataoption is unnecessary for this project. Removing it can help optimize performance and reduce the bundle size.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for decorator usage in TypeScript files # Test: Search for decorator usage (@decorator syntax) rg --type typescript '@\w+' -g '!*.d.ts'Length of output: 77
Script:
#!/bin/bash # Description: Check for decorator usage in TypeScript files # Search for decorator usage (@decorator syntax) in .ts and .tsx files rg '@\w+' --glob '*.ts' --glob '*.tsx' -nLength of output: 11003
Script:
#!/bin/bash # Description: Check for decorator usage in TypeScript files # Search for decorator usage (@Decorator syntax) in .ts and .tsx files, excluding import statements rg '^\s*@\w+' --glob '*.ts' --glob '*.tsx' -nLength of output: 126
libs/ui/postcss.config.js (1)
1-1: LGTM: Proper import of Node.js path moduleThe use of the
node:prefix for importing the built-inpathmodule is a best practice. It ensures clarity and avoids potential conflicts with third-party modules.libs/ui/.storybook/custom-viewports.ts (3)
1-1: LGTM: Correct import of INITIAL_VIEWPORTSThe import statement is correct and imports the necessary INITIAL_VIEWPORTS from the Storybook viewport addon.
3-4: LGTM: Well-structured export of customViewportsThe export statement is correct, and the structure allows for easy extension of INITIAL_VIEWPORTS with custom configurations. This approach preserves the default viewports while adding new ones.
1-40: Overall assessment: Well-implemented custom viewports with room for minor enhancementsThe
custom-viewports.tsfile successfully extends Storybook's default viewports with additional, useful configurations. The implementation is correct, consistent, and covers a good range of display resolutions. The suggestions provided earlier can further improve the clarity and extensibility of the custom viewports.Great job on setting up these custom viewports! They will significantly enhance the ability to test and showcase UI components across various screen sizes.
tsconfig.base.json (1)
17-17: LGTM! New UI library path mapping added.The addition of the "@cuhacking/ui" path mapping is consistent with the existing structure and aligns with the PR's objective of initializing Storybook. This will allow for easier imports of UI components across the project.
To ensure the referenced file exists, please run the following script:
✅ Verification successful
Path mapping verified successfully.
The existence of
libs/ui/src/index.tshas been confirmed. The addition of the"@cuhacking/ui"path mapping is correct and aligns with the project's structure.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the existence of the UI library entry point if [ -f "libs/ui/src/index.ts" ]; then echo "File exists: libs/ui/src/index.ts" else echo "Warning: File not found: libs/ui/src/index.ts" fiLength of output: 110
libs/ui/src/shadcn/button.stories.tsx (3)
1-9: LGTM: Imports are well-structured and appropriate.The imports are correctly set up for a Storybook file. The use of named imports from '@radix-ui/react-icons' is good for tree-shaking, and the TypeScript types are properly imported. The local Button component import is also correct.
47-48: LGTM: Correct type definitions and default export.The default export of the metadata and the Story type definition are correctly implemented for Storybook with TypeScript support.
50-55: LGTM: Comprehensive coverage of button variants.The stories for different button variants (Primary, Secondary, Destructive, Outline, Ghost, and Link) are well-defined. Each variant is correctly implemented as a separate story, allowing for easy visualization and testing of different button styles.
nx.json (2)
13-16: LGTM: Appropriate exclusions for Storybook filesThe added exclusions for Storybook-related files in the
productioninput are correct and align with best practices. This ensures that development-only Storybook artifacts are not included in production builds.
83-87: Clarify the decision to disable unit tests for React librariesThe new generator configuration for
@nx/reactsetsunitTestRunnertononefor libraries. This means that newly generated React libraries will not include unit tests by default.Could you please clarify if this is intentional and aligns with the project's testing strategy? If unit tests are important for your project, you might want to reconsider this setting.
To help understand the current testing setup, please run the following script:
#!/bin/bash # Description: Check for existing test files and configurations # Test: Check for jest.config files echo "Searching for jest.config files:" fd --type f --glob "jest.config.@(js|ts)" # Test: Check for test files echo "Searching for test files:" fd --type f --glob "*.@(spec|test).@(js|jsx|ts|tsx)" # Test: Check for testing-library dependencies echo "Checking for testing-library dependencies:" grep -E "@testing-library|jest" package.jsonlibs/ui/.storybook/main.ts (3)
1-3: LGTM: Proper import and configuration setupThe import statement and configuration object declaration are correctly implemented. The use of TypeScript for type-checking is a good practice that enhances code reliability.
4-5: LGTM: Comprehensive story file inclusionThe 'stories' configuration effectively includes all relevant story file formats (.mdx and stories.{js,jsx,ts,tsx}) from the '../src/' directory. This setup ensures that all story files are properly recognized by Storybook.
86-89: LGTM: Appropriate framework configurationThe framework configuration correctly specifies '@storybook/nextjs', which is suitable for a Next.js project. The empty options object provides flexibility for future customizations if needed.
libs/ui/.storybook/themes-storybook-ui.ts (1)
118-123: Well-structured common theme implementationThe
commonThemeconstant is well-implemented, providing essential branding information for the Storybook UI. The use of a separate constant for common theme elements promotes reusability across different theme variations.package.json (5)
77-92: Excellent addition of Storybook and its ecosystem!The inclusion of Storybook and its various addons (a11y, actions, backgrounds, controls, etc.) provides a robust environment for UI component development and testing. Using the latest version (8.3.5) ensures access to the most recent features and security updates.
This setup will greatly enhance the team's ability to develop, test, and document UI components in isolation, improving overall development efficiency and collaboration.
Also applies to: 144-145
73-74: Great additions for enhanced UI development!The new dependencies significantly improve the project's UI development capabilities:
- Radix UI components (
@radix-ui/react-icons,@radix-ui/react-slot) provide accessible and customizable UI primitives.- Utilities like
class-variance-authority,clsx, andtailwind-mergeenhance CSS class management, particularly useful with Tailwind CSS.tailwindcss-animateadds animation utilities to Tailwind CSS.next-themesenables easy theme switching in Next.js applications.These additions will contribute to a more robust and flexible UI component system, improving developer productivity and user experience.
Also applies to: 113-114, 136-136, 146-146, 148-148
66-66: Nx package additions align with project objectives.The addition of Nx packages, particularly
@nx/reactand@nx/storybook, aligns well with the PR objective of initializing Storybook using Nx commands. The consistent use of versions 19.5.7 and 19.6.0 across different Nx packages ensures compatibility.To maintain optimal performance and access to new features, consider setting up a process to keep these Nx packages updated in the future.
Also applies to: 68-68
Line range hint
1-164: Summary of package.json changesThe updates to
package.jsonsignificantly enhance the project's development capabilities:
- Storybook integration provides a robust environment for UI component development.
- UI-related packages improve component styling and theming capabilities.
- Nx package additions support the project's build and development workflow.
- The Prettier update brings the latest formatting features but requires careful integration.
These changes align well with the PR objectives and should improve the overall development experience. However, please address the concerns raised about Babel dependencies and the Prettier update to ensure a smooth integration of these changes.
49-50:⚠️ Potential issueConsider removing Babel dependencies if not explicitly required.
The addition of
@babel/coreand@babel/preset-reactmight be unnecessary for this project:
- Next.js (which is used in this project) already includes built-in support for JSX transformation and modern JavaScript features.
- Storybook 8.x for Next.js typically doesn't require explicit Babel configuration.
Unless there's a specific need for custom Babel transformations, consider removing these dependencies to simplify the project setup.
To verify if Babel is being used in the project, run the following command:
If no results are found, it's likely safe to remove the Babel dependencies.
✅ Verification successful
Babel dependencies are unused and can be removed.
No Babel configuration files or Babel-related scripts were found in the project, indicating that
@babel/coreand@babel/preset-reactare likely unnecessary.
- Remove
@babel/coreand@babel/preset-reactfrompackage.jsondevDependencies.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for Babel configuration files or usage in the project echo "Searching for Babel configuration files:" fd -e babelrc -e babel.config.js -e babel.config.json echo "Searching for Babel usage in package.json scripts:" jq '.scripts | to_entries[] | select(.value | contains("babel"))' package.jsonLength of output: 338
libs/ui/.storybook/globals.css (1)
1-222: Excellent foundation for project-wide styling and theming.This
globals.cssfile provides a robust and flexible foundation for styling and theming in the project. The use of Tailwind CSS combined with custom CSS variables creates a powerful system for maintaining consistent design across the application. The inclusion of both light and dark mode color schemes demonstrates good forethought for accessibility and user preferences.Key strengths:
- Comprehensive color scheme definitions
- Flexible use of CSS variables and HSL colors
- Integration with Tailwind CSS
- Provision for both light and dark modes
- Addition of utility classes and custom animations
While there are a few minor suggestions for improvement (as noted in previous comments), the overall quality and structure of this file are excellent. It will serve as a crucial component in maintaining a consistent and easily customizable design system for the project.
apps/website/app/nx-welcome.tsx (1)
1-1: LGTM: Import aligns with Storybook initializationThe import of the
Buttoncomponent from@cuhacking/uiis consistent with the PR's objective of initializing Storybook. This suggests that a UI library has been set up, which is a good practice for component-driven development.libs/ui/src/shadcn/button.tsx (1)
1-57: LGTMThe implementation of the
Buttoncomponent is well-structured, and the use ofcvafor managing variants is appropriate. Good job on implementing the customizable button with various styling options.libs/ui/tailwind.config.js (2)
65-66: Confirm that '--font-sans' CSS variable is definedIn the
fontFamilyconfiguration (lines 65-66), you're using the CSS variable--font-sans. Ensure that this variable is defined in your CSS so that the custom font is applied correctly.Would you like assistance in verifying where
--font-sansis defined?
83-83: Ensure 'tailwindcss-animate' plugin is installedIn your plugins array (line 83), you are requiring
'tailwindcss-animate'. Please verify that this plugin is installed and listed in your project's dependencies to prevent any runtime errors.Run the following script to check if the plugin is installed:
If the output is
null, the plugin is not installed.✅ Verification successful
Verified that 'tailwindcss-animate' is installed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that 'tailwindcss-animate' is installed. # Test: Check if 'tailwindcss-animate' is listed in package.json dependencies. jq '.dependencies["tailwindcss-animate"] // .devDependencies["tailwindcss-animate"]' package.jsonLength of output: 108
libs/ui/src/lib/ui.tsx
Outdated
| return ( | ||
| <div className={styles.container}> | ||
| <h1>Welcome to Ui!</h1> | ||
| </div> | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance component flexibility with props.
The current implementation uses a static welcome message. To make this component more reusable, consider accepting the message as a prop. This would allow the component to be used in various contexts with different messages.
Here's a suggested implementation:
-export function Ui() {
+export function Ui({ message = 'Welcome to Ui!' }: { message?: string }) {
return (
<div className={styles.container}>
- <h1>Welcome to Ui!</h1>
+ <h1>{message}</h1>
</div>
)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return ( | |
| <div className={styles.container}> | |
| <h1>Welcome to Ui!</h1> | |
| </div> | |
| ) | |
| export function Ui({ message = 'Welcome to Ui!' }: { message?: string }) { | |
| return ( | |
| <div className={styles.container}> | |
| <h1>{message}</h1> | |
| </div> | |
| ) | |
| } |
nx.json
Outdated
| }, | ||
| { | ||
| "plugin": "@nx/storybook/plugin", | ||
| "options": { | ||
| "serveStorybookTargetName": "storybook", | ||
| "buildStorybookTargetName": "build-storybook", | ||
| "testStorybookTargetName": "test-storybook", | ||
| "staticStorybookTargetName": "static-storybook" | ||
| } |
There was a problem hiding this comment.
💡 Codebase verification
Issue: Incomplete Storybook Configuration
.storybookdirectory is missing..storybook/main.jsor.storybook/main.tsis not found..storybook/preview.jsor.storybook/preview.tsis not found.
🔗 Analysis chain
LGTM: Storybook plugin correctly added
The @nx/storybook/plugin has been properly configured with the standard target names for Storybook integration in an Nx workspace. This aligns with the PR objective of initializing Storybook.
To ensure the Storybook setup is complete, please run the following verification script:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify Storybook configuration files exist
# Test: Check for .storybook configuration directory
if [ -d ".storybook" ]; then
echo ".storybook directory exists"
else
echo "Warning: .storybook directory not found"
fi
# Test: Check for main.js in .storybook
if [ -f ".storybook/main.js" ] || [ -f ".storybook/main.ts" ]; then
echo ".storybook/main.js or .storybook/main.ts exists"
else
echo "Warning: .storybook/main.js or .storybook/main.ts not found"
fi
# Test: Check for preview.js in .storybook
if [ -f ".storybook/preview.js" ] || [ -f ".storybook/preview.ts" ]; then
echo ".storybook/preview.js or .storybook/preview.ts exists"
else
echo "Warning: .storybook/preview.js or .storybook/preview.ts not found"
fi
# Test: Check for any .stories files
if fd --type f --glob "*.stories.@(js|jsx|ts|tsx|mdx)"; then
echo "Story files found"
else
echo "Warning: No story files found"
fi
Length of output: 879
Script:
#!/bin/bash
# Description: Verify Storybook configuration files exist
# Test: Check for .storybook configuration directory
if [ -d ".storybook" ]; then
echo ".storybook directory exists"
else
echo "Warning: .storybook directory not found"
fi
# Test: Check for main.js in .storybook
if [ -f ".storybook/main.js" ] || [ -f ".storybook/main.ts" ]; then
echo ".storybook/main.js or .storybook/main.ts exists"
else
echo "Warning: .storybook/main.js or .storybook/main.ts not found"
fi
# Test: Check for preview.js in .storybook
if [ -f ".storybook/preview.js" ] || [ -f ".storybook/preview.ts" ]; then
echo ".storybook/preview.js or .storybook/preview.ts exists"
else
echo "Warning: .storybook/preview.js or .storybook/preview.ts not found"
fi
# Test: Check for any .stories files
if fd --type f --glob '*.stories.@(js|jsx|ts|tsx|mdx)'; then
echo "Story files found"
else
echo "Warning: No story files found"
fi
Length of output: 615
| export const lightUIStorybook = { | ||
| fontBase: | ||
| '"Nunito Sans", -apple-system, ".SFNSText-Regular", "San Francisco", BlinkMacSystemFont, "Segoe UI", "Helvetica Neue", Helvetica, Arial, sans-serif', | ||
| fontCode: | ||
| 'ui-monospace, Menlo, Monaco, "Roboto Mono", "Oxygen Mono", "Ubuntu Monospace", "Source Code Pro", "Droid Sans Mono", "Courier New", monospace', | ||
| // --background: 0 0% 100%; | ||
| // --foreground: 222.2 84% 4.9%; | ||
| // | ||
| // --card: 0 0% 100%; | ||
| // --card-foreground: 222.2 84% 4.9%; | ||
| // | ||
| // --popover: 0 0% 100%; | ||
| // --popover-foreground: 222.2 84% 4.9%; | ||
| // | ||
| // --primary: 222.2 47.4% 11.2%; | ||
| // --primary-foreground: 210 40% 98%; | ||
| // | ||
| // --secondary: 210 40% 96.1%; | ||
| // --secondary-foreground: 222.2 47.4% 11.2%; | ||
| // | ||
| // --muted: 210 40% 96.1%; | ||
| // --muted-foreground: 215.4 16.3% 46.9%; | ||
| // | ||
| // --accent: 210 40% 96.1%; | ||
| // --accent-foreground: 222.2 47.4% 11.2%; | ||
| // | ||
| // --destructive: 0 84.2% 60.2%; | ||
| // --destructive-foreground: 210 40% 98%; | ||
| // | ||
| // --border: 214.3 31.8% 91.4%; | ||
| // --input: 214.3 31.8% 91.4%; | ||
| // --ring: 222.2 84% 4.9%; | ||
| // | ||
| // --radius: 0.5rem; | ||
| } |
There was a problem hiding this comment.
Incomplete light theme implementation
The lightUIStorybook constant is currently incomplete compared to the dark theme. This inconsistency may lead to issues when switching between themes. Consider the following suggestions:
-
Complete the light theme implementation to match the structure of the dark theme. This ensures consistency and proper functionality when switching between themes.
-
If the light theme is intentionally left incomplete (e.g., work in progress), please add a comment explaining its status and any pending tasks.
-
Remove or implement the commented-out CSS variables. If they are intended for future use, consider moving them to a separate constant or file for clarity.
Would you like assistance in completing the light theme implementation or documenting its current status?
package.json
Outdated
| "nx": "19.5.7", | ||
| "postcss": "8.4.38", | ||
| "prettier": "^2.6.2", | ||
| "prettier": "^3.3.3", |
There was a problem hiding this comment.
💡 Codebase verification
ESLint and Prettier configurations are missing.
While eslint-config-prettier is present in devDependencies, there are no ESLint or Prettier configuration files found in the repository. To ensure proper integration and avoid potential formatting conflicts:
Action items:
- Add an ESLint configuration file (e.g.,
.eslintrc.json) to define linting rules. - Add a Prettier configuration file (e.g.,
.prettierrc) to specify formatting preferences. - Ensure integration between ESLint and Prettier by extending
eslint-config-prettierin the ESLint configuration. - Run linting and formatting checks to verify that the configurations work as expected.
🔗 Analysis chain
Review Prettier update for potential breaking changes.
Prettier has been updated from version 2.6.2 to 3.3.3, which is a major version jump. While this update brings the latest features and improvements, it may also introduce breaking changes in code formatting.
Action items:
- Review the Prettier changelog for any breaking changes between versions 2.x and 3.x.
- Run Prettier on a sample of your codebase to ensure the formatting changes are acceptable.
- Update any Prettier-related configurations or scripts if necessary.
- Ensure this update doesn't conflict with other linting or formatting tools in your project.
To check for potential conflicts with ESLint, run the following command:
If both ESLint and Prettier configurations are found, ensure they are properly integrated to avoid conflicts.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for ESLint and Prettier configuration files
echo "ESLint configuration files:"
fd -e eslintrc -e eslintrc.js -e eslintrc.json
echo "Prettier configuration files:"
fd -e prettierrc -e prettierrc.js -e prettierrc.json
echo "Checking for eslint-config-prettier in package.json:"
jq '.devDependencies | has("eslint-config-prettier")' package.json
Length of output: 426
Script:
#!/bin/bash
# Check for inline ESLint and Prettier configurations in package.json
echo "Checking for 'eslintConfig' in package.json:"
jq '.eslintConfig' package.json
echo "Checking for 'prettier' configuration in package.json:"
jq '.prettier' package.json
Length of output: 299
libs/ui/src/shadcn/button.tsx
Outdated
| const Comp = asChild ? Slot : 'button' | ||
| return ( | ||
| <Comp | ||
| className={cn(buttonVariants({ variant, size, className }))} |
There was a problem hiding this comment.
Fix className merging to include custom class names
Passing className directly into buttonVariants may not merge the custom class names as intended. To ensure that any custom className props are properly merged with the variant classes, consider adjusting the className assignment.
Apply this diff to fix the class name merging:
- className={cn(buttonVariants({ variant, size, className }))}
+ className={cn(buttonVariants({ variant, size }), className)}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| className={cn(buttonVariants({ variant, size, className }))} | |
| className={cn(buttonVariants({ variant, size }), className)} |
libs/ui/tailwind.config.js
Outdated
| join( | ||
| __dirname, | ||
| '{src,pages,components,app}/**/*!(*.stories|*.spec).{ts,tsx,html}', | ||
| ), | ||
| ...createGlobPatternsForDependencies(__dirname), | ||
| ], |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Include Storybook files in Tailwind's content paths
In the content array (lines 9-14), you are excluding files ending with .stories and .spec. If you are using Tailwind CSS classes within your Storybook stories, excluding .stories files might prevent Tailwind from generating the necessary styles for your Storybook components. Consider including .stories files to ensure all styles are applied in Storybook.
You can adjust the content pattern to include .stories files:
11 - '{src,pages,components,app}/**/*!(*.stories|*.spec).{ts,tsx,html}',
+ '{src,pages,components,app}/**/*!(*.spec).{ts,tsx,html}',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| join( | |
| __dirname, | |
| '{src,pages,components,app}/**/*!(*.stories|*.spec).{ts,tsx,html}', | |
| ), | |
| ...createGlobPatternsForDependencies(__dirname), | |
| ], | |
| join( | |
| __dirname, | |
| '{src,pages,components,app}/**/*!(*.spec).{ts,tsx,html}', | |
| ), | |
| ...createGlobPatternsForDependencies(__dirname), | |
| ], |
libs/ui/.storybook/preview.tsx
Outdated
| // currentProps.theme = isDark ? themes.dark : themes.light | ||
| currentProps.theme = isDark | ||
| ? (darkUIStorybook as ThemeVars) | ||
| : themes.light |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use consistent theme variables for light mode
In the DocsContainer, when setting currentProps.theme, you're using darkUIStorybook for dark mode but themes.light for light mode. For consistency and to apply your custom light theme, consider using lightUIStorybook instead.
Apply this change:
currentProps.theme = isDark
- ? (darkUIStorybook as ThemeVars)
- : themes.light
+ ? (darkUIStorybook as ThemeVars)
+ : (lightUIStorybook as ThemeVars)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // currentProps.theme = isDark ? themes.dark : themes.light | |
| currentProps.theme = isDark | |
| ? (darkUIStorybook as ThemeVars) | |
| : themes.light | |
| // currentProps.theme = isDark ? themes.dark : themes.light | |
| currentProps.theme = isDark | |
| ? (darkUIStorybook as ThemeVars) | |
| : (lightUIStorybook as ThemeVars) |
libs/ui/.storybook/preview.tsx
Outdated
| // eslint-disable-next-line react-hooks/rules-of-hooks | ||
| const [isDark, setDark] = React.useState(true) | ||
| props.context.channel.on(DARK_MODE_EVENT_NAME, state => | ||
| setDark(state)) |
There was a problem hiding this comment.
Wrap event listener in useEffect to prevent memory leaks
The event listener for DARK_MODE_EVENT_NAME is added directly in the render function without cleanup. This can lead to memory leaks because the listener is not removed when the component unmounts. Wrap the event listener in a useEffect hook with a cleanup function to manage it properly.
Apply this change:
- // eslint-disable-next-line react-hooks/rules-of-hooks
- const [isDark, setDark] = React.useState(true)
- props.context.channel.on(DARK_MODE_EVENT_NAME, state =>
- setDark(state))
+ const [isDark, setDark] = React.useState(true)
+ React.useEffect(() => {
+ const handleDarkMode = (state: boolean) => setDark(state)
+ props.context.channel.on(DARK_MODE_EVENT_NAME, handleDarkMode)
+ return () => {
+ props.context.channel.off(DARK_MODE_EVENT_NAME, handleDarkMode)
+ }
+ }, [props.context.channel])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // eslint-disable-next-line react-hooks/rules-of-hooks | |
| const [isDark, setDark] = React.useState(true) | |
| props.context.channel.on(DARK_MODE_EVENT_NAME, state => | |
| setDark(state)) | |
| const [isDark, setDark] = React.useState(true) | |
| React.useEffect(() => { | |
| const handleDarkMode = (state: boolean) => setDark(state) | |
| props.context.channel.on(DARK_MODE_EVENT_NAME, handleDarkMode) | |
| return () => { | |
| props.context.channel.off(DARK_MODE_EVENT_NAME, handleDarkMode) | |
| } | |
| }, [props.context.channel]) |
|
Closing as stale |
MFarabi619
force-pushed
the
mfarabi/feat/96-add-storybook-with-tailwindcss-shadcn-ui-and-chromatic-visual-test-server
branch
from
f79dbae
to
9916411
Compare
✅ Deploy Preview for docs-cuhacking ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Actionable comments posted: 16
🧹 Outside diff range and nitpick comments (20)
libs/ui/src/shadcn/cn.ts (1)
4-6: Add JSDoc documentation for better maintainability.The implementation is correct and follows best practices. Consider adding JSDoc documentation to explain the function's purpose and provide usage examples.
+/** + * Merge multiple class names into a single string, resolving Tailwind CSS conflicts. + * @param inputs - Class names to merge (strings, objects, or arrays) + * @returns Merged class names string + * @example + * ```ts + * // Basic usage + * cn('px-2 py-1', 'bg-blue-500') // 'px-2 py-1 bg-blue-500' + * + * // With conditional classes + * cn('px-2', { 'bg-blue-500': true, 'bg-red-500': false }) + * + * // Resolving conflicts + * cn('px-2 py-1', 'px-4') // 'py-1 px-4' + * ``` + */ export function cn(...inputs: ClassValue[]) { return twMerge(clsx(inputs)) }libs/ui/src/lib/ui.tsx (2)
3-9: Add TypeScript types and component documentation.Since this is a UI component that will be showcased in Storybook, it should have proper TypeScript types and documentation.
Add JSDoc and types:
+/** + * Primary UI component for user interaction + * + * @component + * @example + * return ( + * <Ui /> + * ) + */ -export function Ui() { +export function Ui(): JSX.Element { return ( <div className={styles.container}> <h1>Welcome to Ui!</h1> </div> ) }
1-11: Add Storybook story file.Since this PR is about initializing Storybook, we should add a story file for this component.
Would you like me to generate a Storybook story file (
ui.stories.tsx) for this component? It would include:
- Basic story setup
- Component documentation
- Different variations/states of the component
- Controls for any props (once they're added)
libs/ui/.storybook/custom-viewports.ts (1)
1-40: Consider adding type safety and documentation.To improve maintainability and type safety:
- Add TypeScript interfaces for viewport configurations
- Consider extracting dimensions as named constants
- Add JSDoc comments explaining the purpose of each viewport
Here's a suggested implementation:
import { INITIAL_VIEWPORTS } from '@storybook/addon-viewport'; interface ViewportStyle { width: string; height: string; } interface ViewportConfig { name: string; styles: ViewportStyle; } /** * Common display resolutions for testing responsive designs * @constant */ const VIEWPORT_SIZES = { HD: { width: 1280, height: 720 }, FULL_HD: { width: 1920, height: 1080 }, // ... other resolutions } as const; /** * Custom viewport configurations extending Storybook's default viewports * @exports */ export const customViewports = { ...INITIAL_VIEWPORTS, hd_720p: { name: `HD (${VIEWPORT_SIZES.HD.width}x${VIEWPORT_SIZES.HD.height})`, styles: { width: `${VIEWPORT_SIZES.HD.width}px`, height: `${VIEWPORT_SIZES.HD.height}px`, }, }, // ... other viewports } satisfies Record<string, ViewportConfig>;tsconfig.base.json (1)
Line range hint
4-16: Configuration is well-suited for Storybook + Vite setup.The TypeScript configuration is properly set up for Storybook with Vite:
moduleResolution: "Bundler"is optimal for Vite- DOM types are included for UI components
- Modern JavaScript features are supported via ES2015 target
- Synthetic imports are enabled which is beneficial for Storybook
Consider adding
"jsx": "react-jsx"to the compiler options if you plan to use JSX in your Storybook stories without explicit React imports.Also applies to: 18-29
libs/ui/src/shadcn/button.tsx (2)
7-35: Consider enhancing hover feedback for disabled state.While the button variants are well-defined and include proper accessibility features like focus states, the disabled state could benefit from a hover cursor indication.
Consider adding this CSS class to improve user feedback:
- 'inline-flex items-center justify-center whitespace-nowrap rounded-md text-sm font-medium transition-colors focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-ring disabled:pointer-events-none disabled:opacity-50', + 'inline-flex items-center justify-center whitespace-nowrap rounded-md text-sm font-medium transition-colors focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-ring disabled:pointer-events-none disabled:opacity-50 disabled:cursor-not-allowed',
1-57: Consider adding documentation and tests.While the implementation is solid, consider:
- Adding JSDoc comments to document the component's usage and props
- Creating unit tests for different variants and states
- Adding accessibility tests (ARIA attributes, keyboard navigation)
Would you like me to help generate:
- Component documentation template?
- Unit test suite using your preferred testing framework?
- Accessibility test cases?
libs/ui/src/shadcn/button.stories.tsx (2)
11-45: Consider enhancing the meta configuration with additional documentation.While the configuration is well-structured, consider adding:
- A description field to provide component overview
- Parameter docs with examples of usage
- Control descriptions for each variant and size option
Example enhancement:
const meta = { title: 'Shadcn-ui/Button', component: Button, tags: ['autodocs'], parameters: { layout: 'centered', + docs: { + description: { + component: 'A versatile button component supporting multiple variants and sizes.', + }, + }, }, args: { children: 'Button', variant: 'default', onClick: fn(), }, argTypes: { variant: { options: ['default', 'secondary', 'destructive', 'outline', 'ghost', 'link'], control: { type: 'select' }, + description: 'The visual style of the button', }, size: { options: ['default', 'sm', 'lg', 'icon'], control: { type: 'select' }, + description: 'The size of the button', }, }, }
50-55: Consider adding descriptive names and documentation for each story.While the basic variant stories are well-implemented, they could be more self-documenting.
Example enhancement:
-export const Primary: Story = {} +export const Primary: Story = { + name: 'Primary Button', + parameters: { + docs: { + description: 'The primary button style, used for main actions.', + }, + }, +}libs/ui/.storybook/main.ts (2)
1-5: LGTM! Consider adding more specific documentation.The basic configuration is well-structured. The story pattern will correctly pick up MDX and story files.
Consider adding a comment explaining the project's specific structure and where stories should be placed within the
srcdirectory for better developer guidance.
42-85: Add Chromatic configuration for visual testing.While you've added the Chromatic addon, it would be beneficial to add explicit configuration for your visual testing workflow.
Consider adding Chromatic configuration:
// Add to the config object chromatic: { // Delay capture of DOM updates (ms) delay: 300, // Viewport sizes to capture viewports: [375, 768, 1024], // Disable snapshot diffing for certain stories diffIncludeElementAttributes: ['data-testid'], }libs/ui/.storybook/themes-storybook-ui.ts (1)
3-80: Clean up dark theme configuration and improve documentation.The dark theme configuration has several areas that could be improved:
- There are duplicate property definitions (commented vs active)
- Inconsistent color format usage (hex, hsl, rgba)
- Lack of documentation about color choices and design system
Consider applying these improvements:
export const darkUIStorybook = { + // Base theme configuration base: 'dark', + // Primary colors from design system colorPrimary: 'hsl(351, 100%, 64%)', // #FF4785 in HSL colorSecondary: 'hsl(204, 98%, 50%)', // #029CFD in HSL - // --primary: 210 40% 98%; - // --primary-foreground: 222.2 47.4% 11.2%; - // colorPrimary: 'hsl(210 40% 98%)', // ... remove other commented propertiespackage.json (2)
73-74: Consider documenting Radix UI adoption.The addition of Radix UI components suggests a shift towards using this UI library. Consider:
- Documenting this decision in the project's documentation
- Creating example stories showcasing these components
110-110: Document utility libraries and theming setup.You've added several utility libraries for styling and theming:
- class-variance-authority & clsx for class management
- tailwind-merge for Tailwind conflicts
- next-themes & storybook-dark-mode for theming
Consider:
- Creating a styling guide documenting these utilities
- Adding example stories demonstrating dark mode
Also applies to: 113-114, 136-136, 144-148
libs/ui/.storybook/globals.css (3)
1-4: LGTM! Consider adding a brief comment about the Tailwind configuration.The Tailwind directives are correctly configured. Consider adding a brief comment explaining the purpose of the relative path configuration for better maintainability.
+/* Configure Tailwind CSS with custom theme settings from ../tailwind.config.js */ @config '../tailwind.config.js'; @tailwind base; @tailwind components; @tailwind utilities;
6-30: Consider restructuring the color documentation.While the resource links are helpful, the commented-out color values could be better organized into a more readable format.
/*Easily customize theme: https://zippystarter.com/tools/shadcn-ui-theme-generator*/ /*Explore the Shadcn/ui ecosystem: https://github.com/birobirobiro/awesome-shadcn-ui*/ +/* Color Palette Reference + * Primary Colors: + * primary: #00A3E0 + * primary-content: #000a12 + * secondary: #00629b + * secondary-content: #d0dfec + * + * Accent Colors: + * accent: #FFD100 + * accent-content: #161000 + * neutral: #00843d + * neutral-content: #d3e6d7 + * + * Base Colors: + * base-100: #000000 (previously #002855) + * base-200: #002149 + * base-300: #001b3d + * base-content: #c8d1dc + * + * Status Colors: + * info: #009CA6 + * info-content: #00090a + * success: #78be21 + * success-content: #050d00 + * warning: #ffa400 + * warning-content: #160a00 + * error: #BA0C2F + * error-content: #f8d4d3 + */ -/*'primary': '#00A3E0',*/ -/*'primary-content': '#000a12',*/ -/* ... (remove existing commented colors) ... */
209-222: Consider adding documentation for custom utilities.The custom utilities are well-implemented but would benefit from documentation explaining their intended use cases.
+/* Custom backdrop blur utility for subtle transparency effects */ .backdrop-blur-xs { @apply backdrop-blur-[2px]; } +/* Subtle floating animation for interactive elements + * Usage: Add 'animate-bobbing' class to elements + * Duration: Infinite + * Range: 4px total movement (-2px to +2px) + */ @keyframes bobbing { 0%, 100% { transform: translateY(-2px); } 50% { transform: translateY(2px); } }apps/website/app/nx-welcome.tsx (2)
438-442: Consider enhancing the button implementation.The current implementation uses placeholder text and lacks meaningful interaction. Consider:
- Using more descriptive button text
- Adding proper aria-label for accessibility
- Including an onClick handler for interaction
- Adding proper role if this button has a specific function
Here's a suggested improvement:
- This is a - {' '} - <Button> Shadcn button </Button> - {' '} - imported from the ui lib. + <Button + aria-label="Get Started" + onClick={() => {/* Add your action here */}} + > + Get Started + </Button>
Line range hint
1-1000: Consider restructuring for better Storybook integration.Since we're initializing Storybook, consider these architectural improvements:
- Break down this large welcome component into smaller, reusable components (e.g., WelcomeHeader, FeatureCard, CommandList)
- Create separate stories for each component
- Add proper component documentation using JSDoc comments
Would you like help creating the component structure and corresponding Storybook stories?
libs/ui/.storybook/preview.tsx (1)
46-60: Remove unnecessary commented-out code for cleaner configurationThe
globalTypesconfiguration is commented out. If it is no longer needed, consider removing this commented code to keep the file clean and maintainable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
libs/ui/.storybook/cuhacking-logo.pngis excluded by!**/*.pngpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (27)
.gitignore(1 hunks)apps/website/app/nx-welcome.tsx(2 hunks)apps/website/app/tailwind.css(1 hunks)libs/ui/.babelrc(1 hunks)libs/ui/.storybook/custom-viewports.ts(1 hunks)libs/ui/.storybook/globals.css(1 hunks)libs/ui/.storybook/main.ts(1 hunks)libs/ui/.storybook/preview.tsx(1 hunks)libs/ui/.storybook/themes-storybook-ui.ts(1 hunks)libs/ui/README.md(1 hunks)libs/ui/eslint.config.js(1 hunks)libs/ui/package.json(1 hunks)libs/ui/postcss.config.js(1 hunks)libs/ui/project.json(1 hunks)libs/ui/src/index.ts(1 hunks)libs/ui/src/lib/ui.module.css(1 hunks)libs/ui/src/lib/ui.tsx(1 hunks)libs/ui/src/shadcn/button.stories.tsx(1 hunks)libs/ui/src/shadcn/button.tsx(1 hunks)libs/ui/src/shadcn/cn.ts(1 hunks)libs/ui/tailwind.config.js(1 hunks)libs/ui/tsconfig.json(1 hunks)libs/ui/tsconfig.lib.json(1 hunks)libs/ui/tsconfig.storybook.json(1 hunks)nx.json(3 hunks)package.json(4 hunks)tsconfig.base.json(1 hunks)
✅ Files skipped from review due to trivial changes (12)
- .gitignore
- apps/website/app/tailwind.css
- libs/ui/.babelrc
- libs/ui/README.md
- libs/ui/package.json
- libs/ui/postcss.config.js
- libs/ui/project.json
- libs/ui/src/index.ts
- libs/ui/src/lib/ui.module.css
- libs/ui/tsconfig.json
- libs/ui/tsconfig.lib.json
- libs/ui/tsconfig.storybook.json
🔇 Additional comments (23)
libs/ui/src/shadcn/cn.ts (1)
1-2: Verify consistent import usage across the codebase.
Let's ensure these utility functions are imported consistently across the project.
✅ Verification successful
Imports are properly encapsulated and consistently used
The verification shows that:
- The
clsxandtwMergeutilities are only imported incn.tswhere they are encapsulated in thecnutility function - Other components (like
button.tsx) correctly use thecnutility function instead of directly importingclsxortwMerge - No other similar utility functions exist in the codebase that could cause inconsistency
- No direct usage of
clsxortwMergewas found outside ofcn.ts
This demonstrates a clean and consistent pattern of class name management across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent import patterns of clsx and tailwind-merge
echo "Checking for different import patterns of clsx and tailwind-merge..."
rg --type ts "import.*(?:clsx|tailwind-merge)" -A 1
echo "Checking for direct usage without imports..."
rg --type ts "(?:clsx|twMerge)\(" --files-with-matches | grep -v "cn.ts"
Length of output: 534
Script:
#!/bin/bash
# Let's search for any usage of the cn utility function across the codebase
# This will help us verify if the imports are properly encapsulated
echo "Checking for usage of cn utility function..."
rg --type ts "cn\(" -A 1
echo "Checking for any direct usage of clsx or twMerge in other files..."
rg --type ts "(?:clsx|twMerge)\(" -A 1
echo "Looking for any other similar utility functions..."
rg --type ts "function.*(?:classNames|mergeClasses|mergeStyles)" -A 1
Length of output: 852
libs/ui/.storybook/custom-viewports.ts (1)
1-4: LGTM! Good practice extending default viewports.
The implementation correctly preserves Storybook's default viewports while adding custom ones.
tsconfig.base.json (1)
17-17: LGTM! Path mapping follows consistent pattern.
The new path mapping for @cuhacking/ui follows the established pattern and structure of other module mappings in the project.
Let's verify that the mapped file exists:
✅ Verification successful
Path mapping is correctly configured and verified
The path mapping for @cuhacking/ui in tsconfig.base.json correctly points to the existing file at libs/ui/src/index.ts. The directory structure and file existence have been verified.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the existence of the UI library entry point
# Expected: The file should exist as it's referenced in the path mapping
# Check if the UI library entry point exists
if ! fd --absolute-path "libs/ui/src/index.ts"; then
echo "Warning: The mapped file 'libs/ui/src/index.ts' does not exist"
exit 1
fi
Length of output: 41
Script:
#!/bin/bash
# Let's try a different approach to verify the existence of the UI library
# First, check if the directory structure exists
echo "Checking directory structure:"
fd --type directory "ui" libs/
# Then check for any TypeScript files in the UI library
echo -e "\nChecking for TypeScript files:"
fd -e ts . libs/ui/
# Finally, check if there's any index file (even if not exactly at src/index.ts)
echo -e "\nChecking specifically for index files:"
fd "index.ts" libs/ui/
Length of output: 390
libs/ui/eslint.config.js (1)
13-41: Document configuration requirements instead of keeping commented code.
The large block of commented configurations suggests future requirements but creates maintenance overhead. Consider documenting these configurations in a separate markdown file instead.
Let's verify if any of these configurations are actually needed:
libs/ui/src/shadcn/button.tsx (4)
1-5: LGTM! Clean and well-organized imports.
The imports are properly structured and include all necessary dependencies.
37-41: LGTM! Well-structured props interface.
The ButtonProps interface properly extends HTML button attributes while adding type-safe variant support.
43-55: LGTM with one existing issue.
The Button component implementation is solid, using proper React patterns like forwardRef and polymorphic rendering.
Note: The previously identified issue with className merging still needs to be addressed.
57-57: LGTM! Clean exports.
The exports provide appropriate access to both the component and its styling utilities.
libs/ui/src/shadcn/button.stories.tsx (2)
1-9: LGTM! Well-organized imports with proper dependencies.
The imports are well-structured, using appropriate icons from Radix UI (which are accessibility-friendly) and necessary Storybook utilities.
48-48: LGTM! Proper type definition using StoryObj.
The Story type is correctly defined using TypeScript's type inference from the meta object.
nx.json (2)
13-16: LGTM: Production exclusions properly configured for Storybook.
The added exclusions correctly prevent Storybook-related files from being included in production builds, following Nx best practices.
66-74: LGTM: Storybook plugin properly configured.
The plugin configuration is correct with standard target names. However, as noted in the previous review, please ensure you have the required Storybook configuration files:
.storybook/main.jsor.storybook/main.ts.storybook/preview.jsor.storybook/preview.ts
libs/ui/tailwind.config.js (1)
1-3: LGTM! Imports are well-structured.
The imports demonstrate good practices:
- Using the
node:prefix for Node.js built-ins - Leveraging Nx's Tailwind utilities for monorepo support
- Importing only the needed parts from Tailwind's defaults
libs/ui/.storybook/main.ts (1)
7-21: Verify potential conflicts between theme-related addons.
You have multiple addons that can affect theming:
storybook-dark-mode@storybook/addon-themes@storybook/addon-backgrounds
While the comments explain their individual purposes, these addons might interact with each other in unexpected ways.
libs/ui/.storybook/themes-storybook-ui.ts (2)
82-116: Previous review comment about incomplete light theme is still applicable.
The light theme implementation remains incomplete, with only font properties defined and numerous commented-out CSS custom properties.
118-123: 🛠️ Refactor suggestion
Consider hosting brand image locally.
The brand image is currently referenced from an external URL. This could cause issues if:
- The external URL becomes unavailable
- The network connection is unstable
- The image is updated or removed from the external source
Let's verify if the image is available in the local repository:
Consider:
- Hosting the image locally in your assets directory
- Adding a fallback image
- Using a relative path instead of an absolute URL
✅ Verification successful
Update brand image path to use local asset
The Logo.svg file is already available locally at apps/landing-page/public/Logo.svg. Consider updating the brandImage path to use this local asset:
brandImage: '/Logo.svg',This change will:
- Ensure the image is always available
- Reduce external dependencies
- Improve load time reliability
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for Logo.svg in the repository
fd -t f "Logo.svg"
Length of output: 52
package.json (4)
51-51: LGTM: Core Storybook setup with Nx integration.
The core Storybook dependencies align well with the PR objective of initializing Storybook using the Nx command.
Also applies to: 66-66, 68-68
77-92: Review Storybook addon selection.
You've added a comprehensive set of Storybook addons. While these are all useful, consider:
- Documenting the purpose of each addon in your Storybook configuration
- Ensuring all addons are actually being used to avoid unnecessary bloat
Let's check for addon usage:
#!/bin/bash
# Check for addon usage in Storybook configuration
echo "Searching for Storybook addon usage:"
rg -l "addon-|@storybook/addon" --type ts --type tsx139-139: Review Prettier major version update impact.
The update from Prettier 2.x to 3.x (^3.3.3) is a major version change that could affect formatting across the project.
Let's check for potential formatting issues:
#!/bin/bash
# Check for Prettier configuration and formatted files
echo "Searching for Prettier config:"
fd -e prettierrc -e prettierrc.js -e prettierrc.json
echo "Checking for prettier configuration in package.json:"
jq '.prettier' package.json
echo "Searching for formatted files:"
fd -e ts -e tsx -e js -e jsx49-50: Verify if Babel dependencies are necessary.
Since Hasith mentioned in the PR comments that Babel might not be needed and you confirmed not foreseeing its use, consider removing these Babel dependencies:
- "@babel/core": "^7.14.5",
- "@babel/preset-react": "^7.14.5",Let's check if any files actually use Babel:
✅ Verification successful
Let me generate additional scripts to check for Babel usage in the codebase.
This will help us determine if Babel is being used in npm scripts, Storybook configuration, or imported anywhere in the codebase.
This corrected script will:
- Search for .storybook directories with hidden files included
- Search for Babel references without file type restrictions
- Look for any Babel configuration files including hidden ones
Let me check for Storybook configuration since that's the main reason for these Babel dependencies.
Let me fix the file type issue and check again for Storybook files.
Keep Babel dependencies as they are required for Storybook
The Babel dependencies are necessary for the Storybook setup in libs/ui/.storybook/. Specifically:
- The project uses
@storybook/nextjsframework which requires Babel for transpilation - Multiple Storybook addons are configured that rely on Babel for processing
- Story files (like
libs/ui/src/shadcn/button.stories.tsx) need Babel for proper compilation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for Babel configuration files and usage
echo "Searching for Babel config files:"
fd -e babelrc -e babelrc.js -e babelrc.json
echo "Searching for babel.config files:"
fd -g "babel.config.*"
echo "Checking for babel configuration in package.json:"
jq '.babel' package.json
Length of output: 5744
Script:
#!/bin/bash
# Check for Babel usage in configuration files and package.json scripts
echo "Checking package.json scripts for babel usage:"
jq '.scripts | to_entries[] | select(.value | contains("babel"))' package.json
echo "\nChecking for Storybook config files that might need Babel:"
fd -g ".storybook"
echo "\nChecking for any imports of Babel in the codebase:"
rg "@babel" --type ts --type js --type tsx --type jsx
Length of output: 738
Script:
#!/bin/bash
# Fix the previous command errors and check for Babel usage
echo "Checking for Storybook config files:"
fd -H ".storybook"
echo "\nChecking for any Babel imports or usage:"
rg "@babel"
echo "\nChecking for .babelrc or babel.config files:"
fd -H -e babelrc -e "babel.config.js" -e "babel.config.json"
Length of output: 188480
Script:
#!/bin/bash
# Check for Storybook configuration files that might require Babel
echo "Checking for Storybook config files:"
fd -H ".storybook"
echo "\nChecking for Storybook-related files:"
rg -l "storybook|stories" --type ts --type tsx --type js --type jsx
Length of output: 382
Script:
#!/bin/bash
# Check for Storybook-related files and configurations
echo "Checking for Storybook-related files:"
rg -l "storybook|stories"
echo "\nChecking Storybook main config:"
cat libs/ui/.storybook/main.* 2>/dev/null || echo "No main config found"
echo "\nChecking Storybook preview config:"
cat libs/ui/.storybook/preview.* 2>/dev/null || echo "No preview config found"
echo "\nChecking for story files:"
fd -e stories.tsx -e stories.ts -e stories.jsx -e stories.js
Length of output: 9387
libs/ui/.storybook/globals.css (2)
199-207: LGTM! Base styles are appropriately configured.
The base styles provide essential defaults while keeping the configuration minimal and maintainable.
1-222: Verify Storybook integration with the theme configuration.
The theme configuration looks comprehensive, but let's verify its integration with Storybook:
✅ Verification successful
Theme configuration is properly integrated and functional
The verification shows a well-structured theme setup:
- The globals.css correctly defines CSS variables that match the Tailwind configuration
- Storybook preview.tsx properly imports globals.css and sets up theme providers
- Tailwind config correctly maps the CSS variables to the color system
- Theme integration is demonstrated in the button.stories.tsx with various variants
- Dark/light mode switching is properly configured with next-themes
The implementation provides a solid foundation for UI component development with proper theme support.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify Storybook configuration and theme integration
# Check if the Storybook preview configuration references this CSS file
echo "Checking Storybook preview configuration..."
rg -l "globals.css" "libs/ui/.storybook/preview"
# Verify Tailwind configuration
echo "Checking Tailwind configuration..."
cat ../tailwind.config.js
# Check for any existing stories using the theme
echo "Checking for stories using the theme..."
fd -e stories.tsx -e stories.ts . libs/ui/
Length of output: 543
Script:
#!/bin/bash
# Let's verify the actual Storybook configuration and theme setup
# Check the preview file with correct extension
echo "Checking Storybook preview files..."
fd "preview" "libs/ui/.storybook/"
# Check the content of any preview file found
echo "\nChecking preview content..."
fd "preview" "libs/ui/.storybook/" -x cat {}
# Check for tailwind config in the UI library
echo "\nChecking for tailwind config in UI library..."
fd "tailwind.config" "libs/ui/"
# Let's also check the button story to see theme usage
echo "\nChecking button story implementation..."
cat "libs/ui/src/shadcn/button.stories.tsx"
Length of output: 7562
Script:
#!/bin/bash
# Let's check the tailwind config content and verify its integration
# Check tailwind config content
echo "Checking tailwind config content..."
cat "libs/ui/tailwind.config.js"
# Verify if there are any other theme-related files
echo "\nChecking for theme-related files..."
fd -e ts -e tsx -e js "theme" "libs/ui/"
# Check the storybook themes configuration
echo "\nChecking storybook themes..."
cat "libs/ui/.storybook/themes-storybook-ui.ts"
Length of output: 6463
apps/website/app/nx-welcome.tsx (1)
1-1: LGTM! Clean import of Button component.
The import from '@cuhacking/ui' follows good practices by utilizing a shared UI library.
libs/ui/src/lib/ui.tsx
Outdated
| @@ -0,0 +1,11 @@ | |||
| import styles from './ui.module.css' | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add type safety for CSS modules.
For better type safety and IDE support, consider adding type definitions for your CSS modules.
Add a declaration file ui.module.css.d.ts:
declare const styles: {
readonly [key: string]: string;
readonly container: string;
};
export default styles;
libs/ui/src/lib/ui.tsx
Outdated
| ) | ||
| } | ||
|
|
||
| export default Ui |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider named exports for better tree-shaking.
When building a component library, named exports are generally preferred over default exports as they work better with tree-shaking and make imports more explicit.
Remove the default export and use named exports consistently:
-export default UiCommittable suggestion was skipped due to low confidence.
| '720p': { | ||
| name: '720p', | ||
| styles: { | ||
| width: '1280px', | ||
| height: '720px', | ||
| }, | ||
| }, | ||
| '1080p': { | ||
| name: '1080p', | ||
| styles: { | ||
| width: '1920px', | ||
| height: '1080px', | ||
| }, | ||
| }, | ||
| '2k': { | ||
| name: '2K', | ||
| styles: { | ||
| width: '2560px', | ||
| height: '1440px', | ||
| }, | ||
| }, | ||
| '4k': { | ||
| name: '4K', | ||
| styles: { | ||
| width: '3840px', | ||
| height: '2160px', | ||
| }, | ||
| }, | ||
| '21/9': { | ||
| name: '21/9', | ||
| styles: { | ||
| width: '2560px', | ||
| height: '1080px', | ||
| }, | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider improving viewport naming and key conventions.
A few suggestions to enhance maintainability and clarity:
- The key
'21/9'uses a slash which could be problematic. Consider using an underscore or dash instead. - The viewport names could be more descriptive by including both the resolution and common name.
Here's a suggested improvement:
- '21/9': {
- name: '21/9',
+ 'ultrawide_21_9': {
+ name: '21:9 Ultrawide (2560x1080)',
styles: {
width: '2560px',
height: '1080px',
},
},Similar descriptive names could be applied to other viewports:
- name: '720p',
+ name: 'HD (1280x720)',Committable suggestion was skipped due to low confidence.
libs/ui/eslint.config.js
Outdated
| export default (async () => { | ||
| const baseConfig = await baseConfigPromise | ||
|
|
||
| return [ | ||
| ...baseConfig, | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for async configuration loading.
The async configuration loading could fail silently if the base config import fails.
Consider adding error handling:
export default (async () => {
- const baseConfig = await baseConfigPromise
+ try {
+ const baseConfig = await baseConfigPromise
- return [
- ...baseConfig,
+ return [
+ ...baseConfig,
+ ]
+ } catch (error) {
+ console.error('Failed to load ESLint base configuration:', error)
+ throw error
+ }
- ]
})()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export default (async () => { | |
| const baseConfig = await baseConfigPromise | |
| return [ | |
| ...baseConfig, | |
| export default (async () => { | |
| try { | |
| const baseConfig = await baseConfigPromise | |
| return [ | |
| ...baseConfig, | |
| ] | |
| } catch (error) { | |
| console.error('Failed to load ESLint base configuration:', error) | |
| throw error | |
| } | |
| })() |
libs/ui/eslint.config.js
Outdated
| // TODO: merge with antfu eslint config | ||
| // const { FlatCompat } = require('@eslint/eslintrc'); | ||
| // const baseConfigPromise = require('../../eslint.config.js') | ||
| import baseConfigPromise from '../../eslint.config.js' | ||
| // const js = require('@eslint/js'); | ||
|
|
There was a problem hiding this comment.
Improve configuration management and module consistency.
Several improvements can be made to this section:
- The TODO comment lacks context and tracking
- There's a mix of ES modules (import) and CommonJS (require) syntax
- Commented-out code creates unnecessary noise
Apply this diff to clean up the imports:
-// TODO: merge with antfu eslint config
-// const { FlatCompat } = require('@eslint/eslintrc');
-// const baseConfigPromise = require('../../eslint.config.js')
+// TODO(#98): Merge with antfu eslint config and document the migration strategy
+
import baseConfigPromise from '../../eslint.config.js'
-// const js = require('@eslint/js');Would you like me to create a GitHub issue to track the antfu config integration task?
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // TODO: merge with antfu eslint config | |
| // const { FlatCompat } = require('@eslint/eslintrc'); | |
| // const baseConfigPromise = require('../../eslint.config.js') | |
| import baseConfigPromise from '../../eslint.config.js' | |
| // const js = require('@eslint/js'); | |
| // TODO(#98): Merge with antfu eslint config and document the migration strategy | |
| import baseConfigPromise from '../../eslint.config.js' |
libs/ui/.storybook/main.ts
Outdated
| // --------------- ACTION BAR --------------- | ||
| // https://storybook.js.org/docs/essentials/controls | ||
| '@storybook/addon-controls', | ||
| // Accessibility tab added by '@storybook/addon-a11y' | ||
| { | ||
| name: '@storybook/addon-designs', // Actually has amazing docs for once: https://storybookjs.github.io/addon-designs/?path=/docs/docs-quick-start--docs | ||
| options: { | ||
| // renderTarget: 'canvas' | 'tab' | ||
| }, | ||
| }, | ||
| // Person inside circle icon. Also 'Accessibility' tab to action bar | ||
| '@storybook/addon-a11y', | ||
|
|
||
| // https://storybook.js.org/docs/essentials/actions | ||
| '@storybook/addon-actions', | ||
| // https://storybook.js.org/docs/essentials/interactions | ||
| '@storybook/addon-interactions', // Must be listed after @storybook/addon-actions or @storybook/addon-essentials | ||
|
|
||
| '@storybook/addon-links', | ||
|
|
||
| // Chromatic Visual Test Server | ||
| '@chromatic-com/storybook', | ||
|
|
||
| // Code for component story | ||
| // { | ||
| // name: '@storybook/addon-storysource', | ||
| // options: { | ||
| // rule: { | ||
| // test: [/\.stories\.tsx?$/], | ||
| // include: [path.resolve(__dirname, '../src')], // You can specify directories | ||
| // }, | ||
| // loaderOptions: { | ||
| // parser: 'typescript', | ||
| // injectStoryParameters: false, | ||
| // prettierConfig: { printWidth: 80, singleQuote: false }, | ||
| // }, | ||
| // enforce: 'pre', | ||
| // }, | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider enabling the storysource addon for better development experience.
The commented out storysource addon configuration looks well-configured for TypeScript. Consider enabling it as it would provide valuable insights during development.
- // {
- // name: '@storybook/addon-storysource',
+ {
+ name: '@storybook/addon-storysource',
- // options: {
+ options: {
- // rule: {
+ rule: {
- // test: [/\.stories\.tsx?$/],
+ test: [/\.stories\.tsx?$/],
- // include: [path.resolve(__dirname, '../src')],
+ include: [path.resolve(__dirname, '../src')],
- // },
+ },
- // loaderOptions: {
+ loaderOptions: {
- // parser: 'typescript',
+ parser: 'typescript',
- // injectStoryParameters: false,
+ injectStoryParameters: false,
- // prettierConfig: { printWidth: 80, singleQuote: false },
+ prettierConfig: { printWidth: 80, singleQuote: false },
- // },
+ },
- // enforce: 'pre',
+ enforce: 'pre',
- // },
+ },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // --------------- ACTION BAR --------------- | |
| // https://storybook.js.org/docs/essentials/controls | |
| '@storybook/addon-controls', | |
| // Accessibility tab added by '@storybook/addon-a11y' | |
| { | |
| name: '@storybook/addon-designs', // Actually has amazing docs for once: https://storybookjs.github.io/addon-designs/?path=/docs/docs-quick-start--docs | |
| options: { | |
| // renderTarget: 'canvas' | 'tab' | |
| }, | |
| }, | |
| // Person inside circle icon. Also 'Accessibility' tab to action bar | |
| '@storybook/addon-a11y', | |
| // https://storybook.js.org/docs/essentials/actions | |
| '@storybook/addon-actions', | |
| // https://storybook.js.org/docs/essentials/interactions | |
| '@storybook/addon-interactions', // Must be listed after @storybook/addon-actions or @storybook/addon-essentials | |
| '@storybook/addon-links', | |
| // Chromatic Visual Test Server | |
| '@chromatic-com/storybook', | |
| // Code for component story | |
| // { | |
| // name: '@storybook/addon-storysource', | |
| // options: { | |
| // rule: { | |
| // test: [/\.stories\.tsx?$/], | |
| // include: [path.resolve(__dirname, '../src')], // You can specify directories | |
| // }, | |
| // loaderOptions: { | |
| // parser: 'typescript', | |
| // injectStoryParameters: false, | |
| // prettierConfig: { printWidth: 80, singleQuote: false }, | |
| // }, | |
| // enforce: 'pre', | |
| // }, | |
| // --------------- ACTION BAR --------------- | |
| // https://storybook.js.org/docs/essentials/controls | |
| '@storybook/addon-controls', | |
| // Accessibility tab added by '@storybook/addon-a11y' | |
| { | |
| name: '@storybook/addon-designs', // Actually has amazing docs for once: https://storybookjs.github.io/addon-designs/?path=/docs/docs-quick-start--docs | |
| options: { | |
| // renderTarget: 'canvas' | 'tab' | |
| }, | |
| }, | |
| // Person inside circle icon. Also 'Accessibility' tab to action bar | |
| '@storybook/addon-a11y', | |
| // https://storybook.js.org/docs/essentials/actions | |
| '@storybook/addon-actions', | |
| // https://storybook.js.org/docs/essentials/interactions | |
| '@storybook/addon-interactions', // Must be listed after @storybook/addon-actions or @storybook/addon-essentials | |
| '@storybook/addon-links', | |
| // Chromatic Visual Test Server | |
| '@chromatic-com/storybook', | |
| // Code for component story | |
| { | |
| name: '@storybook/addon-storysource', | |
| options: { | |
| rule: { | |
| test: [/\.stories\.tsx?$/], | |
| include: [path.resolve(__dirname, '../src')], // You can specify directories | |
| }, | |
| loaderOptions: { | |
| parser: 'typescript', | |
| injectStoryParameters: false, | |
| prettierConfig: { printWidth: 80, singleQuote: false }, | |
| }, | |
| enforce: 'pre', | |
| }, |
libs/ui/.storybook/globals.css
Outdated
| .dark { | ||
| --background: 218 100% 1%; | ||
| /* hsl(218, 100%, 1%) */ | ||
| --foreground: 196 5% 90%; | ||
| /* hsl(196, 5%, 90%) */ | ||
| --card: 196 50% 0%; | ||
| /* hsl(196, 50%, 0%) */ | ||
| --card-foreground: 196 5% 90%; | ||
| /* hsl(196, 5%, 90%) */ | ||
| --popover: 196 50% 5%; | ||
| /* hsl(196, 50%, 5%) */ | ||
| --popover-foreground: 196 5% 90%; | ||
| /* hsl(196, 5%, 90%) */ | ||
| --primary: 196 100% 43.9%; | ||
| /* hsl(196, 100%, 43.9%) */ | ||
| --primary-foreground: 0 0% 100%; | ||
| /* hsl(0, 0%, 100%) */ | ||
|
|
||
| --auxiliary: 203, 100%, 37%; | ||
| /* hsl(203, 100%, 37%) */ | ||
| --auxiliary-foreground: 213, 13%, 91%; | ||
| /* hsl(213, 13%, 91%) */ | ||
| --tertiary: 52, 100%, 50%; | ||
| /* hsl(52, 100%, 50%) */ | ||
| --tertiary-foreground: 60, 4%, 9%; | ||
| /* hsl(60, 4%, 9%) */ | ||
| --neutral: 143, 100%, 26%; | ||
| /* hsl(143, 100%, 26%) */ | ||
| --neutral-foreground: 141, 13%, 85%; | ||
| /* hsl(141, 13%, 85%) */ | ||
| /*// "base-100": "#002855",*/ | ||
| /*'base-100': '#000000',*/ | ||
| /*'base-200': '#002149',*/ | ||
| /*'base-300': '#001b3d',*/ | ||
| /*'base-content': '#c8d1dc',*/ | ||
| --info: 183, 100%, 33%; | ||
| /* hsl(183, 100%, 33%) */ | ||
| --info-foreground: 180, 100%, 3%; | ||
| /* hsl(180, 100%, 3%) */ | ||
| --success: 83, 100%, 25%; | ||
| /* hsl(83, 100%, 25%) */ | ||
| --success-foreground: 120, 100%, 2%; | ||
| /* hsl(120, 100%, 2%) */ | ||
| --warning: 38, 100%, 50%; | ||
| /* hsl(38, 100%, 50%) */ | ||
| --warning-foreground: 30, 100%, 5%; | ||
| /* hsl(30, 100%, 5%) */ | ||
|
|
||
| --secondary: 196 30% 10%; | ||
| /* hsl(196, 30%, 10%) */ | ||
| --secondary-foreground: 0 0% 100%; | ||
| /* hsl(0, 0%, 100%) */ | ||
| --muted: 158 30% 15%; | ||
| /* hsl(158, 30%, 15%) */ | ||
| --muted-foreground: 196 5% 60%; | ||
| /* hsl(196, 5%, 60%) */ | ||
| --accent: 158 30% 15%; | ||
| /* hsl(158, 30%, 15%) */ | ||
| --accent-foreground: 196 5% 90%; | ||
| /* hsl(196, 5%, 90%) */ | ||
| --destructive: 0 100% 30%; | ||
| /* hsl(0, 100%, 30%) */ | ||
| --destructive-foreground: 196 5% 90%; | ||
| /* hsl(196, 5%, 90%) */ | ||
| --border: 196 30% 18%; | ||
| /* hsl(196, 30%, 18%) */ | ||
| --input: 196 30% 18%; | ||
| /* hsl(196, 30%, 18%) */ | ||
| --ring: 196 100% 43.9%; | ||
| /* hsl(196, 100%, 43.9%) */ | ||
| --radius: 0.75rem; | ||
| --chart-1: 220 70% 50%; | ||
| /* hsl(220, 70%, 50%) */ | ||
| --chart-2: 160 60% 45%; | ||
| /* hsl(160, 60%, 45%) */ | ||
| --chart-3: 30 80% 55%; | ||
| /* hsl(30, 80%, 55%) */ | ||
| --chart-4: 280 65% 60%; | ||
| /* hsl(280, 65%, 60%) */ | ||
| --chart-5: 340 75% 55%; | ||
| /* hsl(340, 75%, 55%) */ | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider reducing duplication in theme variables.
Several variables have identical values in both light and dark themes. Consider using CSS custom properties more efficiently.
@layer base {
+ /* Common theme variables */
+ :root {
+ /* Variables that don't change between themes */
+ --primary: 196 100% 43.9%;
+ --primary-foreground: 0 0% 100%;
+ --radius: 0.75rem;
+ /* ... other common variables ... */
+ }
+
:root {
- --primary: 196 100% 43.9%;
- --primary-foreground: 0 0% 100%;
- --radius: 0.75rem;
/* ... theme-specific variables ... */
}
.dark {
- --primary: 196 100% 43.9%;
- --primary-foreground: 0 0% 100%;
- --radius: 0.75rem;
/* ... theme-specific variables ... */
}
}Committable suggestion was skipped due to low confidence.
libs/ui/.storybook/globals.css
Outdated
| @layer base { | ||
| :root { | ||
| --background: 196 100% 95%; | ||
| /* hsl(196, 100%, 95%) */ | ||
| --foreground: 196 5% 0%; | ||
| /* hsl(196, 5%, 0%) */ | ||
| --card: 196 50% 90%; | ||
| /* hsl(196, 50%, 90%) */ | ||
| --card-foreground: 196 5% 10%; | ||
| /* hsl(196, 5%, 10%) */ | ||
| --popover: 196 100% 95%; | ||
| /* hsl(196, 100%, 95%) */ | ||
| --popover-foreground: 196 100% 0%; | ||
| /* hsl(196, 100%, 0%) */ | ||
|
|
||
| --primary: 196 100% 43.9%; | ||
| /* hsl(196, 100%, 43.9%) */ | ||
| --primary-foreground: 0 0% 100%; | ||
| /* hsl(0, 0%, 100%) */ | ||
| --auxiliary: 203, 100%, 37%; | ||
| /* hsl(203, 100%, 37%) */ | ||
| --auxiliary-foreground: 213, 13%, 91%; | ||
| /* hsl(213, 13%, 91%) */ | ||
| --tertiary: 52, 100%, 50%; | ||
| /* hsl(52, 100%, 50%) */ | ||
| --tertiary-foreground: 60, 4%, 9%; | ||
| /* hsl(60, 4%, 9%) */ | ||
| --neutral: 143, 100%, 26%; | ||
| /* hsl(143, 100%, 26%) */ | ||
| --neutral-foreground: 141, 13%, 85%; | ||
| /* hsl(141, 13%, 85%) */ | ||
| /*// "base-100": "#002855",*/ | ||
| /*'base-100': '#000000',*/ | ||
| /*'base-200': '#002149',*/ | ||
| /*'base-300': '#001b3d',*/ | ||
| /*'base-content': '#c8d1dc',*/ | ||
| --info: 183, 100%, 33%; | ||
| /* hsl(183, 100%, 33%) */ | ||
| --info-foreground: 180, 100%, 3%; | ||
| /* hsl(180, 100%, 3%) */ | ||
| --success: 83, 100%, 25%; | ||
| /* hsl(83, 100%, 25%) */ | ||
| --success-foreground: 120, 100%, 2%; | ||
| /* hsl(120, 100%, 2%) */ | ||
| --warning: 38, 100%, 50%; | ||
| /* hsl(38, 100%, 50%) */ | ||
| --warning-foreground: 30, 100%, 5%; | ||
| /* hsl(30, 100%, 5%) */ | ||
|
|
||
| --secondary: 196 30% 70%; | ||
| /* hsl(196, 30%, 70%) */ | ||
| --secondary-foreground: 0 0% 0%; | ||
| /* hsl(0, 0%, 0%) */ | ||
| --muted: 158 30% 85%; | ||
| /* hsl(158, 30%, 85%) */ | ||
| --muted-foreground: 196 5% 35%; | ||
| /* hsl(196, 5%, 35%) */ | ||
| --accent: 158 30% 80%; | ||
| /* hsl(158, 30%, 80%) */ | ||
| --accent-foreground: 196 5% 10%; | ||
| /* hsl(196, 5%, 10%) */ | ||
| --destructive: 0 100% 30%; | ||
| /* hsl(0, 100%, 30%) */ | ||
| --destructive-foreground: 196 5% 90%; | ||
| /* hsl(196, 5%, 90%) */ | ||
| --border: 196 30% 50%; | ||
| /* hsl(196, 30%, 50%) */ | ||
| --input: 196 30% 18%; | ||
| /* hsl(196, 30%, 18%) */ | ||
| --ring: 196 100% 43.9%; | ||
| /* hsl(196, 100%, 43.9%) */ | ||
| --radius: 0.75rem; | ||
| --chart-1: 12 76% 61%; | ||
| /* hsl(12, 76%, 61%) */ | ||
| --chart-2: 173 58% 39%; | ||
| /* hsl(173, 58%, 39%) */ | ||
| --chart-3: 197 37% 24%; | ||
| /* hsl(197, 37%, 24%) */ | ||
| --chart-4: 43 74% 66%; | ||
| /* hsl(43, 74%, 66%) */ | ||
| --chart-5: 27 87% 67%; | ||
| /* hsl(27, 87%, 67%) */ | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider improving variable organization and documentation.
The CSS custom properties are well-structured, but there are a few suggestions for improvement:
- The order of color variables could be more consistent (e.g., primary/secondary/tertiary grouped together)
- Some variables have HSL comments while others don't
- The chart colors could benefit from more descriptive names
Consider reorganizing the variables into logical groups and ensuring consistent documentation:
@layer base {
:root {
+ /* Base Theme Colors */
--background: 196 100% 95%;
--foreground: 196 5% 0%;
+
+ /* Primary Color Scheme */
--primary: 196 100% 43.9%;
--primary-foreground: 0 0% 100%;
--secondary: 196 30% 70%;
--secondary-foreground: 0 0% 0%;
--tertiary: 52 100% 50%;
--tertiary-foreground: 60 4% 9%;
+
+ /* Component Colors */
--card: 196 50% 90%;
--card-foreground: 196 5% 10%;
/* ... rest of the variables ... */
+
+ /* Data Visualization Colors */
+ /* Used for primary data series in charts */
--chart-1: 12 76% 61%;
+ /* Used for secondary data series */
--chart-2: 173 58% 39%;
/* ... rest of chart colors ... */
}Committable suggestion was skipped due to low confidence.
libs/ui/.storybook/preview.tsx
Outdated
| withThemeByClassName({ | ||
| themes: { | ||
| light: 'light', | ||
| dark: 'dark', | ||
| }, | ||
| defaultTheme: 'dark', | ||
| }), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consolidate theming approach to simplify configuration
You're using both storybook-dark-mode and @storybook/addon-themes to manage theming and dark mode functionality. This may lead to redundant configurations and potential conflicts. Consider using a single theming addon to streamline your setup.
Review whether you can achieve the desired theming behavior using only one of these addons. This could simplify your configuration and reduce potential issues arising from overlapping functionalities.
libs/ui/.storybook/preview.tsx
Outdated
| }, | ||
| // Override the default light theme | ||
| light: { | ||
| ...themes.normal, |
There was a problem hiding this comment.
Fix invalid theme reference: use themes.light instead of themes.normal
In the darkMode configuration for the light theme, you're spreading ...themes.normal, but themes.normal is not a valid theme provided by Storybook. To correctly apply the default light theme settings, you should use themes.light instead.
Apply this diff to fix the issue:
light: {
- ...themes.normal,
+ ...themes.light,
...lightUIStorybook,
...commonTheme,
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ...themes.normal, | |
| ...themes.light, |
MFarabi619
force-pushed
the
mfarabi/feat/96-add-storybook-with-tailwindcss-shadcn-ui-and-chromatic-visual-test-server
branch
from
9916411
to
1412605
Compare
…' to 'dist/config-inspector'
MFarabi619
force-pushed
the
mfarabi/feat/96-add-storybook-with-tailwindcss-shadcn-ui-and-chromatic-visual-test-server
branch
from
1412605
to
714bade
Compare
✅ Deploy Preview for website-cuhacking ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for eslint-cuhacking ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
nx add @nx/storybookSummary by CodeRabbit
New Features
Buttoncomponent with various styling options.Uicomponent for better user interface integration.Chores
.gitignoreto refine ignored files and directories.package.json.