Conversation
|
Caution Review failedThe pull request is closed. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds Storybook integration and a new themed Button UI component (component, styled wrapper, and stories), updates theme tokens (button2, colors), integrates Storybook config/preview with webpack aliases and theme decorator, replaces native buttons in EnvironmentVariables with the new Button, bumps Node in .nvmrc, and adds ESLint overrides for Storybook files. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR introduces Storybook support for a new reusable Button UI component and updates the Workspace Environments feature to use this component instead of custom styled buttons. The changes include comprehensive Storybook configuration, theme updates, and migration of existing button usage.
Key Changes:
- Added Storybook configuration with theme support
- Implemented a flexible Button component with multiple variants, sizes, and states
- Updated theme files to include button2 color definitions
- Migrated EnvironmentVariables component to use the new Button component
- Added polished library for color manipulation
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/bruno-app/storybook/preview.jsx | Storybook preview configuration with theme provider and global styles |
| packages/bruno-app/storybook/main.js | Storybook main config with webpack path aliases |
| packages/bruno-app/src/ui/Button/index.js | Main Button component with comprehensive props API |
| packages/bruno-app/src/ui/Button/StyledWrapper.js | Styled wrapper with variant, size, and state styles |
| packages/bruno-app/src/ui/Button/Button.stories.jsx | Comprehensive Storybook stories for Button |
| packages/bruno-app/src/themes/light.js | Added button2 theme colors for light mode |
| packages/bruno-app/src/themes/dark.js | Added button2 theme colors and updated TEXT_LINK color |
| packages/bruno-app/src/components/WorkspaceHome/WorkspaceEnvironments/EnvironmentList/EnvironmentDetails/EnvironmentVariables/index.js | Migrated to use new Button component |
| packages/bruno-app/src/components/WorkspaceHome/WorkspaceEnvironments/EnvironmentList/EnvironmentDetails/EnvironmentVariables/StyledWrapper.js | Removed old button styles |
| packages/bruno-app/package.json | Added Storybook scripts and polished dependency |
| package.json | Added Storybook dev dependencies |
| eslint.config.js | Added ESLint config for Storybook files |
| .nvmrc | Updated Node version from v22.11.0 to v22.12.0 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| justify-content: center; | ||
| width: ${(props) => (props.$fullWidth ? '100%' : 'auto')}; | ||
| font-family: inherit; | ||
| font-weight: ${(props) => fontWeightStyles[props.$fontWeight] || 400}; |
There was a problem hiding this comment.
The fontWeight default value fallback should be explicitly handled. When fontWeight prop is undefined, the code falls back to 400, but this might not match the intended design system. Consider defining a default weight in the Button component props or ensuring this fallback aligns with the design requirements.
| font-weight: ${(props) => fontWeightStyles[props.$fontWeight] || 400}; | |
| font-weight: ${(props) => fontWeightStyles[props.$fontWeight || 'regular']}; |
| const Button = ({ | ||
| children, | ||
| size = 'base', | ||
| variant = 'filled', | ||
| color = 'primary', | ||
| disabled = false, | ||
| loading = false, | ||
| icon, | ||
| iconPosition = 'left', | ||
| fullWidth = false, | ||
| type = 'button', |
There was a problem hiding this comment.
The button type defaults to 'button', but when used in a form context (as seen in EnvironmentVariables where type='submit' and type='reset' are used), this should be documented or the prop description should clarify when to use different types. Consider adding JSDoc or PropTypes documentation to the Button component.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/bruno-app/.gitignore (1)
22-25: Optional: Simplify log file patterns.The new
*.logpattern on line 25 already covers the specific patterns on lines 22-24 (npm-debug.log*,yarn-debug.log*,yarn-error.log*). You could optionally remove lines 22-24 for simplicity, though keeping them doesn't cause issues.packages/bruno-app/storybook/preview.jsx (2)
54-59: Consider using theme values instead of hardcoded colors.Per coding guidelines, styled-components should use theme props for colors. The theme object should have these values available (e.g., background, text colors).
🔎 Suggested approach
// Update background and text color based on theme const isDark = themeName === 'dark'; - const backgroundColor = isDark ? '#1e1e1e' : '#ffffff'; - const textColor = isDark ? '#d4d4d4' : '#333333'; + const backgroundColor = theme.sidebar?.bg || (isDark ? '#1e1e1e' : '#ffffff'); + const textColor = theme.text?.primary || (isDark ? '#d4d4d4' : '#333333'); document.body.style.backgroundColor = backgroundColor; document.body.style.color = textColor;Alternatively, if the theme doesn't expose these values directly, keeping the hardcoded fallbacks is acceptable for Storybook configuration.
24-30: Background values are duplicated.The background hex values here duplicate those in the decorator (lines 56-57). Consider extracting to constants to keep them in sync.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (14)
.nvmrceslint.config.jspackage.jsonpackages/bruno-app/.gitignorepackages/bruno-app/package.jsonpackages/bruno-app/src/components/WorkspaceHome/WorkspaceEnvironments/EnvironmentList/EnvironmentDetails/EnvironmentVariables/StyledWrapper.jspackages/bruno-app/src/components/WorkspaceHome/WorkspaceEnvironments/EnvironmentList/EnvironmentDetails/EnvironmentVariables/index.jspackages/bruno-app/src/themes/dark.jspackages/bruno-app/src/themes/light.jspackages/bruno-app/src/ui/Button/Button.stories.jsxpackages/bruno-app/src/ui/Button/StyledWrapper.jspackages/bruno-app/src/ui/Button/index.jspackages/bruno-app/storybook/main.jspackages/bruno-app/storybook/preview.jsx
💤 Files with no reviewable changes (1)
- packages/bruno-app/src/components/WorkspaceHome/WorkspaceEnvironments/EnvironmentList/EnvironmentDetails/EnvironmentVariables/StyledWrapper.js
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
packages/bruno-app/src/components/WorkspaceHome/WorkspaceEnvironments/EnvironmentList/EnvironmentDetails/EnvironmentVariables/index.jseslint.config.jspackages/bruno-app/src/themes/dark.jspackages/bruno-app/storybook/preview.jsxpackages/bruno-app/src/ui/Button/StyledWrapper.jspackages/bruno-app/src/ui/Button/index.jspackages/bruno-app/src/ui/Button/Button.stories.jsxpackages/bruno-app/storybook/main.jspackages/bruno-app/src/themes/light.js
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{jsx,tsx}: Use styled component's theme prop to manage CSS colors and not CSS variables when in the context of a styled component or any React component using the styled component
Styled Components are used as wrappers to define both self and children components style; Tailwind classes are used specifically for layout based styles
Styled Component CSS might also change layout but Tailwind classes shouldn't define colors
Files:
packages/bruno-app/storybook/preview.jsxpackages/bruno-app/src/ui/Button/Button.stories.jsx
🧠 Learnings (5)
📚 Learning: 2025-12-17T21:41:24.730Z
Learnt from: naman-bruno
Repo: usebruno/bruno PR: 6407
File: packages/bruno-app/src/components/Environments/ConfirmCloseEnvironment/index.js:5-41
Timestamp: 2025-12-17T21:41:24.730Z
Learning: Do not suggest PropTypes validation for React components in the Bruno codebase. The project does not use PropTypes, so reviews should avoid proposing PropTypes and rely on the existing typing/validation approach (e.g., TypeScript or alternative runtime checks) if applicable. This guideline applies broadly to all JavaScript/JSX components in the repo.
Applied to files:
packages/bruno-app/src/components/WorkspaceHome/WorkspaceEnvironments/EnvironmentList/EnvironmentDetails/EnvironmentVariables/index.jseslint.config.jspackages/bruno-app/src/themes/dark.jspackages/bruno-app/src/ui/Button/StyledWrapper.jspackages/bruno-app/src/ui/Button/index.jspackages/bruno-app/storybook/main.jspackages/bruno-app/src/themes/light.js
📚 Learning: 2025-12-05T20:31:33.005Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : No trailing commas
Applied to files:
eslint.config.js
📚 Learning: 2025-12-05T20:31:33.005Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.{jsx,tsx} : Use styled component's theme prop to manage CSS colors and not CSS variables when in the context of a styled component or any React component using the styled component
Applied to files:
packages/bruno-app/src/themes/dark.jspackages/bruno-app/storybook/preview.jsxpackages/bruno-app/src/ui/Button/StyledWrapper.js
📚 Learning: 2025-12-05T20:31:33.005Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.{jsx,tsx} : Styled Components are used as wrappers to define both self and children components style; Tailwind classes are used specifically for layout based styles
Applied to files:
packages/bruno-app/storybook/preview.jsxpackages/bruno-app/src/ui/Button/StyledWrapper.jspackages/bruno-app/src/ui/Button/index.js
📚 Learning: 2025-12-05T20:31:33.005Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.{jsx,tsx} : Styled Component CSS might also change layout but Tailwind classes shouldn't define colors
Applied to files:
packages/bruno-app/storybook/preview.jsxpackages/bruno-app/src/ui/Button/StyledWrapper.js
🧬 Code graph analysis (5)
packages/bruno-app/src/components/WorkspaceHome/WorkspaceEnvironments/EnvironmentList/EnvironmentDetails/EnvironmentVariables/index.js (2)
packages/bruno-app/src/ui/Button/index.js (1)
Button(4-79)packages/bruno-app/src/components/Environments/EnvironmentSettings/EnvironmentList/EnvironmentDetails/EnvironmentVariables/index.js (2)
handleSave(305-351)handleReset(353-368)
packages/bruno-app/src/themes/dark.js (1)
packages/bruno-app/src/themes/light.js (1)
colors(1-36)
packages/bruno-app/src/ui/Button/index.js (1)
packages/bruno-app/src/ui/Button/StyledWrapper.js (1)
StyledWrapper(206-272)
packages/bruno-app/src/ui/Button/Button.stories.jsx (1)
packages/bruno-app/src/ui/Button/index.js (1)
Button(4-79)
packages/bruno-app/src/themes/light.js (1)
packages/bruno-app/src/themes/dark.js (1)
colors(1-35)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: CodeQL analysis (javascript-typescript)
- GitHub Check: Playwright E2E Tests
- GitHub Check: Unit Tests
- GitHub Check: CLI Tests
- GitHub Check: SSL Tests - macOS
- GitHub Check: SSL Tests - Linux
- GitHub Check: SSL Tests - Windows
🔇 Additional comments (23)
.nvmrc (1)
1-1: Node.js version bump to v22.12.0.Routine patch-level update. Verify this bump is necessary for Storybook compatibility or if it's a general maintenance upgrade, but no concerns with the change itself.
packages/bruno-app/src/themes/light.js (1)
351-374: LGTM!The
button2theme definitions are well-structured and consistent with the existing theme pattern. The color mappings appropriately reference existing constants.eslint.config.js (1)
120-131: LGTM!The ESLint override for Storybook config files is properly configured with Node globals for CommonJS usage.
packages/bruno-app/src/themes/dark.js (2)
5-5: LGTM!The color constant updates (TEXT_LINK adjustment and WHITE/BLACK additions) align the dark theme with the light theme structure.
Also applies to: 10-11
348-371: LGTM!The
button2theme definitions are well-structured and use appropriate dark mode colors that complement the existing theme.packages/bruno-app/package.json (2)
13-14: LGTM!The Storybook scripts are properly configured with the custom config directory and standard port.
68-68: No action needed — polished is already on the latest stable version.Version 4.3.1 is the latest stable release (published Feb 2024) and remains current. The
^4.3.1constraint is appropriate for allowing compatible updates.packages/bruno-app/src/components/WorkspaceHome/WorkspaceEnvironments/EnvironmentList/EnvironmentDetails/EnvironmentVariables/index.js (2)
20-20: LGTM!The Button import uses the correct aliased path.
429-437: LGTM!The refactor from native buttons to the Button component is well-executed. The layout classes and Button props (size, color, variant, onClick, data-testid) are properly configured.
packages/bruno-app/storybook/main.js (1)
1-35: LGTM!The Storybook configuration is well-structured with proper path aliases that enable clean imports in stories. The
webpackFinalhook correctly extends the existing config without replacing it.package.json (1)
23-27: No action required — Storybook packages are compatible.The
@storybook/addon-webpack5-compiler-babelv4.0.0 is designed for Storybook 10.x (requires >= 10.0.0). The version disparity reflects independent versioning across packages, not an incompatibility issue.Likely an incorrect or invalid review comment.
packages/bruno-app/src/ui/Button/index.js (3)
1-21: LGTM!Clean component definition with sensible defaults. The prop destructuring is well-organized and follows project conventions.
22-30: Click handlers are redundant but harmless.These guards are technically unnecessary since the
<button>already receivesdisabled={disabled || loading}, which prevents native click events. However, they provide defense-in-depth for edge cases (e.g., programmatic invocation) and don't add meaningful overhead.
32-78: Solid implementation of the Button render logic.The loading/icon/children rendering order is correct, and transient props (
$size,$variant, etc.) properly avoid DOM pollution. The inline spinner SVG keeps dependencies minimal.packages/bruno-app/storybook/preview.jsx (2)
9-13: LGTM!GlobalStyle correctly sets the font-family for the Storybook environment.
49-70: Decorator pattern is correct for theme switching.The ThemeProvider wrapping and GlobalStyle injection follow standard Storybook practices.
packages/bruno-app/src/ui/Button/StyledWrapper.js (5)
1-11: LGTM!Clean imports and standard spin animation keyframes.
13-89: Size styles are well-structured.Consistent pattern for padding, font-size, gap, and icon/spinner dimensions across all size variants.
74-88: Verify:lgsize usesfont.size.mdinstead of a larger size.Line 76 uses
props.theme.font.size.mdfor thelgsize variant. Is this intentional, or should it reference a larger font size (e.g.,lg)?
119-204: Variant styles correctly use theme colors.The
getVariantStylesfunction properly derives colors fromprops.theme.button2.color[color]and usesdarken/rgbafrom polished for hover/active states. This follows the project's styled-component theming guidelines.
206-272: StyledWrapper is comprehensive and well-organized.Focus-visible, disabled states, and content layout are all properly handled. The transient props prevent DOM pollution.
packages/bruno-app/src/ui/Button/Button.stories.jsx (2)
65-115: Local icon components are appropriate for stories.Defining PlusIcon, SendIcon, and TrashIcon inline keeps stories self-contained without polluting the main codebase.
117-430: Excellent story coverage.The stories comprehensively demonstrate all variants, sizes, colors, states, and combinations. This will be valuable for both documentation and visual regression testing.
Button Storybook + Updated Workspace Environments to use Button UI Component
Summary by CodeRabbit
New Features
Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.