-
Notifications
You must be signed in to change notification settings - Fork 415
Enhance UI: video icons, share modal, Tailwind colors, rounded modals #1578
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
📝 WalkthroughWalkthroughMigrated many UI color tokens to a neutral palette, added Webex support and image icons in session listen UI, introduced a Popover-based ShareButton, added a Separator component, adjusted dropdown/popover rounding, and updated several sidebar/profile/header UI pieces and one MenuItem prop signature. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Page as SessionHeader
participant UI as ShareButton (Popover)
participant Search as PeopleSearch
participant API as Invite/Copy (stub)
User->>Page: click Share icon
Page->>UI: open popover
UI-->>User: shows search input + results
User->>Search: type query
Search->>UI: update searchResults
User->>UI: select person(s)
UI->>UI: add to selectedPeople
User->>UI: click Invite
UI->>API: (stub) send invites / add invitedPeople
API-->>UI: success (stub)
UI-->>User: show invited list / clear selection
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Mix of repetitive styling updates (low per-file complexity) and several heterogeneous functional changes: new ShareButton, Webex branch and assets, MenuItem API change, and new Separator component. 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)
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/ui/src/components/block/event-chip.tsx (1)
260-268: Inconsistent color token migration.Line 267 still uses
placeholder:text-color3while all other text elements in this file have been migrated to neutral tokens (e.g.,text-neutral-700,text-neutral-500). This creates inconsistency in the color palette migration.Apply this diff to complete the migration:
placeholder="Search past events..." value={searchQuery} onChange={(e) => onSearchChange?.(e.target.value)} - className="w-full bg-transparent text-sm focus:outline-none placeholder:text-color3" + className="w-full bg-transparent text-sm focus:outline-none placeholder:text-neutral-500" />packages/ui/src/components/block/meeting-metadata-chip.tsx (2)
300-317: Inconsistent color token migration.Line 316 still uses
placeholder:text-color3while all other text elements have been migrated to neutral tokens. This creates inconsistency in the color palette migration.Apply this diff to complete the migration:
placeholder="Add participant" - className="w-full bg-transparent text-sm focus:outline-none placeholder:text-color3" + className="w-full bg-transparent text-sm focus:outline-none placeholder:text-neutral-500" />
319-329: Fix className syntax error.Line 325 has a typo:
"text-neutral-700 neutral-700transition-colors"is missing a space betweenneutral-700andtransition-colors.Apply this diff to fix the typo:
{searchQuery.trim() && ( <button onClick={() => { if (selectedIndex >= 0 && selectedIndex < searchResults.length) { handleSelectParticipant(searchResults[selectedIndex].id); } }} - className="text-neutral-700 neutral-700transition-colors flex-shrink-0" + className="text-neutral-700 transition-colors flex-shrink-0" > <CornerDownLeft className="size-4" /> </button>apps/desktop2/src/components/main/body/sessions/floating/listen.tsx (1)
129-129: Remove debug console.log statement.This debug logging should be removed before merging.
- console.log(note);
🧹 Nitpick comments (2)
apps/desktop2/src/components/main/body/sessions/outer-header/share.tsx (2)
50-73: TODOs marked for invite and copy link functionality.The mock data and placeholder implementations are appropriate for initial development. The core state management logic appears sound.
Would you like me to help implement the actual invite functionality and copy-to-clipboard logic, or open an issue to track these tasks?
138-248: Well-structured invited people management with role controls.The implementation correctly handles avatar hover interactions and role selection. The repeated avatar-with-hover-remove pattern (lines 146-164 and 199-217) could optionally be extracted into a reusable component, but the current approach is acceptable given the limited scope.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
apps/desktop2/public/assets/hyprnote-pro.pngis excluded by!**/*.pngapps/desktop2/public/assets/meet.pngis excluded by!**/*.pngapps/desktop2/public/assets/webex.pngis excluded by!**/*.pngapps/desktop2/public/assets/zoom.pngis excluded by!**/*.pngpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (18)
apps/desktop2/src/components/main/body/index.tsx(1 hunks)apps/desktop2/src/components/main/body/sessions/floating/listen.tsx(3 hunks)apps/desktop2/src/components/main/body/sessions/floating/shared.tsx(2 hunks)apps/desktop2/src/components/main/body/sessions/outer-header/folder.tsx(3 hunks)apps/desktop2/src/components/main/body/sessions/outer-header/other.tsx(1 hunks)apps/desktop2/src/components/main/body/sessions/outer-header/share.tsx(1 hunks)apps/desktop2/src/components/main/body/shared.tsx(2 hunks)apps/desktop2/src/components/main/sidebar/index.tsx(1 hunks)apps/desktop2/src/components/main/sidebar/profile/banner.tsx(2 hunks)apps/desktop2/src/components/main/sidebar/profile/index.tsx(4 hunks)apps/desktop2/src/components/main/sidebar/search/index.tsx(1 hunks)apps/desktop2/tailwind.config.ts(0 hunks)packages/ui/src/components/block/event-chip.tsx(6 hunks)packages/ui/src/components/block/meeting-metadata-chip.tsx(12 hunks)packages/ui/src/components/block/participants-chip.tsx(9 hunks)packages/ui/src/components/ui/dropdown-menu.tsx(2 hunks)packages/ui/src/components/ui/popover.tsx(1 hunks)packages/ui/src/components/ui/separator.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- apps/desktop2/tailwind.config.ts
🧰 Additional context used
📓 Path-based instructions (2)
apps/desktop2/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (apps/desktop2/.cursor/rules/style.mdc)
apps/desktop2/**/*.{tsx,jsx}: When there are many Tailwind classNames with conditional logic, use the cn utility imported asimport { cn } from "@hypr/ui/lib/utils"
Always pass an array to cn when composing Tailwind classNames
Split cn array entries by logical grouping when composing Tailwind classNames
Files:
apps/desktop2/src/components/main/sidebar/search/index.tsxapps/desktop2/src/components/main/body/index.tsxapps/desktop2/src/components/main/body/sessions/outer-header/folder.tsxapps/desktop2/src/components/main/body/shared.tsxapps/desktop2/src/components/main/body/sessions/floating/shared.tsxapps/desktop2/src/components/main/body/sessions/floating/listen.tsxapps/desktop2/src/components/main/sidebar/profile/banner.tsxapps/desktop2/src/components/main/sidebar/profile/index.tsxapps/desktop2/src/components/main/body/sessions/outer-header/other.tsxapps/desktop2/src/components/main/body/sessions/outer-header/share.tsxapps/desktop2/src/components/main/sidebar/index.tsx
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit configuration file
**/*.{js,ts,tsx,rs}: 1. Do not add any error handling. Keep the existing one.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
Files:
apps/desktop2/src/components/main/sidebar/search/index.tsxapps/desktop2/src/components/main/body/index.tsxpackages/ui/src/components/block/event-chip.tsxapps/desktop2/src/components/main/body/sessions/outer-header/folder.tsxpackages/ui/src/components/block/participants-chip.tsxapps/desktop2/src/components/main/body/shared.tsxapps/desktop2/src/components/main/body/sessions/floating/shared.tsxpackages/ui/src/components/block/meeting-metadata-chip.tsxapps/desktop2/src/components/main/body/sessions/floating/listen.tsxapps/desktop2/src/components/main/sidebar/profile/banner.tsxapps/desktop2/src/components/main/sidebar/profile/index.tsxapps/desktop2/src/components/main/body/sessions/outer-header/other.tsxpackages/ui/src/components/ui/separator.tsxpackages/ui/src/components/ui/popover.tsxapps/desktop2/src/components/main/body/sessions/outer-header/share.tsxapps/desktop2/src/components/main/sidebar/index.tsxpackages/ui/src/components/ui/dropdown-menu.tsx
🧬 Code graph analysis (7)
apps/desktop2/src/components/main/body/sessions/outer-header/folder.tsx (2)
packages/ui/src/components/ui/breadcrumb.tsx (1)
BreadcrumbList(112-112)packages/ui/src/components/ui/dropdown-menu.tsx (1)
DropdownMenuTrigger(194-194)
packages/ui/src/components/block/participants-chip.tsx (1)
packages/ui/src/components/ui/avatar.tsx (1)
AvatarFallback(58-58)
packages/ui/src/components/block/meeting-metadata-chip.tsx (2)
apps/desktop2/src/components/main/body/contacts/shared.tsx (1)
getInitials(5-15)packages/ui/src/components/icons/linkedin.tsx (1)
LinkedInIcon(7-23)
apps/desktop2/src/components/main/body/sessions/floating/listen.tsx (1)
apps/desktop2/src/components/main/body/sessions/floating/shared.tsx (1)
FloatingButton(5-29)
apps/desktop2/src/components/main/sidebar/profile/banner.tsx (2)
packages/ui/src/lib/utils.ts (1)
cn(4-6)packages/ui/src/components/ui/button.tsx (1)
Button(38-69)
packages/ui/src/components/ui/separator.tsx (1)
packages/ui/src/lib/utils.ts (1)
cn(4-6)
apps/desktop2/src/components/main/body/sessions/outer-header/share.tsx (1)
apps/desktop2/src/components/main/body/contacts/shared.tsx (1)
getInitials(5-15)
🪛 GitHub Actions: .github/workflows/fmt.yaml
apps/desktop2/src/components/main/body/sessions/outer-header/share.tsx
[error] 1-236: dprint check failed. Found 1 not formatted file. Run '~/.dprint/bin/dprint fmt' or 'dprint --config dprint.json' to format the file. Command: Run ~/.dprint/bin/dprint check --config 'dprint.json'
⏰ 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). (1)
- GitHub Check: ci (macos, macos-14)
🔇 Additional comments (43)
packages/ui/src/components/ui/popover.tsx (1)
63-63: LGTM! Border radius update aligns with UI consistency pass.The change from
rounded-mdtorounded-lgis consistent with the broader styling updates across UI components (e.g., dropdown-menu.tsx). This improves visual consistency across popovers and dropdowns.packages/ui/src/components/block/event-chip.tsx (5)
123-128: LGTM! Icon and text colors updated to neutral palette.The migration from legacy color tokens to
text-neutral-700improves consistency with the broader UI theming updates.
214-214: LGTM! Date text color updated consistently.
239-239: LGTM! Note text color migrated to neutral palette.
273-275: LGTM! Empty state text color updated.
288-290: LGTM! Search result date text color migrated.packages/ui/src/components/ui/separator.tsx (1)
1-31: LGTM! Well-structured Separator component.The new Separator component follows established patterns for wrapping Radix UI primitives. The implementation is clean with appropriate defaults (
orientation="horizontal",decorative=true) and responsive sizing based on orientation.packages/ui/src/components/block/participants-chip.tsx (6)
85-87: LGTM! Chip header colors migrated to neutral palette.The icon, text, and count label colors have been consistently updated to neutral tokens.
144-146: LGTM! Organization name styling updated.
202-204: LGTM! Participant item colors migrated consistently.Avatar fallback, participant name, and job title colors all use appropriate neutral tokens.
Also applies to: 220-223
232-245: LGTM! Action link colors updated with proper hover states.The email and LinkedIn links now use
text-neutral-500withhover:text-neutral-700, providing clear visual feedback.
275-291: LGTM! Search input styling migrated to neutral palette.All colors including the placeholder text have been consistently updated to neutral tokens.
304-318: LGTM! Search results and create button colors updated.packages/ui/src/components/block/meeting-metadata-chip.tsx (10)
124-137: LGTM! Button text color updated to neutral palette.
149-153: LGTM! Location icon and text colors migrated.
162-165: LGTM! Meeting link colors updated consistently.
180-182: LGTM! Date range text color migrated.
200-202: LGTM! Description text color updated.
281-281: LGTM! Participants section label color updated.
350-352: LGTM! Search result organization name color updated.
358-360: LGTM! Empty state text color migrated.
390-400: LGTM! ParticipantChip container and content colors updated.The chip styling now uses consistent neutral palette colors with appropriate background and text colors.
402-422: LGTM! Action buttons migrated to neutral colors with proper hover states.Email and LinkedIn link colors use
text-neutral-500withhover:text-neutral-700transitions.packages/ui/src/components/ui/dropdown-menu.tsx (1)
47-47: LGTM! Border radius updates align with UI consistency pass.The changes from
rounded-mdtorounded-lgin bothDropdownMenuSubContentandDropdownMenuContentare consistent with the broader styling updates across UI components (e.g., popover.tsx). This improves visual consistency across dropdown menus and popovers.Also applies to: 64-64
apps/desktop2/src/components/main/body/index.tsx (1)
145-145: LGTM! Color token migration to neutral palette.The update from
text-color3totext-neutral-500aligns with the broader theme migration across the codebase.apps/desktop2/src/components/main/body/shared.tsx (2)
62-64: LGTM! Neutral color palette migration.The color token updates from legacy tokens (bg-color1, text-color3/4) to neutral tokens (bg-neutral-50, text-neutral-500/700) are consistent with the PR's theming objectives.
75-80: LGTM! Close button color updates align with neutral palette.The active/inactive state colors now use neutral-700/neutral-500 instead of the legacy color tokens, maintaining clear visual hierarchy.
apps/desktop2/src/components/main/sidebar/profile/banner.tsx (1)
39-47: LGTM! Hover-revealed dismiss button pattern.The ghost button with opacity transition on hover provides a clean, unobtrusive dismiss interaction.
apps/desktop2/src/components/main/sidebar/index.tsx (1)
26-26: LGTM! Sidebar header color migration.The background color update to
bg-neutral-50is consistent with the neutral palette migration across the application.apps/desktop2/src/components/main/body/sessions/outer-header/folder.tsx (1)
36-36: LGTM! Breadcrumb and input color token migration.All color updates (text-color3/4 → text-neutral-500/700) are consistent with the neutral palette migration throughout the codebase.
Also applies to: 109-109, 146-146
apps/desktop2/src/components/main/body/sessions/outer-header/other.tsx (1)
108-112: LGTM! Proper dropdown item interaction pattern.Using
onSelect={(e) => e.preventDefault()}prevents the dropdown from closing when clicking the menu item, while the Switch'sonCheckedChangehandler still controls the lock toggle. This is the correct pattern for dropdown items containing interactive controls.apps/desktop2/src/components/main/body/sessions/floating/shared.tsx (3)
20-20: LGTM! Simplified button styling.Replacing the
cn-based className with a static"shadow-lg"string is cleaner for this single-class case.
25-25: LGTM! Direct icon rendering with fallback.The updated logic renders the provided icon directly, or displays a pulsing red indicator when no icon is provided. This is clearer than wrapping the icon in a span.
1-3: Remove unused cn import.The
cnutility is imported on line 3 but is no longer used after the className was changed to a static string on line 20.Apply this diff:
import { Button } from "@hypr/ui/components/ui/button"; - import { type ReactNode } from "react";As per coding guidelines: "No unused imports, variables, or functions."
⛔ Skipped due to learnings
Learnt from: CR PR: fastrepl/hyprnote#0 File: apps/desktop2/.cursor/rules/style.mdc:0-0 Timestamp: 2025-10-12T11:14:50.462Z Learning: Applies to apps/desktop2/**/*.{tsx,jsx} : When there are many Tailwind classNames with conditional logic, use the cn utility imported as `import { cn } from "hypr/ui/lib/utils"`apps/desktop2/src/components/main/sidebar/search/index.tsx (1)
13-13: LGTM! Color migration and simplified className.The change from
cn(["h-full rounded-md bg-color1"])to the static string"h-full rounded-md bg-neutral-50"is appropriate since there's no conditional logic here. The color token migration aligns with the broader neutral palette adoption.apps/desktop2/src/components/main/body/sessions/floating/listen.tsx (3)
13-16: LGTM! Webex variant follows the existing pattern.The type addition is consistent with the existing zoom and google-meet variants.
89-95: LGTM! Default icon behavior is correctly utilized.The removal of the icon prop allows the FloatingButton to display its default pulsing red dot indicator, which is appropriate for the "Start listening" state.
56-87: Asset files verified—implementation is correct.All required PNG files exist in
apps/desktop2/public/assets/(zoom.png, meet.png, webex.png). The implementation correctly follows the established pattern for all three meeting platforms.apps/desktop2/src/components/main/body/sessions/outer-header/share.tsx (2)
1-22: LGTM! Clean imports and well-defined types.The imports and Person interface are appropriately structured for the share modal functionality.
92-136: LGTM! Search UI correctly uses neutral color tokens.The search input and results rendering properly implement the Tailwind neutral color migration mentioned in the PR objectives.
apps/desktop2/src/components/main/sidebar/profile/index.tsx (3)
1-2: LGTM! Import is correctly used.The windowsCommands import is properly utilized at line 81 for opening the settings window.
124-192: LGTM! Color migration to neutral tokens is correct.The migration from
bg-color1tobg-neutral-50aligns with the PR's objective to switch to Tailwind colors. The mix of neutral and slate tokens is appropriate for semantic color usage.
196-215: LGTM! Avatar visibility and class simplifications are correct.The removal of
hidden sm:flexmakes the avatar always visible, and the conversion of static classes from clsx to plain strings is appropriate since they don't involve conditional logic.
apps/desktop2/src/components/main/body/sessions/outer-header/share.tsx
Outdated
Show resolved
Hide resolved
81a0857 to
bf55e85
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
🧹 Nitpick comments (3)
apps/desktop2/src/components/main/sidebar/search/index.tsx (1)
13-13: Consider usingcnwith array syntax for consistency.While this static className doesn't require
cn, the rest of the file consistently usescn([...])for all className declarations (lines 23-47). Maintaining a uniform pattern improves readability.Apply this diff to align with the file's existing style:
- <div className="h-full rounded-md bg-neutral-50"> + <div className={cn(["h-full rounded-md bg-neutral-50"])}>apps/desktop2/src/components/main/sidebar/profile/notification.tsx (1)
1-8: Consider standardizing oncnutility throughout this file.The file imports both
cn(line 6) andclsx(line 3), usingcnat lines 93, 101, and 111, butclsxat line 123. Standardizing oncnthroughout would improve consistency and align with the coding guidelines for desktop2 files.Apply this diff to use
cnconsistently:-import { clsx } from "clsx"; -import { ArrowLeft, ArrowRight, Bell, CheckCheck, MessageSquare } from "lucide-react"; +import { ArrowLeft, ArrowRight, Bell, CheckCheck, MessageSquare } from "lucide-react"; import { cn } from "@hypr/ui/lib/utils";Then update line 123:
<p - className={clsx( + className={cn([ "text-sm font-medium text-black truncate", !notification.read && "font-semibold", - )} + ])} >apps/desktop2/src/components/main/sidebar/profile/ota.tsx (1)
256-256: Consider clarifying the intent of the empty onClick handler.The empty function
onClick={() => { }}appears to disable MenuItem interaction while preserving the internal Retry button behavior. Consider adding a brief comment explaining why this is a no-op, or use a more explicit approach.Example:
- onClick={() => { }} + onClick={() => { }} // Prevent MenuItem click; only Retry button should be interactive
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
apps/desktop2/public/assets/hyprnote-pro.pngis excluded by!**/*.pngpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (19)
apps/desktop2/src/components/main/body/index.tsx(1 hunks)apps/desktop2/src/components/main/body/sessions/outer-header/folder.tsx(3 hunks)apps/desktop2/src/components/main/body/sessions/outer-header/other.tsx(1 hunks)apps/desktop2/src/components/main/body/sessions/outer-header/share.tsx(1 hunks)apps/desktop2/src/components/main/body/shared.tsx(2 hunks)apps/desktop2/src/components/main/sidebar/index.tsx(1 hunks)apps/desktop2/src/components/main/sidebar/profile/banner.tsx(3 hunks)apps/desktop2/src/components/main/sidebar/profile/index.tsx(6 hunks)apps/desktop2/src/components/main/sidebar/profile/notification.tsx(6 hunks)apps/desktop2/src/components/main/sidebar/profile/ota.tsx(8 hunks)apps/desktop2/src/components/main/sidebar/profile/shared.tsx(3 hunks)apps/desktop2/src/components/main/sidebar/search/index.tsx(1 hunks)apps/desktop2/tailwind.config.ts(0 hunks)packages/ui/src/components/block/event-chip.tsx(5 hunks)packages/ui/src/components/block/meeting-metadata-chip.tsx(13 hunks)packages/ui/src/components/block/participants-chip.tsx(9 hunks)packages/ui/src/components/ui/dropdown-menu.tsx(2 hunks)packages/ui/src/components/ui/popover.tsx(1 hunks)packages/ui/src/components/ui/separator.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- apps/desktop2/tailwind.config.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- packages/ui/src/components/block/event-chip.tsx
- packages/ui/src/components/block/participants-chip.tsx
- apps/desktop2/src/components/main/sidebar/profile/banner.tsx
- packages/ui/src/components/block/meeting-metadata-chip.tsx
- apps/desktop2/src/components/main/body/shared.tsx
- apps/desktop2/src/components/main/body/index.tsx
- apps/desktop2/src/components/main/sidebar/profile/index.tsx
- apps/desktop2/src/components/main/sidebar/index.tsx
🧰 Additional context used
📓 Path-based instructions (2)
apps/desktop2/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (apps/desktop2/.cursor/rules/style.mdc)
apps/desktop2/**/*.{tsx,jsx}: When there are many Tailwind classNames with conditional logic, use the cn utility imported asimport { cn } from "@hypr/ui/lib/utils"
Always pass an array to cn when composing Tailwind classNames
Split cn array entries by logical grouping when composing Tailwind classNames
Files:
apps/desktop2/src/components/main/body/sessions/outer-header/share.tsxapps/desktop2/src/components/main/sidebar/profile/notification.tsxapps/desktop2/src/components/main/sidebar/search/index.tsxapps/desktop2/src/components/main/body/sessions/outer-header/other.tsxapps/desktop2/src/components/main/sidebar/profile/shared.tsxapps/desktop2/src/components/main/sidebar/profile/ota.tsxapps/desktop2/src/components/main/body/sessions/outer-header/folder.tsx
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit configuration file
**/*.{js,ts,tsx,rs}: 1. Do not add any error handling. Keep the existing one.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
Files:
apps/desktop2/src/components/main/body/sessions/outer-header/share.tsxpackages/ui/src/components/ui/popover.tsxapps/desktop2/src/components/main/sidebar/profile/notification.tsxapps/desktop2/src/components/main/sidebar/search/index.tsxapps/desktop2/src/components/main/body/sessions/outer-header/other.tsxapps/desktop2/src/components/main/sidebar/profile/shared.tsxapps/desktop2/src/components/main/sidebar/profile/ota.tsxapps/desktop2/src/components/main/body/sessions/outer-header/folder.tsxpackages/ui/src/components/ui/separator.tsxpackages/ui/src/components/ui/dropdown-menu.tsx
🧠 Learnings (1)
📚 Learning: 2025-10-12T11:14:50.462Z
Learnt from: CR
PR: fastrepl/hyprnote#0
File: apps/desktop2/.cursor/rules/style.mdc:0-0
Timestamp: 2025-10-12T11:14:50.462Z
Learning: Applies to apps/desktop2/**/*.{tsx,jsx} : When there are many Tailwind classNames with conditional logic, use the cn utility imported as `import { cn } from "hypr/ui/lib/utils"`
Applied to files:
apps/desktop2/src/components/main/sidebar/profile/notification.tsx
🧬 Code graph analysis (4)
apps/desktop2/src/components/main/body/sessions/outer-header/share.tsx (6)
packages/ui/src/components/ui/popover.tsx (3)
Popover(85-85)PopoverTrigger(85-85)PopoverContent(85-85)packages/ui/src/components/ui/button.tsx (1)
Button(38-69)packages/ui/src/components/ui/avatar.tsx (2)
Avatar(58-58)AvatarFallback(58-58)apps/desktop2/src/components/main/body/contacts/shared.tsx (1)
getInitials(5-15)packages/ui/src/components/ui/separator.tsx (1)
Separator(31-31)packages/ui/src/components/ui/select.tsx (5)
Select(174-174)SelectTrigger(174-174)SelectValue(174-174)SelectContent(174-174)SelectItem(174-174)
apps/desktop2/src/components/main/sidebar/profile/notification.tsx (1)
packages/ui/src/components/ui/button.tsx (1)
Button(38-69)
apps/desktop2/src/components/main/sidebar/profile/ota.tsx (1)
packages/ui/src/components/ui/spinner.tsx (1)
Spinner(58-58)
packages/ui/src/components/ui/separator.tsx (1)
packages/ui/src/lib/utils.ts (1)
cn(4-6)
🪛 GitHub Actions: .github/workflows/fmt.yaml
apps/desktop2/src/components/main/sidebar/profile/ota.tsx
[error] 1-1: dprint formatting check failed. 3 files not formatted.
🔇 Additional comments (12)
apps/desktop2/src/components/main/body/sessions/outer-header/folder.tsx (3)
36-36: LGTM: Clean migration to Tailwind neutral palette.The color token migration from
text-color4totext-neutral-700aligns with the PR's objective to standardize on Tailwind colors.
109-109: LGTM: Hover state colors migrated correctly.The migration from
text-color3/text-color4totext-neutral-500/text-neutral-700maintains the lighter-to-darker hover pattern appropriately.
146-146: LGTM: Input text color migrated correctly.The color migration to
text-neutral-700is consistent with the broader palette standardization. Thecnutility is not needed here since there's no conditional logic in the className.packages/ui/src/components/ui/separator.tsx (1)
1-31: LGTM! Clean Separator component implementation.The component is well-structured with proper ref forwarding, type safety, and conditional styling. The defaults (
orientation="horizontal",decorative=true) are sensible choices, and the use ofcnfor className merging follows best practices.packages/ui/src/components/ui/popover.tsx (1)
63-63: LGTM! Consistent border radius update.The change from
rounded-mdtorounded-lgaligns with the same update indropdown-menu.tsx, ensuring visual consistency across popover-style surfaces throughout the UI.apps/desktop2/src/components/main/body/sessions/outer-header/other.tsx (1)
108-112: LGTM! Correct interaction pattern for inline Switch.The
onSelectwithe.preventDefault()correctly prevents the dropdown from closing when interacting with the Switch, whileonCheckedChangeon the Switch itself handles the toggle logic. This pattern ensures the dropdown stays open for the inline control.apps/desktop2/src/components/main/body/sessions/outer-header/share.tsx (1)
25-254: LGTM! Well-structured ShareButton component.The implementation demonstrates solid interaction patterns:
- Proper event delegation with
stopPropagationprevents unintended popover closure (lines 149, 202, 227)- Disabled state logic prevents duplicate selections (lines 106-109)
- Case-insensitive search enhances UX (line 37)
- State management clearly separates selected vs. invited people
The TODO placeholders (lines 57, 72) appropriately mark where real backend integration is needed.
packages/ui/src/components/ui/dropdown-menu.tsx (1)
47-47: LGTM! Consistent border radius update.Both
DropdownMenuSubContentandDropdownMenuContentnow userounded-lg, matching the popover styling changes for visual consistency across dropdown and popover surfaces.Also applies to: 64-64
apps/desktop2/src/components/main/sidebar/profile/notification.tsx (1)
43-54: LGTM! Clean refactor to Button component.The back navigation now uses the Button component with appropriate size and variant props, improving consistency with the design system while maintaining the same functionality.
apps/desktop2/src/components/main/sidebar/profile/shared.tsx (1)
4-11: LGTM! Clean extension with optional suffixIcon prop.The new
suffixIconprop is properly typed as optional and rendered with appropriate sizing and neutral coloring. The implementation is straightforward and maintains consistency with the existing icon pattern.Also applies to: 40-40
apps/desktop2/src/components/main/sidebar/profile/ota.tsx (2)
1-1: LGTM! Clean component migration.The import changes correctly replace
Loader2with the newSpinnercomponent, and the old import is properly removed.Also applies to: 8-8
210-210: LGTM! Consistent color palette migration.The migration from
slatetoneutralcolor tokens is applied consistently across all UI states, and theSpinnercomponent is properly integrated with appropriate color variants for different states (neutral, blue, green).Also applies to: 213-213, 225-225, 274-274, 276-276, 285-285, 290-290, 293-293, 320-320, 323-323
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop2/src/components/main/sidebar/profile/banner.tsx (1)
12-12: Critical: Empty signup handler prevents banner's primary action.The
handleSignUpfunction is empty, so clicking "Start 1 week Free Trial" does nothing. Either implement the signup flow or accept anonSignUpcallback prop similar toonDismiss.Apply this diff to accept a callback prop:
-export function TryProBanner({ isDismissed, onDismiss }: { isDismissed: boolean; onDismiss: () => void }) { +export function TryProBanner({ isDismissed, onDismiss, onSignUp }: { isDismissed: boolean; onDismiss: () => void; onSignUp: () => void }) {Then update the handler:
- const handleSignUp = () => {}; + const handleSignUp = () => { + onSignUp(); + };
♻️ Duplicate comments (1)
apps/desktop2/src/components/main/sidebar/profile/ota.tsx (1)
7-7: Replace clsx with cn utility per project guidelines.This file still uses
clsxinstead of the requiredcnutility from@hypr/ui/lib/utils. Allclsx()calls should be replaced withcn([...])using array syntax with logically grouped entries.Based on coding guidelines.
🧹 Nitpick comments (3)
apps/desktop2/src/components/main/body/sessions/outer-header/share.tsx (3)
45-53: TODO: Implement the actual invite functionality.The invite handler currently only manages local state. Real implementation should call an API to grant session access to the invited users.
Do you want me to generate the API integration code or open a new issue to track this task?
65-68: TODO: Implement the copy link functionality.The copy link handler is stubbed. Real implementation should generate a shareable link and copy it to the clipboard.
Do you want me to generate the clipboard copy implementation or open a new issue to track this task?
120-158: Consider extracting a shared PersonListItem component.The selected people list (lines 120-158) and invited people list (lines 168-224) share significant rendering logic—both display avatars with hover-reveal remove buttons, name, and email. Extracting a reusable
PersonListItemcomponent would reduce duplication and improve maintainability.The component could accept props like:
interface PersonListItemProps { person: Person; onRemove: (id: string) => void; actionSlot?: React.ReactNode; // For the role selector in invited list }Also applies to: 168-224
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/desktop2/src/components/main/body/sessions/outer-header/share.tsx(1 hunks)apps/desktop2/src/components/main/sidebar/profile/banner.tsx(2 hunks)apps/desktop2/src/components/main/sidebar/profile/ota.tsx(6 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
apps/desktop2/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (apps/desktop2/.cursor/rules/style.mdc)
apps/desktop2/**/*.{tsx,jsx}: When there are many Tailwind classNames with conditional logic, use the cn utility imported asimport { cn } from "@hypr/ui/lib/utils"
Always pass an array to cn when composing Tailwind classNames
Split cn array entries by logical grouping when composing Tailwind classNames
Files:
apps/desktop2/src/components/main/sidebar/profile/ota.tsxapps/desktop2/src/components/main/sidebar/profile/banner.tsxapps/desktop2/src/components/main/body/sessions/outer-header/share.tsx
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit configuration file
**/*.{js,ts,tsx,rs}: 1. Do not add any error handling. Keep the existing one.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
Files:
apps/desktop2/src/components/main/sidebar/profile/ota.tsxapps/desktop2/src/components/main/sidebar/profile/banner.tsxapps/desktop2/src/components/main/body/sessions/outer-header/share.tsx
🧠 Learnings (3)
📚 Learning: 2025-10-12T11:14:50.462Z
Learnt from: CR
PR: fastrepl/hyprnote#0
File: apps/desktop2/.cursor/rules/style.mdc:0-0
Timestamp: 2025-10-12T11:14:50.462Z
Learning: Applies to apps/desktop2/**/*.{tsx,jsx} : When there are many Tailwind classNames with conditional logic, use the cn utility imported as `import { cn } from "hypr/ui/lib/utils"`
Applied to files:
apps/desktop2/src/components/main/sidebar/profile/ota.tsx
📚 Learning: 2025-10-12T11:14:50.462Z
Learnt from: CR
PR: fastrepl/hyprnote#0
File: apps/desktop2/.cursor/rules/style.mdc:0-0
Timestamp: 2025-10-12T11:14:50.462Z
Learning: Applies to apps/desktop2/**/*.{tsx,jsx} : Split cn array entries by logical grouping when composing Tailwind classNames
Applied to files:
apps/desktop2/src/components/main/sidebar/profile/ota.tsx
📚 Learning: 2025-10-12T11:14:50.462Z
Learnt from: CR
PR: fastrepl/hyprnote#0
File: apps/desktop2/.cursor/rules/style.mdc:0-0
Timestamp: 2025-10-12T11:14:50.462Z
Learning: Applies to apps/desktop2/**/*.{tsx,jsx} : Always pass an array to cn when composing Tailwind classNames
Applied to files:
apps/desktop2/src/components/main/sidebar/profile/ota.tsx
🧬 Code graph analysis (3)
apps/desktop2/src/components/main/sidebar/profile/ota.tsx (1)
packages/ui/src/components/ui/spinner.tsx (1)
Spinner(58-58)
apps/desktop2/src/components/main/sidebar/profile/banner.tsx (2)
packages/ui/src/lib/utils.ts (1)
cn(4-6)packages/ui/src/components/ui/button.tsx (1)
Button(38-69)
apps/desktop2/src/components/main/body/sessions/outer-header/share.tsx (2)
packages/ui/src/components/ui/button.tsx (1)
Button(38-69)apps/desktop2/src/components/main/body/contacts/shared.tsx (1)
getInitials(5-15)
⏰ 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). (1)
- GitHub Check: ci (macos, macos-14)
🔇 Additional comments (4)
apps/desktop2/src/components/main/sidebar/profile/ota.tsx (1)
1-1: LGTM: Spinner integration and neutral color tokens.The Spinner component is correctly integrated with appropriate sizing and semantic colors for each state (neutral for checking, blue for downloading, green for installing). The migration from slate-* to neutral-* color tokens is consistent throughout the component.
Also applies to: 210-210, 213-213, 225-225, 274-276, 285-285, 290-290, 293-293, 320-320, 323-323
apps/desktop2/src/components/main/sidebar/profile/banner.tsx (1)
50-50: Image asset verified and accessible.The
/assets/hyprnote-pro.pngfile exists atapps/desktop2/public/assets/hyprnote-pro.pngand is correctly referenced in the code.apps/desktop2/src/components/main/body/sessions/outer-header/share.tsx (2)
1-17: LGTM! Clean imports and type definition.The imports are well-organized and the Person interface is properly typed with the role union type.
22-32: Verify that real data integration is planned.The component initializes with hard-coded mock data for
invitedPeopleandsearchResults. Ensure that the actual implementation will fetch real session participants and integrate with your user search API.
| setInvitedPeople([...invitedPeople, ...newInvites]); | ||
| setSelectedPeople([]); | ||
| // TODO: Implement actual invite functionality | ||
| console.log("Invite:", newInvites); |
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.
🛠️ Refactor suggestion | 🟠 Major
Remove console.log statements before merging.
Debug console.log statements should be removed from production code.
Apply this diff:
setInvitedPeople([...invitedPeople, ...newInvites]);
setSelectedPeople([]);
- // TODO: Implement actual invite functionality
- console.log("Invite:", newInvites);
+ // TODO: Implement actual invite functionality
}; const handleCopyLink = () => {
- // TODO: Implement copy link functionality
- console.log("Copy link");
+ // TODO: Implement copy link functionality
};Also applies to: 67-67
🤖 Prompt for AI Agents
In apps/desktop2/src/components/main/body/sessions/outer-header/share.tsx around
lines 52 and 67, there are debug console.log statements (e.g.,
console.log("Invite:", newInvites)) that must be removed before merging; delete
these console.log calls (or replace them with a proper logger conditional on dev
mode if runtime debug is required), ensure no other stray console.* remains in
the file, and run a quick build/test to confirm no references remain.
• Added video chat icons
• Applied icons to listen CTA button
• Updated profile banner
• Implemented share modal
• Used Tailwind colors
• Added separator
• Styled modals with rounded corners
• Prevented toggle from closing