-
Notifications
You must be signed in to change notification settings - Fork 236
style: styling improvements to align w/ brand #54
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
WalkthroughThis pull request updates dependency versions and refines the UI. In the Changes
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
🔇 Additional comments (8)
🪧 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
apps/app/src/components/forms/risks/task/create-task-comment-form.tsx (1)
93-102: Consider removing the arrow icon for consistency.While the action variant update is good, the arrow icon should be removed to maintain consistency with other comment forms (e.g., create-risk-comment-form.tsx).
<div className="flex items-center justify-center"> {t("common.comments.save")} - <ArrowRightIcon className="ml-2 h-4 w-4" /> </div>apps/app/src/components/forms/risks/risk-overview.tsx (1)
82-82: Remove debug console.log statement.Debug statement should be removed before merging.
- console.log(form.formState.defaultValues);apps/app/src/components/forms/risks/task/create-task-form.tsx (1)
100-100: Consider documenting the height calculation.The fixed height calculation
h-[calc(100vh-250px)]might need documentation for future maintainability.- <div className="h-[calc(100vh-250px)] scrollbar-hide overflow-auto"> + {/* Height accounts for header, footer, and padding */} + <div className="h-[calc(100vh-250px)] scrollbar-hide overflow-auto">packages/ui/src/globals.css (1)
56-56: Inconsistent HSL Formatting for--primaryin Dark ModeNotice that
--primaryis set as151, 100%, 43%(with commas) in the dark theme, whereas in the light theme the values are space-separated (e.g.,151 100% 43%). For consistency and to avoid potential parsing issues when used with a function likehsl(), please update this to151 100% 43%.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/app/languine.lockis excluded by!**/*.lock
📒 Files selected for processing (28)
apps/app/src/components/forms/risks/create-risk-comment-form.tsx(1 hunks)apps/app/src/components/forms/risks/create-risk-form.tsx(1 hunks)apps/app/src/components/forms/risks/inherent-risk-form.tsx(1 hunks)apps/app/src/components/forms/risks/residual-risk-form.tsx(1 hunks)apps/app/src/components/forms/risks/risk-overview.tsx(1 hunks)apps/app/src/components/forms/risks/task/create-task-comment-form.tsx(1 hunks)apps/app/src/components/forms/risks/task/create-task-form.tsx(1 hunks)apps/app/src/components/forms/risks/task/update-task-form.tsx(1 hunks)apps/app/src/components/forms/risks/task/update-task-overview-form.tsx(1 hunks)apps/app/src/components/forms/risks/update-risk-form.tsx(1 hunks)apps/app/src/components/risks/risk-comments.tsx(1 hunks)apps/app/src/components/risks/tasks/task-comments.tsx(1 hunks)apps/app/src/components/tables/people/filter-toolbar.tsx(5 hunks)apps/app/src/components/tables/risk-register/data-table-header.tsx(1 hunks)apps/app/src/components/tables/risk-register/filter-toolbar.tsx(2 hunks)apps/app/src/components/tables/risk-tasks/filter-toolbar.tsx(2 hunks)apps/app/src/components/upload-dialog.tsx(1 hunks)apps/app/src/locales/en.ts(3 hunks)apps/app/src/locales/es.ts(3 hunks)apps/app/src/locales/fr.ts(2 hunks)apps/app/src/locales/no.ts(3 hunks)apps/app/src/locales/pt.ts(3 hunks)apps/web/src/app/(home)/page.tsx(2 hunks)apps/web/src/app/components/site-footer.tsx(1 hunks)apps/web/src/app/components/waitlist-form.tsx(2 hunks)packages/ui/src/components/button.tsx(4 hunks)packages/ui/src/components/secondary-menu.tsx(2 hunks)packages/ui/src/globals.css(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/app/src/locales/no.ts
- apps/app/src/locales/es.ts
- apps/app/src/locales/fr.ts
- packages/ui/src/components/button.tsx
- apps/app/src/locales/en.ts
- apps/app/src/locales/pt.ts
🔇 Additional comments (34)
apps/web/src/app/components/site-footer.tsx (1)
6-34: LGTM! Improved footer structure and styling.The changes enhance the footer's accessibility and visual consistency through:
- Semantic list structure for navigation links
- Consistent muted text with hover underline effects
- Right-aligned content layout
apps/web/src/app/(home)/page.tsx (1)
2-2: LGTM! Enhanced UI consistency.The changes improve brand alignment through:
- Consistent button styling using shared UI component
- Refined text sizing for better visual hierarchy
Also applies to: 37-48
apps/app/src/components/upload-dialog.tsx (1)
57-60: LGTM! Consistent button styling.The changes enhance brand consistency through:
- Unified action variant matching other buttons
- Proper localization support
packages/ui/src/components/secondary-menu.tsx (1)
47-47: LGTM! Refined menu styling.The changes enhance visual consistency through:
- Optimized spacing for child menu items
- Cleaner active state indication with underline
Also applies to: 58-59
apps/app/src/components/risks/tasks/task-comments.tsx (1)
30-30: LGTM! Button styling aligns with brand guidelines.The update to use
variant="action"for the comment addition button maintains consistency with the new brand styling.apps/app/src/components/risks/risk-comments.tsx (1)
33-33: LGTM! Button styling aligns with brand guidelines.The update to use
variant="action"for the comment addition button maintains consistency with the new brand styling while preserving the icon.apps/app/src/components/tables/people/filter-toolbar.tsx (2)
83-86: LGTM! Button styling aligns with brand guidelines.The update to use
variant="action"for the invite button maintains consistency with the new brand styling while preserving the icon.
10-10:Details
✅ Verification successful
Verify if useCallback removal was intentional.
The removal of
useCallbackfrom the import list seems unrelated to the styling changes. Please verify if this was intentional, as the hook is still used in the component (e.g.,createQueryString).
🏁 Script executed:
#!/bin/bash # Check if useCallback is used in the file rg "useCallback" apps/app/src/components/tables/people/filter-toolbar.tsxLength of output: 187
UseCallback import verified & button styling approved
The code still imports and uses
useCallback(e.g., in thecreateQueryStringfunction), so the observation about its removal is incorrect. Additionally, the styling updates on the button adhere to our brand guidelines.apps/app/src/components/tables/risk-tasks/filter-toolbar.tsx (2)
6-6: LGTM! Import path uses package alias.The update to use
@bubba/ui/cnimproves consistency by using the package alias instead of a relative path.
100-103: LGTM! Button styling and text align with brand guidelines.The changes maintain consistency with the new brand styling:
- Using
variant="action"for the add button- Updated text to use the standardized "addNew" translation key
- Preserved the Plus icon
apps/web/src/app/components/waitlist-form.tsx (2)
85-85: Verify input field appearance after style simplification.The removal of specific height and padding styles could affect the visual consistency of the input field. Please ensure the simplified styling maintains proper alignment with other form elements.
96-112: LGTM! Button styling aligns with brand guidelines.The change from outline to action variant maintains consistency with other form buttons across the application.
apps/app/src/components/forms/risks/create-risk-comment-form.tsx (1)
92-100: LGTM! Button styling matches form standards.The update to use the action variant and removal of the arrow icon improves visual consistency across forms.
apps/app/src/components/forms/risks/update-risk-form.tsx (1)
111-121: LGTM! Clean button implementation with proper loading state.The action variant update is well-implemented and maintains good UX with the loading indicator.
apps/app/src/components/forms/risks/inherent-risk-form.tsx (1)
111-121: LGTM! Button styling aligns with brand guidelines.The change to
variant="action"maintains consistency with other form submission buttons across the application.apps/app/src/components/forms/risks/task/update-task-overview-form.tsx (1)
109-119: LGTM! Button styling matches brand guidelines.The change to
variant="action"maintains consistency with other form submission buttons.apps/app/src/components/forms/risks/residual-risk-form.tsx (1)
117-127: LGTM! Button styling follows brand guidelines.The change to
variant="action"maintains consistency with other form submission buttons.apps/app/src/components/tables/risk-register/data-table-header.tsx (1)
111-111: LGTM! Improved responsive design.Changed visibility breakpoint from
smtomdfor the owner column, which helps reduce table width on smaller screens and improves mobile experience.apps/app/src/components/tables/risk-register/filter-toolbar.tsx (3)
113-114: LGTM! Improved layout structure.The new layout with
flex-rowandmin-w-0provides better overflow handling and alignment.
125-130: LGTM! Enhanced mobile responsiveness.Good addition of a mobile-specific action button, improving the UX on smaller screens.
138-138: LGTM! Optimized select widths.The width adjustments for status and department filters provide better space utilization:
- Status: More flexible with
w-auto min-w-[100px]- Department: Compact but readable with
w-[150px]Also applies to: 158-158
apps/app/src/components/forms/risks/task/update-task-form.tsx (1)
182-186: LGTM! Consistent button styling.Button variant update to "action" aligns with the brand styling improvements.
apps/app/src/components/forms/risks/risk-overview.tsx (1)
225-229: LGTM! Consistent button styling.Button variant update to "action" aligns with the brand styling improvements.
apps/app/src/components/forms/risks/task/create-task-form.tsx (1)
235-239: LGTM! Consistent button styling.Button variant update to "action" aligns with the brand styling improvements.
apps/app/src/components/forms/risks/create-risk-form.tsx (1)
269-273: LGTM! Button styling aligns with brand guidelines.The Button component's variant change to "action" maintains consistency with the broader styling improvements across the application.
packages/ui/src/globals.css (9)
7-20: Update Color Palette in Light Theme (:root)The new HSL values for properties such as
--background,--foreground,--card,--popover,--primary, etc., clearly reflect the intended brand styling improvements. These updates align with the new design direction. Please double-check that these values, when applied via functions likehsl(var(--primary)), render as expected in the UI.
23-25: Revised Border, Input, and Ring Colors in Light ThemeThe updated custom properties
--border,--input, and--ringnow use fresh HSL values aligned with the new palette. Ensure these provide sufficient contrast and meet accessibility guidelines.
28-28: Clarification on Success/Warning/Chart ColorsThe comment indicates that the success, warning, and chart colors remain unchanged. This explicit annotation aids clarity for future updates—good job on that.
37-46: Novel-Highlight Colors for Light ThemeThe novel-highlight custom properties are preserved as before, ensuring that these colors remain consistent with prior design decisions. Confirm that they integrate well with the overall light theme.
50-55: Dark Mode Base Color UpdatesThe dark mode properties (e.g.,
--background,--foreground,--card,--popover) have been updated to provide a clear contrast against the light theme. It’s important to verify that these values maintain legibility and visual coherence in the UI.
57-63: Dark Mode Accent and Secondary ColorsThe custom properties for
--primary-foreground,--secondary,--secondary-foreground,--muted,--muted-foreground,--accent, and--accent-foregroundhave been updated to suit the dark mode. These changes look in line with achieving a coherent dark theme. Please verify against the design guidelines for any needed contrasts or adjustments.
66-68: Updated Border, Input, and Ring Colors in Dark ModeThe values for
--border,--input, and--ringin the dark class have been revised to225 2% 36%and151 100% 43%respectively. This shift supports the overall dark theme aesthetic. It would be good to check their appearance in various states (hover, focus, etc.) to ensure consistency.
70-72: Maintained Success/Warning/Chart Colors in Dark ModeThe preservation of the success, warning, and chart color values in dark mode is clearly documented. This ensures consistent visual cues across both themes. Make sure these retained colors appropriately contrast with the dark background.
79-88: Novel-Highlight Colors for Dark ModeThe novel-highlight colors have been updated for dark mode (e.g., default changes to
#000000, along with new hex values for purple, red, yellow, blue, green, orange, pink, and gray). These updates should enrich the brand’s visual identity within dark mode. Please confirm that the contrast and hierarchy meet design expectations.
Summary by CodeRabbit
Chores
Localization
Style/UI Enhancements
Visual Improvements
These improvements result in a more cohesive, responsive, and modern user interface.