-
Notifications
You must be signed in to change notification settings - Fork 433
Refactor UI component styling with updated rounded designs #1899
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
✅ Deploy Preview for hyprnote ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for hyprnote-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughUI-only updates across the desktop app: primarily replaced smaller border radii (rounded-md/rounded-lg) with rounded-xl, adjusted several paddings/margins and some tab/menu markup reorganization. No control-flow, state, exported API, or runtime behavior changes. Changes
Sequence Diagram(s)Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Areas to pay attention to:
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
614ac53 to
0dfe5fe
Compare
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/components/main/sidebar/banner/component.tsx (1)
61-68: Fix invalid Tailwind CSS gradient class name.Line 64 uses
bg-linear-to-t, which is not a valid Tailwind CSS class. The correct syntax for gradients isbg-gradient-to-{direction}.Apply this diff to fix the gradient class:
<Button onClick={banner.secondaryAction.onClick} variant="outline" - className="w-full rounded-full bg-linear-to-t from-neutral-200 to-neutral-100 text-neutral-900" + className="w-full rounded-full bg-gradient-to-t from-neutral-200 to-neutral-100 text-neutral-900" > {banner.secondaryAction.label} </Button>
🧹 Nitpick comments (3)
apps/desktop/src/components/settings/ai/llm/configure.tsx (1)
87-90: Provider cards’ static rounded-xl shells are appropriateUsing a plain
"rounded-xl border-2 border-dashed bg-neutral-50"class on both AccordionItems simplifies the markup and matches the new rounded card style used elsewhere. Keepingcn([...])only where conditional styles are needed (triggers) is a good balance; no further refactor is required unless you want to DRY the shared shell class later.Also applies to: 166-169
apps/desktop/src/components/main/sidebar/profile/shared.tsx (1)
17-52: LGTM - Layout restructure maintains functionality.The MenuItem component has been restructured with improved layout semantics. The changes correctly preserve all functionality (badge, suffix icon, click handling) while updating the visual presentation.
Optional refinement: Some
cn()calls (lines 19-24, 34-39) contain only static strings without conditional logic. While the current approach follows the guideline to "pass an array and split by logical grouping," these could be simplified to plain strings for slightly better performance:className="flex w-full justify-between rounded-lg px-3 py-1.5 text-sm text-black transition-colors hover:bg-neutral-100"However, keeping the array format provides consistency and makes future conditional additions easier, so the current implementation is acceptable.
apps/desktop/src/components/main/sidebar/profile/index.tsx (1)
141-143: Notifications view is now effectively unreachable; consider trimming or annotatingWith
handleClickNotificationsand theNotificationsMenuHeaderusage commented out, nothing setscurrentViewto"notifications", so the<NotificationsMenuContent>branch will never render. That aligns with the goal of temporarily disabling the header, but it does leave unused view state and related layout logic in place. Consider either adding a brief TODO noting this temporary disablement or pruning the unused"notifications"view and associated effects until it’s reintroduced, to keep this component simpler to reason about.Also applies to: 202-205
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/ui/src/components/ui/button.tsxis excluded by!packages/ui/src/components/ui/**
📒 Files selected for processing (20)
apps/desktop/src/components/chat/body/index.tsx(1 hunks)apps/desktop/src/components/chat/header.tsx(1 hunks)apps/desktop/src/components/main/body/index.tsx(1 hunks)apps/desktop/src/components/main/body/search.tsx(1 hunks)apps/desktop/src/components/main/body/shared.tsx(3 hunks)apps/desktop/src/components/main/sidebar/banner/component.tsx(1 hunks)apps/desktop/src/components/main/sidebar/index.tsx(1 hunks)apps/desktop/src/components/main/sidebar/profile/auth.tsx(1 hunks)apps/desktop/src/components/main/sidebar/profile/index.tsx(5 hunks)apps/desktop/src/components/main/sidebar/profile/shared.tsx(1 hunks)apps/desktop/src/components/settings/ai/llm/configure.tsx(2 hunks)apps/desktop/src/components/settings/ai/llm/select.tsx(1 hunks)apps/desktop/src/components/settings/ai/stt/configure.tsx(2 hunks)apps/desktop/src/components/settings/ai/stt/select.tsx(1 hunks)apps/desktop/src/components/settings/template/editor.tsx(1 hunks)apps/desktop/src/components/settings/template/index.tsx(1 hunks)apps/desktop/src/components/settings/template/local.tsx(1 hunks)apps/desktop/src/components/settings/template/sections.tsx(1 hunks)apps/desktop/src/components/settings/template/shared.tsx(1 hunks)apps/desktop/src/routes/app/settings/_layout.tsx(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
If there are many classNames with conditional logic, usecn(import from@hypr/utils). It is similar toclsx. Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/desktop/src/components/chat/body/index.tsxapps/desktop/src/components/settings/template/index.tsxapps/desktop/src/components/settings/template/sections.tsxapps/desktop/src/components/settings/ai/stt/select.tsxapps/desktop/src/components/settings/ai/llm/select.tsxapps/desktop/src/components/settings/template/shared.tsxapps/desktop/src/components/settings/ai/llm/configure.tsxapps/desktop/src/components/main/sidebar/profile/shared.tsxapps/desktop/src/routes/app/settings/_layout.tsxapps/desktop/src/components/main/body/search.tsxapps/desktop/src/components/settings/template/editor.tsxapps/desktop/src/components/chat/header.tsxapps/desktop/src/components/main/body/shared.tsxapps/desktop/src/components/main/body/index.tsxapps/desktop/src/components/settings/ai/stt/configure.tsxapps/desktop/src/components/settings/template/local.tsxapps/desktop/src/components/main/sidebar/banner/component.tsxapps/desktop/src/components/main/sidebar/profile/auth.tsxapps/desktop/src/components/main/sidebar/profile/index.tsxapps/desktop/src/components/main/sidebar/index.tsx
🧠 Learnings (2)
📚 Learning: 2025-11-24T16:32:19.706Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:32:19.706Z
Learning: Applies to **/*.{ts,tsx} : If there are many classNames with conditional logic, use `cn` (import from `hypr/utils`), similar to `clsx`. Always pass an array and split by logical grouping.
Applied to files:
apps/desktop/src/components/chat/body/index.tsxapps/desktop/src/components/settings/ai/stt/select.tsxapps/desktop/src/components/settings/ai/llm/select.tsxapps/desktop/src/components/main/body/shared.tsxapps/desktop/src/components/settings/ai/stt/configure.tsx
📚 Learning: 2025-11-24T16:32:23.055Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:32:23.055Z
Learning: Applies to **/*.{ts,tsx} : If there are many classNames with conditional logic, use `cn` (import from `hypr/utils`). It is similar to `clsx`. Always pass an array and split by logical grouping.
Applied to files:
apps/desktop/src/components/chat/body/index.tsxapps/desktop/src/components/settings/ai/stt/select.tsxapps/desktop/src/components/settings/ai/llm/select.tsxapps/desktop/src/components/main/body/shared.tsxapps/desktop/src/components/settings/ai/stt/configure.tsx
🧬 Code graph analysis (4)
apps/desktop/src/components/main/sidebar/profile/shared.tsx (1)
packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/routes/app/settings/_layout.tsx (1)
packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/main/body/shared.tsx (1)
packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/main/sidebar/profile/index.tsx (1)
apps/desktop/src/components/main/sidebar/profile/ota/index.tsx (1)
UpdateChecker(9-159)
🪛 Biome (2.1.2)
apps/desktop/src/routes/app/settings/_layout.tsx
[error] 186-186: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
⏰ 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). (8)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
- GitHub Check: fmt
- GitHub Check: ci (macos, macos-14)
- GitHub Check: ci (macos, depot-macos-14)
- GitHub Check: ci (linux, depot-ubuntu-24.04-8)
- GitHub Check: ci (linux, depot-ubuntu-22.04-8)
🔇 Additional comments (19)
apps/desktop/src/components/main/body/shared.tsx (3)
83-97: Active/selected styling matrix is clear and exhaustiveThe four explicit
active/selectedbranches are mutually exclusive and cover all states, and thecn([...])usage with grouped conditionals matches our Tailwind/cn conventions. No issues from a logic or UX perspective.As per coding guidelines, the cn array grouping is used correctly.
103-106: Hover opacity swap between icon and close overlay is consistentUsing
cn([baseClasses, isHovered ? "opacity-0/100" : "opacity-100/0"])on both overlays keeps the fade transition symmetric and readable; behavior remains identical while aligning with the array-based cn pattern.Based on learnings, this matches the preferred cn usage.
Also applies to: 118-121
130-136: Close button color variants align with tab state semanticsThe
active/selected-based text and hover colors on the close button are consistent with the parent tab’s color scheme and keep neutral vs. red states easy to scan. No functional concerns here.apps/desktop/src/components/main/body/search.tsx (1)
142-148: Search input radius change looks good
rounded-xlon the search input aligns with the new rounded design and keeps focus/placeholder behavior intact. No logic impact here.apps/desktop/src/components/settings/ai/stt/select.tsx (1)
74-80: STT model card radius update is consistentSwitching the container to
rounded-xlkeeps the existing conditional bg logic and matches the rest of the rounded settings cards.cngrouping also follows the array pattern from the guidelines.apps/desktop/src/components/chat/body/index.tsx (1)
38-42: Right-panel chat container styling matches new card shapeThe
border border-neutral-200 rounded-xl rounded-b-nonevariant forRightPanelOpencleanly fits the new rounded card style while keeping the bottom edge flush for adjoining UI. As long as the header/composer are visually stacked as expected, this looks good.apps/desktop/src/components/main/body/index.tsx (1)
338-345: StandardTabWrapper card radius change is safeUpdating the inner wrapper to
rounded-xlkeeps the same border/overflow behavior and just adjusts the visual shape of the main content card to match the new design language.apps/desktop/src/components/chat/header.tsx (1)
38-41: Chat header rounded-xl variant matches the updated panelThe
chat.mode === "RightPanelOpen" && "border rounded-xl"change cleanly aligns the header with the new rounded card aesthetic. The extraborder-bin the base class is redundant but not harmful, so no change needed unless you want to micro-optimize.apps/desktop/src/components/settings/ai/llm/select.tsx (1)
80-86: LLM model card radius aligns with STT and global stylingMoving this container to
rounded-xlkeeps the existing conditional background and form behavior while matching the STT card and the rest of the updated settings surfaces. Class composition viacn([...])is also consistent with the documented pattern. Based on learnings, this is the intended usage.apps/desktop/src/components/settings/ai/stt/configure.tsx (1)
94-94: LGTM - Appropriate simplification of static classNames.Removing the
cnwrapper for purely static className strings is correct, as the coding guidelines specify usingcnwhen there are "many classNames with conditional logic." Since these classNames have no conditional logic, the direct string is cleaner and more efficient.The border radius update from
rounded-lgtorounded-xlaligns with the PR's UI styling objectives.Also applies to: 167-167
apps/desktop/src/routes/app/settings/_layout.tsx (2)
142-142: LGTM - Consistent border radius updates.The updates to
rounded-xlacross the layout container, header wrappers, and group containers maintain visual consistency with the PR's design objectives.Also applies to: 233-233, 273-273
174-179: LGTM - Appropriate use ofcnwith conditional logic.The
cn()utility is correctly used here with conditional expressions (expandHeight && "flex-1"and the ternary for active tab styling), which aligns with the coding guideline to usecnfor "many classNames with conditional logic."Also applies to: 190-196
apps/desktop/src/components/settings/template/editor.tsx (1)
71-71: LGTM - Consistent border radius styling.The addition of
rounded-xlto the Textarea maintains consistency with the broader UI styling updates across the PR.apps/desktop/src/components/settings/template/sections.tsx (1)
216-222: LGTM - Border radius update with proper conditional styling.The update from
rounded-mdtorounded-xlis consistent with the PR's styling objectives, and the use ofcn()with conditional focus state styling is appropriate.apps/desktop/src/components/settings/template/index.tsx (1)
42-42: LGTM - Consistent empty state styling.The border radius update to
rounded-xlmaintains visual consistency with other empty state containers across the PR.apps/desktop/src/components/settings/template/local.tsx (1)
25-25: LGTM - Consistent empty state styling.The border radius update to
rounded-xlaligns with the empty state styling in the relatedindex.tsxfile and maintains visual consistency across the template components.apps/desktop/src/components/settings/template/shared.tsx (1)
39-40: Rounded card container and badge styling look consistentThe move to
rounded-xlandoverflow-clipon the card plus the matchingrounded-bl-xlon the “Made by me” badge are consistent with the new rounded design and don’t affect interaction or accessibility behavior.Also applies to: 47-47
apps/desktop/src/components/main/sidebar/profile/index.tsx (1)
190-192: Profile popover and trigger rounding/spacing updates look goodThe switch to
rounded-xlwithoverflow-hiddenon the expanded profile panel and the bottom wrapper, plus the slight padding/myreductions, cleanly align this area with the new rounded design language without altering behavior or focus/interaction patterns.Also applies to: 207-207, 240-245
apps/desktop/src/components/main/sidebar/index.tsx (1)
27-31: Sidebar header layout and rounded-xl styling are consistent with the new UIThe updated header flex layout and
rounded-xl bg-neutral-50styling preserve existing behavior (fixed-height, right-aligned controls) while matching the broader rounded design direction; nothing functionally risky here.
934cf7c to
26d8d5f
Compare
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 (1)
apps/desktop/src/components/main/sidebar/profile/shared.tsx (1)
20-20: Verify rounded-lg vs rounded-xl consistency.The PR objectives mention "Updated button and UI component styling with rounded corners," primarily replacing rounded-md/rounded-lg with rounded-xl. This button uses
rounded-lg. Is this intentional for the sidebar profile button, or should it berounded-xlfor consistency?
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/ui/src/components/ui/button.tsxis excluded by!packages/ui/src/components/ui/**
📒 Files selected for processing (20)
apps/desktop/src/components/chat/body/index.tsx(1 hunks)apps/desktop/src/components/chat/header.tsx(1 hunks)apps/desktop/src/components/main/body/index.tsx(1 hunks)apps/desktop/src/components/main/body/search.tsx(1 hunks)apps/desktop/src/components/main/body/shared.tsx(3 hunks)apps/desktop/src/components/main/sidebar/banner/component.tsx(1 hunks)apps/desktop/src/components/main/sidebar/index.tsx(1 hunks)apps/desktop/src/components/main/sidebar/profile/auth.tsx(1 hunks)apps/desktop/src/components/main/sidebar/profile/index.tsx(5 hunks)apps/desktop/src/components/main/sidebar/profile/shared.tsx(1 hunks)apps/desktop/src/components/settings/ai/llm/configure.tsx(2 hunks)apps/desktop/src/components/settings/ai/llm/select.tsx(1 hunks)apps/desktop/src/components/settings/ai/stt/configure.tsx(2 hunks)apps/desktop/src/components/settings/ai/stt/select.tsx(1 hunks)apps/desktop/src/components/settings/template/editor.tsx(1 hunks)apps/desktop/src/components/settings/template/index.tsx(1 hunks)apps/desktop/src/components/settings/template/local.tsx(1 hunks)apps/desktop/src/components/settings/template/sections.tsx(1 hunks)apps/desktop/src/components/settings/template/shared.tsx(1 hunks)apps/desktop/src/routes/app/settings/_layout.tsx(4 hunks)
✅ Files skipped from review due to trivial changes (2)
- apps/desktop/src/components/settings/template/index.tsx
- apps/desktop/src/components/settings/template/local.tsx
🚧 Files skipped from review as they are similar to previous changes (11)
- apps/desktop/src/components/main/sidebar/banner/component.tsx
- apps/desktop/src/components/main/sidebar/profile/index.tsx
- apps/desktop/src/components/settings/template/editor.tsx
- apps/desktop/src/components/settings/ai/stt/select.tsx
- apps/desktop/src/components/main/body/search.tsx
- apps/desktop/src/components/chat/header.tsx
- apps/desktop/src/components/settings/template/sections.tsx
- apps/desktop/src/components/settings/ai/llm/configure.tsx
- apps/desktop/src/components/settings/ai/stt/configure.tsx
- apps/desktop/src/components/main/sidebar/profile/auth.tsx
- apps/desktop/src/components/chat/body/index.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
If there are many classNames with conditional logic, usecn(import from@hypr/utils). It is similar toclsx. Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/desktop/src/components/settings/ai/llm/select.tsxapps/desktop/src/routes/app/settings/_layout.tsxapps/desktop/src/components/main/body/shared.tsxapps/desktop/src/components/settings/template/shared.tsxapps/desktop/src/components/main/sidebar/profile/shared.tsxapps/desktop/src/components/main/body/index.tsxapps/desktop/src/components/main/sidebar/index.tsx
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: apps/web/content/changelog/AGENTS.md:0-0
Timestamp: 2025-11-24T16:32:30.770Z
Learning: Applies to apps/web/content/changelog/** : Only include desktop-related changes in the changelog when reading through commits and diffs
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: apps/web/content/changelog/AGENTS.md:0-0
Timestamp: 2025-11-24T16:32:29.314Z
Learning: Applies to apps/web/content/changelog/** : Only keep desktop-related changes when maintaining changelog entries from commits and diffs
📚 Learning: 2025-11-24T16:32:19.706Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:32:19.706Z
Learning: Applies to **/*.{ts,tsx} : If there are many classNames with conditional logic, use `cn` (import from `hypr/utils`), similar to `clsx`. Always pass an array and split by logical grouping.
Applied to files:
apps/desktop/src/components/settings/ai/llm/select.tsxapps/desktop/src/components/main/body/shared.tsx
📚 Learning: 2025-11-24T16:32:23.055Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:32:23.055Z
Learning: Applies to **/*.{ts,tsx} : If there are many classNames with conditional logic, use `cn` (import from `hypr/utils`). It is similar to `clsx`. Always pass an array and split by logical grouping.
Applied to files:
apps/desktop/src/components/settings/ai/llm/select.tsxapps/desktop/src/components/main/body/shared.tsx
🧬 Code graph analysis (3)
apps/desktop/src/routes/app/settings/_layout.tsx (1)
packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/main/body/shared.tsx (1)
packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/main/sidebar/profile/shared.tsx (2)
packages/utils/src/cn.ts (1)
cn(20-22)apps/storybook/stories/Button.stories.tsx (1)
Icon(72-77)
⏰ 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). (8)
- GitHub Check: ci (macos, macos-14)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: ci (linux, depot-ubuntu-24.04-8)
- GitHub Check: ci (macos, depot-macos-14)
- GitHub Check: ci (linux, depot-ubuntu-22.04-8)
- GitHub Check: fmt
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
🔇 Additional comments (13)
apps/desktop/src/components/main/body/index.tsx (1)
338-343: Rounded-xl container matches the new rounded design directionSwitching the main tab wrapper to
rounded-xlkeeps behavior unchanged while aligning with the updated, more-rounded UI style.overflow-hiddenon the same element still correctly clips inner content. Looks good as-is.apps/desktop/src/components/main/sidebar/index.tsx (1)
24-32: LGTM! Clean styling update following guidelines.The className composition properly uses
cnwith logical grouping (layout, dimensions, behavior, styling) and the border radius update fromrounded-lgtorounded-xlaligns with the PR objectives.As per coding guidelines, the use of
cnwith an array split by logical grouping is appropriate.apps/desktop/src/components/settings/ai/llm/select.tsx (1)
82-82: LGTM! Styling update aligns with PR objectives.The border radius update from
rounded-lgtorounded-xlis consistent with the broader UI refresh across the codebase.apps/desktop/src/components/settings/template/shared.tsx (1)
39-47: LGTM! Consistent styling updates.The changes improve visual consistency:
- Added
overflow-clipfor proper content clipping- Updated border radii from
rounded-lgtorounded-xl- Simplified badge corner styling to
rounded-bl-xlAll align with the PR's UI refresh objectives.
apps/desktop/src/routes/app/settings/_layout.tsx (3)
142-142: LGTM! Main content container styling updated.Border radius updated from
rounded-lgtorounded-xl, consistent with the UI refresh across the application.
174-179: LGTM! Group wrapper styling improved.The updated styling adds vertical padding (
py-1) and increases border radius torounded-xl, aligning with the broader design refresh.
186-201: LGTM! Tab rendering restructured correctly.The refactoring addresses the previous review feedback and improves the layout:
- The
keyprop is now correctly placed on the outermost element (<div>) returned by themap()callback- Added horizontal padding wrapper for better spacing
- Button layout changed from
justify-betweentojustify-startfor cleaner alignment- Icon and label now properly contained within the Button
The previous concern about the missing key prop has been resolved.
apps/desktop/src/components/main/sidebar/profile/shared.tsx (2)
19-24: LGTM! Excellent use ofcnutility.The className composition follows the coding guidelines perfectly by using
cnwith arrays split by logical grouping (layout, spacing, colors, transitions, etc.).As per coding guidelines,
cnis correctly used with arrays split by logical grouping.Also applies to: 34-39, 48-48
27-50: Verify layout when both badge and SuffixIcon are present.The button uses
justify-betweenwith potentially three direct children: the left group (icon + label), badge, and SuffixIcon. When all three are present,justify-betweenwill space them as: start - middle - end. This means the badge won't be grouped with the SuffixIcon on the right side.If badge and SuffixIcon can coexist, consider wrapping them in a right-side container:
- {badge && - (typeof badge === "number" ? ( - <span - className={cn( - "rounded-full", - "px-2 py-0.5", - "bg-red-500", - "text-xs font-semibold text-white", - )} - > - {badge} - </span> - ) : ( - badge - ))} - {SuffixIcon && ( - <SuffixIcon - className={cn("h-4 w-4 flex-shrink-0", "text-neutral-400")} - /> - )} + <div className="flex items-center gap-2"> + {badge && + (typeof badge === "number" ? ( + <span + className={cn( + "rounded-full", + "px-2 py-0.5", + "bg-red-500", + "text-xs font-semibold text-white", + )} + > + {badge} + </span> + ) : ( + badge + ))} + {SuffixIcon && ( + <SuffixIcon + className={cn("h-4 w-4 flex-shrink-0", "text-neutral-400")} + /> + )} + </div>If they are mutually exclusive in practice, the current implementation is fine.
apps/desktop/src/components/main/body/shared.tsx (4)
82-98: Excellent refactoring following coding guidelines!The className refactoring correctly uses
cnwith array format and clear logical grouping, which aligns perfectly with the project's coding guidelines. The explicit four-case conditional logic improves readability and maintainability.Minor: Verify border inconsistency in active non-selected state.
Line 89 uses
border-0(removes border entirely) for theactive && !selectedstate, while other states use explicit border classes (border-red-400,border-stone-400,border-transparent). Usingborder-0vsborder-transparentcan affect layout/spacing differently. Verify this is intentional for the design.As per coding guidelines, the use of
cnwith arrays and logical grouping is exactly what's required for complex conditional classNames.
103-106: Clean refactoring with proper array usage.The className refactoring follows the coding guidelines correctly, using
cnwith an array and clear logical grouping (base styles, then conditional opacity).
118-121: Consistent refactoring pattern.The className refactoring mirrors the first overlay pattern correctly, using
cnwith an array. The inverse opacity logic (showing on hover) is appropriate for the close button overlay.
128-137: Well-structured conditional text color logic.The className refactoring correctly uses
cnwith an array and clear three-case conditional logic. Each state appropriately includes both base and hover colors, maintaining visual consistency with the tab's active/selected states.
26d8d5f to
941baa6
Compare
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: 1
♻️ Duplicate comments (1)
apps/desktop/src/components/main/sidebar/profile/auth.tsx (1)
140-147: Inconsistent padding and icon spacing across auth states.The unauthenticated wrapper now uses
className="p-1 pt-2"(line 140) while authenticated (line 35) and pending (line 90) states usepx-1 py-2. Additionally, theLogInicon (line 146) usessize={16}without margin, while theLogOuticon (line 41) still usesclassName="w-4 h-4 mr-2".For visual consistency:
- Standardize padding across all three auth states (either use
px-1 py-2everywhere or adjust all top-1 pt-2)- Align icon spacing: either add spacing to
LogInor removemr-2fromLogOutand rely on the Button's internal gapBased on learnings, this was previously flagged and remains unresolved.
🧹 Nitpick comments (1)
apps/desktop/src/routes/app/settings/_layout.tsx (1)
186-201: Tab row structure and key usage are solidWrapping each tab in
<div key={tab} className="px-1">correctly satisfies React’s key requirements and matches the earlier lint feedback, and theButtonusescnwith grouped base/active/hover classes in a clear way. Visually this should produce full-width, left-aligned tab buttons consistent with the new sidebar style.If you later revisit accessibility, consider adding an ARIA hint for the active tab (e.g.,
aria-current="page"oraria-selected) on the activeButtonto improve screen-reader navigation, but this can be deferred.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/ui/src/components/ui/button.tsxis excluded by!packages/ui/src/components/ui/**
📒 Files selected for processing (20)
apps/desktop/src/components/chat/body/index.tsx(1 hunks)apps/desktop/src/components/chat/header.tsx(1 hunks)apps/desktop/src/components/main/body/index.tsx(1 hunks)apps/desktop/src/components/main/body/search.tsx(1 hunks)apps/desktop/src/components/main/body/shared.tsx(3 hunks)apps/desktop/src/components/main/sidebar/banner/component.tsx(1 hunks)apps/desktop/src/components/main/sidebar/index.tsx(1 hunks)apps/desktop/src/components/main/sidebar/profile/auth.tsx(1 hunks)apps/desktop/src/components/main/sidebar/profile/index.tsx(5 hunks)apps/desktop/src/components/main/sidebar/profile/shared.tsx(1 hunks)apps/desktop/src/components/settings/ai/llm/configure.tsx(2 hunks)apps/desktop/src/components/settings/ai/llm/select.tsx(1 hunks)apps/desktop/src/components/settings/ai/stt/configure.tsx(2 hunks)apps/desktop/src/components/settings/ai/stt/select.tsx(1 hunks)apps/desktop/src/components/settings/template/editor.tsx(1 hunks)apps/desktop/src/components/settings/template/index.tsx(1 hunks)apps/desktop/src/components/settings/template/local.tsx(1 hunks)apps/desktop/src/components/settings/template/sections.tsx(1 hunks)apps/desktop/src/components/settings/template/shared.tsx(1 hunks)apps/desktop/src/routes/app/settings/_layout.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (10)
- apps/desktop/src/components/settings/template/editor.tsx
- apps/desktop/src/components/settings/template/sections.tsx
- apps/desktop/src/components/chat/header.tsx
- apps/desktop/src/components/settings/template/local.tsx
- apps/desktop/src/components/main/sidebar/profile/index.tsx
- apps/desktop/src/components/settings/ai/llm/select.tsx
- apps/desktop/src/components/main/body/index.tsx
- apps/desktop/src/components/settings/template/index.tsx
- apps/desktop/src/components/main/body/shared.tsx
- apps/desktop/src/components/main/sidebar/profile/shared.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
If there are many classNames with conditional logic, usecn(import from@hypr/utils). It is similar toclsx. Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/desktop/src/components/main/body/search.tsxapps/desktop/src/components/settings/ai/stt/select.tsxapps/desktop/src/components/settings/ai/stt/configure.tsxapps/desktop/src/components/main/sidebar/banner/component.tsxapps/desktop/src/components/main/sidebar/index.tsxapps/desktop/src/components/settings/template/shared.tsxapps/desktop/src/routes/app/settings/_layout.tsxapps/desktop/src/components/settings/ai/llm/configure.tsxapps/desktop/src/components/main/sidebar/profile/auth.tsxapps/desktop/src/components/chat/body/index.tsx
🧠 Learnings (2)
📚 Learning: 2025-11-24T16:32:19.706Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:32:19.706Z
Learning: Applies to **/*.{ts,tsx} : If there are many classNames with conditional logic, use `cn` (import from `hypr/utils`), similar to `clsx`. Always pass an array and split by logical grouping.
Applied to files:
apps/desktop/src/components/settings/ai/stt/select.tsxapps/desktop/src/components/settings/ai/llm/configure.tsxapps/desktop/src/components/chat/body/index.tsx
📚 Learning: 2025-11-24T16:32:23.055Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:32:23.055Z
Learning: Applies to **/*.{ts,tsx} : If there are many classNames with conditional logic, use `cn` (import from `hypr/utils`). It is similar to `clsx`. Always pass an array and split by logical grouping.
Applied to files:
apps/desktop/src/components/settings/ai/stt/select.tsxapps/desktop/src/components/settings/ai/stt/configure.tsxapps/desktop/src/components/settings/ai/llm/configure.tsxapps/desktop/src/components/chat/body/index.tsx
🧬 Code graph analysis (1)
apps/desktop/src/routes/app/settings/_layout.tsx (1)
packages/utils/src/cn.ts (1)
cn(20-22)
⏰ 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). (8)
- GitHub Check: ci (macos, macos-14)
- GitHub Check: ci (linux, depot-ubuntu-24.04-8)
- GitHub Check: ci (linux, depot-ubuntu-22.04-8)
- GitHub Check: ci (macos, depot-macos-14)
- GitHub Check: fmt
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
🔇 Additional comments (13)
apps/desktop/src/components/main/body/search.tsx (1)
146-146: LGTM! Border radius update is consistent with the design system refresh.The change from
rounded-lgtorounded-xlaligns with the broader UI update across the desktop app. Thecnusage properly groups classes by logical concern.apps/desktop/src/components/settings/ai/stt/select.tsx (1)
76-76: LGTM! Consistent styling update.The border radius update from
rounded-lgtorounded-xlmaintains consistency with the design refresh throughout the app.apps/desktop/src/components/settings/template/shared.tsx (1)
39-47: LGTM! Styling refinements align with the design update.The additions are appropriate:
overflow-clip(line 39) ensures the "Made by me" badge renders cleanly- Border radius updates to
rounded-xl(line 40) and simplified badge cornerrounded-bl-xl(line 47) are consistent with the broader UI refreshapps/desktop/src/components/chat/body/index.tsx (1)
41-41: LGTM! Border radius update with margin adjustment.The change updates the border radius from
rounded-mdtorounded-xland removesmt-1, aligning with the design system refresh whenRightPanelOpenmode is active.apps/desktop/src/components/settings/ai/stt/configure.tsx (2)
94-94: LGTM! Good simplification and consistent styling.Replacing
cn([...])with a static className string is appropriate here since there's no conditional logic. The border radius update torounded-xlaligns with the design system refresh.
167-167: LGTM! Consistent with styling updates.Same appropriate simplification and
rounded-xlupdate as in theNonHyprProviderCard.apps/desktop/src/components/settings/ai/llm/configure.tsx (2)
89-89: LGTM! Appropriate simplification with consistent styling.Removing the
cn()wrapper for static classes is cleaner when there's no conditional logic. Therounded-xlupdate maintains consistency with the design refresh.
168-168: LGTM! Consistent styling update.Same appropriate simplification and border radius update as in
NonHyprProviderCard.apps/desktop/src/components/main/sidebar/banner/component.tsx (1)
53-58: LGTM! Enhanced button styling with gradient.The updated className adds
rounded-fulland a gradient background, elevating the primary action button's visual prominence.apps/desktop/src/components/main/sidebar/index.tsx (1)
27-30: Header styling update is consistent and usescncorrectlyThe updated header classes (flex layout, sizing,
shrink-0, androunded-xl bg-neutral-50) look consistent with the rest of the PR’s rounded design direction, and usingcnwith an array keeps the grouping clear and maintainable.apps/desktop/src/routes/app/settings/_layout.tsx (3)
142-145: Main content container radius/border change looks goodThe settings content wrapper’s updated
p-6 border border-neutral-200 rounded-xlkeeps the scroll behavior and layout intact while matching the new rounded visual language.
174-179: Group wrapper styling matches design andcnguidelinesUsing
cnwith["rounded-xl bg-neutral-50 py-1", expandHeight && "flex-1"]cleanly separates base styling from conditional height and aligns with the rounded container pattern used elsewhere in the PR.
232-233: Header radius updates are consistent with sidebar groupsBoth
TopLevelHeaderandInnerHeadernow usebg-neutral-50 rounded-xl, keeping the tauri drag-region intact while visually aligning the headers with the rounded Group containers in the sidebar. No behavioral changes, just cohesive styling.Also applies to: 272-273
• Updated button and UI component styling with rounded corners
• Adjusted sidebar profile button layout
• Temporarily disabled notifications header
• Updated banner buttons with rounded styles and gradients
• Improved sign-in button styling
• Simplified conditional class generation logic
This is part 1 of 2 in a stack made with GitButler: