Conversation
Implement custom key handling to prevent default browser shortcuts like Cmd/Ctrl+W, Cmd/Ctrl+N, and Cmd/Ctrl+T from interfering with the editor's functionality. This ensures a more consistent editing experience by blocking these common browser navigation shortcuts.
Implement new hotkeys for creating and closing tabs with mod+n and mod+t. Add middle-click tab closing and update tab styling for better usability. Enhance hotkey behavior to work with form and content editable elements.
📝 WalkthroughWalkthroughThis PR introduces a new SessionMetadata component with meeting participant management, adds hotkey handlers for tab creation, reorganizes the session UI with breadcrumb-based folder navigation, introduces MeetingMetadataChip for event metadata display, updates styling color tokens across components, and expands the notification handler's app ignore list for mic-detection events. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Editor as Editor/Window
participant Hotkey as Hotkey Handler
participant Store as Session Store
participant Tab as Tab Manager
User->>Editor: Press Cmd/Ctrl+N or Mod+T
Editor->>Hotkey: Keydown event
Hotkey->>Hotkey: Check if Cmd/Ctrl+N or Mod/Ctrl+T
alt Is target hotkey
Hotkey->>Store: Store session metadata
Store-->>Hotkey: Metadata stored
Hotkey->>Tab: Create new "sessions" tab
Tab->>Tab: Open with editor: "raw"
Note over Tab: Optional: Close current tab on N
else Not target
Hotkey->>Editor: Allow default behavior
end
sequenceDiagram
participant Component as SessionMetadata
participant Store as Persisted Store
participant Hooks as useMemo/useQuery
participant UI as MeetingMetadataChip
Component->>Store: Read session/events/humans/orgs
Store-->>Component: Return data
Component->>Hooks: Compute meeting metadata
Hooks-->>Component: Memoized metadata
Component->>Component: Build participant list with enrichment
Component-->>UI: Pass metadata, participants, handlers
UI->>UI: Render meeting details
Note over UI: User can search, add, remove participants
UI->>Component: onParticipantAdd/Remove/Search callback
Component->>Store: Update mappings/state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Multiple heterogeneous changes spanning UI reorganization, new component introduction (SessionMetadata, MeetingMetadataChip), hotkey handling, breadcrumb-based navigation refactoring, color token updates, and notification plugin expansion. While individual file changes are often straightforward (styling or component wiring), the cumulative scope, density of new React components with store integration, and cross-cutting UI restructuring require careful review across different domains. Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✨ 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.
Actionable comments posted: 9
🧹 Nitpick comments (10)
plugins/notification/src/handler.rs (1)
103-110: App ignore list expanded appropriately.The additions correctly skip mic-detection notifications for development tools (VS Code, Zed, Windsurf, Raycast), AI assistants (OpenAI Chat, Claude Desktop), and Voice Memos—all of which legitimately use the microphone for their core functionality.
Optional: Consider externalizing this growing list (now 19 apps) to a configuration file or constant module for easier maintenance and updates.
// Example: Extract to a constants module const IGNORED_APP_IDS: &[&str] = &[ "com.electron.wispr-flow", "com.seewillow.WillowMac", // ... rest of the list ];apps/desktop2/src/components/main/body/sessions/note-input/raw.tsx (1)
68-80: Consider using semantic color tokens.The hard-coded hex color
#e5e5e5works but doesn't scale well with theming. If a semantic token exists for this shade (e.g.,text-gray-200or a custom token), prefer that for maintainability.apps/desktop2/src/components/main/body/shared.tsx (2)
59-64: Use cn from @hypr/ui and drop clsx.apps/desktop2 should use the shared cn utility (array form). Replace clsx calls and import.
-import { clsx } from "clsx"; +import { cn } from "@hypr/ui/lib/utils"; ... - className={clsx([ + className={cn([ "flex items-center gap-2 cursor-pointer group", "w-48 h-full pl-2 pr-1", "bg-color1 rounded-lg border", active ? "text-black border-black" : "text-color3 border-transparent", ])} ... - <Button + <Button onClick={(e) => { e.stopPropagation(); handleCloseThis(); }} - className={clsx([ + className={cn([ "flex-shrink-0 transition-opacity", active ? "opacity-100 text-color4" : "opacity-0 group-hover:opacity-100 text-color3", ])}As per coding guidelines.
Also applies to: 75-83
70-86: Add an accessible label to the close button.Provide aria-label for screen readers.
- <Button + <Button + aria-label="Close tab" onClick={(e) => { e.stopPropagation(); handleCloseThis(); }}packages/ui/src/components/block/meeting-metadata-chip.tsx (2)
283-286: Remove “what” comments; keep comments for “why”.Comments like “Existing Participants Chips”, “Search Input”, and “Search Results Dropdown” narrate the UI. Remove or replace with brief “why” notes if needed.
As per coding guidelines.
Also applies to: 298-301, 332-364
70-83: Unused prop candidates.MeetingMetadataChipProps includes isVeryNarrow, isNarrow, and formatRelativeDate but they’re unused. Remove or wire them to avoid dead API surface.
apps/desktop2/src/components/main/body/sessions/outer-header/other.tsx (3)
59-63: Use functional state updates for toggles.Avoid stale closures in rapid interactions.
- const handleToggleListening = () => { - setIsListening(!isListening); + const handleToggleListening = () => { + setIsListening((v) => !v); // TODO: Implement start/stop listening functionality console.log("Toggle listening"); };- const handleToggleLock = () => { - setIsLocked(!isLocked); + // If you keep a lock handler elsewhere, prefer: + const handleToggleLock = () => { + setIsLocked((v) => !v); // TODO: Implement lock note functionality console.log("Toggle lock"); };Also applies to: 48-52
43-46: Pass the selected folder id to the handler.Current handler lacks context; selection can’t act on a specific folder.
- const handleMoveToFolder = () => { + const handleMoveToFolder = (folderId: string) => { // TODO: Implement move to folder functionality - console.log("Move to folder"); + console.log("Move to folder", folderId); };- {folders.map((folder) => ( - <DropdownMenuItem key={folder.id} onClick={handleMoveToFolder}> + {folders.map((folder) => ( + <DropdownMenuItem key={folder.id} onClick={() => handleMoveToFolder(folder.id)}> <FolderIcon /> <span>{folder.name}</span> </DropdownMenuItem> ))}If the UI library supports onSelect for menu items, prefer onSelect to avoid accidental text selection. Please confirm with the component docs.
Also applies to: 99-105
38-41: Remove console logs before merging.Keep the UI silent or route to your telemetry; logs add noise.
Also applies to: 44-46, 54-57, 60-63, 66-68, 71-73, 76-78
apps/desktop2/src/components/main/body/sessions/outer-header/metadata.tsx (1)
42-84: Optional: narrow memo deps to reduce recomputes.Depending on the eventRow object may recompute more than needed. Track scalar fields instead.
- }, [sessionRow?.event_id, eventRow, store, participantMappingIds]); + }, [ + sessionRow?.event_id, + eventRow?.title, + eventRow?.started_at, + eventRow?.ended_at, + store, + participantMappingIds, + ]);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (20)
apps/desktop2/src/components/interactive-button.tsx(4 hunks)apps/desktop2/src/components/main/body/index.tsx(7 hunks)apps/desktop2/src/components/main/body/sessions/index.tsx(1 hunks)apps/desktop2/src/components/main/body/sessions/note-input/index.tsx(1 hunks)apps/desktop2/src/components/main/body/sessions/note-input/raw.tsx(1 hunks)apps/desktop2/src/components/main/body/sessions/outer-header/folder.tsx(4 hunks)apps/desktop2/src/components/main/body/sessions/outer-header/index.tsx(2 hunks)apps/desktop2/src/components/main/body/sessions/outer-header/metadata.tsx(1 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(3 hunks)apps/desktop2/src/components/main/sidebar/index.tsx(2 hunks)apps/desktop2/src/components/main/sidebar/timeline/index.tsx(2 hunks)packages/tiptap/src/editor/index.tsx(2 hunks)packages/ui/src/components/block/event-chip.tsx(6 hunks)packages/ui/src/components/block/meeting-metadata-chip.tsx(1 hunks)packages/ui/src/components/block/participants-chip.tsx(10 hunks)packages/ui/src/components/icons/linkedin.tsx(1 hunks)packages/ui/src/components/ui/button.tsx(3 hunks)plugins/notification/src/handler.rs(1 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/interactive-button.tsxapps/desktop2/src/components/main/body/sessions/outer-header/metadata.tsxapps/desktop2/src/components/main/sidebar/index.tsxapps/desktop2/src/components/main/body/sessions/index.tsxapps/desktop2/src/components/main/body/shared.tsxapps/desktop2/src/components/main/body/index.tsxapps/desktop2/src/components/main/body/sessions/note-input/index.tsxapps/desktop2/src/components/main/sidebar/timeline/index.tsxapps/desktop2/src/components/main/body/sessions/outer-header/folder.tsxapps/desktop2/src/components/main/body/sessions/outer-header/index.tsxapps/desktop2/src/components/main/body/sessions/note-input/raw.tsxapps/desktop2/src/components/main/body/sessions/outer-header/share.tsxapps/desktop2/src/components/main/body/sessions/outer-header/other.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/interactive-button.tsxpackages/tiptap/src/editor/index.tsxapps/desktop2/src/components/main/body/sessions/outer-header/metadata.tsxapps/desktop2/src/components/main/sidebar/index.tsxpackages/ui/src/components/ui/button.tsxapps/desktop2/src/components/main/body/sessions/index.tsxpackages/ui/src/components/icons/linkedin.tsxpackages/ui/src/components/block/participants-chip.tsxapps/desktop2/src/components/main/body/shared.tsxapps/desktop2/src/components/main/body/index.tsxapps/desktop2/src/components/main/body/sessions/note-input/index.tsxapps/desktop2/src/components/main/sidebar/timeline/index.tsxapps/desktop2/src/components/main/body/sessions/outer-header/folder.tsxapps/desktop2/src/components/main/body/sessions/outer-header/index.tsxpackages/ui/src/components/block/meeting-metadata-chip.tsxapps/desktop2/src/components/main/body/sessions/note-input/raw.tsxpackages/ui/src/components/block/event-chip.tsxapps/desktop2/src/components/main/body/sessions/outer-header/share.tsxapps/desktop2/src/components/main/body/sessions/outer-header/other.tsxplugins/notification/src/handler.rs
🧬 Code graph analysis (14)
apps/desktop2/src/components/main/body/sessions/outer-header/metadata.tsx (3)
apps/desktop2/src/store/zustand/tabs/index.ts (1)
useTabs(15-20)packages/ui/src/components/block/meeting-metadata-chip.tsx (3)
MeetingMetadata(47-56)MeetingParticipant(58-68)MeetingMetadataChip(85-209)apps/desktop2/src/hooks/useQuery.ts (1)
useQuery(3-46)
apps/desktop2/src/components/main/sidebar/index.tsx (1)
packages/ui/src/components/ui/button.tsx (1)
Button(38-69)
packages/ui/src/components/ui/button.tsx (1)
packages/ui/src/components/ui/spinner.tsx (1)
Spinner(58-58)
apps/desktop2/src/components/main/body/sessions/index.tsx (4)
apps/desktop2/src/components/main/body/sessions/outer-header/index.tsx (1)
OuterHeader(8-25)apps/desktop2/src/components/main/body/sessions/title-input.tsx (1)
TitleInput(6-32)apps/desktop2/src/components/main/body/sessions/note-input/index.tsx (1)
NoteInput(12-38)apps/desktop2/src/components/main/body/sessions/floating/index.tsx (1)
FloatingActionButton(8-18)
packages/ui/src/components/block/participants-chip.tsx (1)
packages/ui/src/components/icons/linkedin.tsx (1)
LinkedInIcon(7-23)
apps/desktop2/src/components/main/body/shared.tsx (1)
packages/ui/src/components/ui/button.tsx (1)
Button(38-69)
apps/desktop2/src/components/main/body/index.tsx (4)
packages/ui/src/components/ui/button.tsx (1)
Button(38-69)packages/ui/src/lib/utils.ts (1)
cn(4-6)apps/desktop2/src/components/main/body/search.tsx (1)
Search(8-107)apps/desktop2/src/store/zustand/tabs/index.ts (1)
useTabs(15-20)
apps/desktop2/src/components/main/body/sessions/note-input/index.tsx (1)
packages/ui/src/lib/utils.ts (1)
cn(4-6)
apps/desktop2/src/components/main/body/sessions/outer-header/folder.tsx (2)
packages/ui/src/components/ui/breadcrumb.tsx (5)
Breadcrumb(108-108)BreadcrumbSeparator(114-114)BreadcrumbItem(110-110)BreadcrumbLink(111-111)BreadcrumbPage(113-113)packages/ui/src/components/ui/dropdown-menu.tsx (4)
DropdownMenu(180-180)DropdownMenuTrigger(194-194)DropdownMenuContent(182-182)DropdownMenuItem(184-184)
apps/desktop2/src/components/main/body/sessions/outer-header/index.tsx (1)
apps/desktop2/src/components/main/body/sessions/outer-header/metadata.tsx (1)
SessionMetadata(14-223)
packages/ui/src/components/block/meeting-metadata-chip.tsx (5)
packages/ui/src/components/ui/button.tsx (1)
Button(38-69)packages/ui/src/lib/utils.ts (1)
cn(4-6)apps/desktop2/src/components/main/body/contacts/shared.tsx (1)
getInitials(5-15)packages/ui/src/components/ui/avatar.tsx (2)
Avatar(58-58)AvatarFallback(58-58)packages/ui/src/components/icons/linkedin.tsx (1)
LinkedInIcon(7-23)
apps/desktop2/src/components/main/body/sessions/note-input/raw.tsx (1)
packages/ui/src/lib/utils.ts (1)
cn(4-6)
apps/desktop2/src/components/main/body/sessions/outer-header/share.tsx (1)
packages/ui/src/components/ui/button.tsx (1)
Button(38-69)
apps/desktop2/src/components/main/body/sessions/outer-header/other.tsx (4)
packages/db/src/schema.ts (1)
folders(30-34)packages/ui/src/components/ui/dropdown-menu.tsx (8)
DropdownMenu(180-180)DropdownMenuTrigger(194-194)DropdownMenuContent(182-182)DropdownMenuItem(184-184)DropdownMenuSub(191-191)DropdownMenuSubTrigger(193-193)DropdownMenuSubContent(192-192)DropdownMenuSeparator(189-189)packages/ui/src/components/ui/button.tsx (1)
Button(38-69)packages/ui/src/components/ui/switch.tsx (1)
Switch(57-57)
⏰ 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). (2)
- GitHub Check: zizmor
- GitHub Check: ci (macos, macos-14)
🔇 Additional comments (22)
packages/ui/src/components/block/event-chip.tsx (2)
110-110: LGTM! Consistent spacing and color token migration.The spacing reduction (gap-2 → gap-1) and color token updates (text-color4) align with the PR's design system standardization. These visual changes improve consistency without affecting functionality.
Also applies to: 123-128
214-214: Color tokens are properly defined and migration is valid.The verification confirms that
text-color3(#8e8e8e) andtext-color4(#484848) are defined in the Tailwind configuration for the packages/ui workspace. The color distinction is semantically appropriate:color3(lighter) for placeholder text andcolor4(darker) for content text maintains proper visual hierarchy across all six updated locations.apps/desktop2/src/components/main/sidebar/timeline/index.tsx (1)
93-93: Verify the red shadow color is intentional.The shadow uses red color (
rgba(255,0,0,0.1)) while all other colors in the button are neutral grays. Was this red tint intentional for visual emphasis, or should it use a neutral gray shadow to match the overall color scheme?apps/desktop2/src/components/main/body/sessions/outer-header/share.tsx (1)
1-9: LGTM! Clean Button component migration.The replacement of the native button with the UI library Button component is well-executed and aligns with the broader UI refactor across the PR.
apps/desktop2/src/components/main/body/sessions/note-input/index.tsx (1)
56-72: LGTM! Clean tab bar styling refactor.The removal of the border container and adjustment to button styling simplifies the layout while maintaining functionality. Proper use of
cnwith array syntax follows the coding guidelines.packages/ui/src/components/ui/button.tsx (2)
17-17: Verify the gap reduction doesn't cause visual issues.Changing
gap-2togap-1tightens the spacing between the loading spinner and button text. Ensure this doesn't make buttons with loading states feel cramped, especially forsize="sm".
4-4: LGTM! Clean Spinner component extraction.Replacing the inline SVG with the reusable Spinner component improves maintainability.
Also applies to: 64-64
apps/desktop2/src/components/main/sidebar/index.tsx (1)
29-33: LGTM! Consistent Button component adoption.The change aligns with the PR-wide pattern of wrapping icon actions in Button components for consistent styling and interaction behavior.
packages/ui/src/components/icons/linkedin.tsx (1)
1-25: LGTM! Well-structured icon component.The LinkedInIcon follows React best practices with proper forwardRef usage, TypeScript typing, and a configurable size prop. The displayName assignment aids debugging.
apps/desktop2/src/components/main/body/index.tsx (4)
52-63: LGTM! Session creation with proper metadata.The
handleNewNotefunction correctly initializes the session withuser_id,created_at, andtitlefields before opening the new tab.
68-97: LGTM! Consistent Button component adoption.The migration of navigation and sidebar controls to Button components improves UI consistency. Icon sizes are standardized to 16px.
137-151: LGTM! Clean layout refactor with drag region.The new drag-region wrapper properly contains the new note button and search component, maintaining the draggable window area.
306-306: LGTM! Hotkey consistency improvement.Adding
enableOnContentEditable: trueensures hotkeys work consistently across all editor contexts, including the Tiptap editor.Also applies to: 331-331, 361-361
apps/desktop2/src/components/interactive-button.tsx (1)
8-8: LGTM! Clean onMouseDown prop addition.The new
onMouseDownhandler is properly typed, forwarded to the underlying element in both code paths, and doesn't interfere with existing click behavior. This enables additional interaction patterns like middle-click handling.Also applies to: 19-19, 45-52, 59-66
apps/desktop2/src/components/main/body/shared.tsx (1)
37-43: Middle‑click close behavior: LGTM.Clean handling of mouse button 1 with preventDefault + stopPropagation to close the tab without selecting it.
apps/desktop2/src/components/main/body/sessions/outer-header/folder.tsx (2)
63-72: Breadcrumb wiring looks correct.Good use of renderSeparator with BreadcrumbSeparator and clickable crumbs via BreadcrumbLink asChild.
107-119: Fallback when no folders exist: LGTM.Clear UX with disabled “No folders available” and a dedicated trigger.
packages/ui/src/components/block/meeting-metadata-chip.tsx (2)
416-421: Good: external link is safe.Using target="_blank" with rel="noopener noreferrer" is correct.
120-138: Popover trigger event handlers: double-check interactions.stopPropagation on both click and mousedown shouldn’t block the PopoverTrigger (asChild) from toggling, but please verify open/close behavior via keyboard/mouse.
apps/desktop2/src/components/main/body/sessions/index.tsx (2)
42-47: Content reflow: LGTM.Moving OuterHeader above Title/Note with tighter spacing looks consistent with the updated header system.
41-48: The positioning context is already established—no changes needed.StandardTabWrapper's inner div already includes the
relativeclass in its className (line 282 ofindex.tsx), which serves as the positioned ancestor for FloatingActionButton's absolute positioning. The nested div with "mt-3 px-2" doesn't requirerelativebecause the FAB correctly resolves its position relative to StandardTabWrapper's positioned ancestor above it. The code is working as intended.Likely an incorrect or invalid review comment.
apps/desktop2/src/components/main/body/sessions/outer-header/index.tsx (1)
16-19: LGTM: composition swap to SessionMetadata is clean.Props are wired correctly; no extraneous imports or props churn.
| const useNewTabHotkeys = () => { | ||
| const { persistedStore, internalStore } = useRouteContext({ from: "__root__" }); | ||
| const { currentTab, close, openNew } = useTabs(); | ||
|
|
||
| useHotkeys( | ||
| ["mod+n", "mod+t"], | ||
| (e) => { | ||
| e.preventDefault(); | ||
|
|
||
| const sessionId = id(); | ||
| const user_id = internalStore?.getValue("user_id"); | ||
|
|
||
| persistedStore?.setRow("sessions", sessionId, { user_id, created_at: new Date().toISOString() }); | ||
|
|
||
| if (e.key === "n" && currentTab) { | ||
| close(currentTab); | ||
| } | ||
|
|
||
| openNew({ | ||
| type: "sessions", | ||
| id: sessionId, | ||
| active: true, | ||
| state: { editor: "raw" }, | ||
| }); | ||
| }, | ||
| { enableOnFormTags: true, enableOnContentEditable: true }, | ||
| [persistedStore, internalStore, currentTab, close, openNew], | ||
| ); | ||
| }; |
There was a problem hiding this comment.
Fix inconsistent session initialization.
Line 348 creates a session row without the title field, but handleNewNote (line 56) includes title: "". This inconsistency could cause issues if the schema expects the title field.
Apply this diff to align with handleNewNote:
- persistedStore?.setRow("sessions", sessionId, { user_id, created_at: new Date().toISOString() });
+ persistedStore?.setRow("sessions", sessionId, { user_id, created_at: new Date().toISOString(), title: "" });🤖 Prompt for AI Agents
In apps/desktop2/src/components/main/body/index.tsx around lines 336 to 364, the
session row written in the hotkey handler is missing the title field (created at
line ~348) while handleNewNote (line 56) sets title: ""; update the
persistedStore?.setRow call to include title: "" alongside user_id and
created_at so the session initialization matches handleNewNote and the schema
expectation.
| const sessionRow = persisted.UI.useRow("sessions", sessionId, persisted.STORE_ID); | ||
|
|
||
| const eventRow = persisted.UI.useRow( | ||
| "events", | ||
| sessionRow.event_id || "dummy-event-id", | ||
| persisted.STORE_ID, | ||
| ); |
There was a problem hiding this comment.
Guard against undefined session row.
Direct property access on sessionRow can throw. Use optional chaining; keep the dummy id pattern to preserve hook order.
- const sessionRow = persisted.UI.useRow("sessions", sessionId, persisted.STORE_ID);
+ const sessionRow = persisted.UI.useRow("sessions", sessionId, persisted.STORE_ID);
- const eventRow = persisted.UI.useRow(
+ const eventRow = persisted.UI.useRow(
"events",
- sessionRow.event_id || "dummy-event-id",
+ sessionRow?.event_id ?? "dummy-event-id",
persisted.STORE_ID,
);- if (!sessionRow.event_id || !eventRow || !eventRow.started_at || !eventRow.ended_at) {
+ if (!sessionRow?.event_id || !eventRow || !eventRow.started_at || !eventRow.ended_at) {
return null;
}- id: sessionRow.event_id,
+ id: sessionRow!.event_id,- }, [sessionRow.event_id, eventRow, store, participantMappingIds]);
+ }, [sessionRow?.event_id, eventRow, store, participantMappingIds]);Also applies to: 42-45, 74-76, 84-84
🤖 Prompt for AI Agents
In apps/desktop2/src/components/main/body/sessions/outer-header/metadata.tsx
around lines 28-34 (and similarly at 42-45, 74-76, 84), the code directly
accesses properties on sessionRow which may be undefined and can throw; update
those hook calls to use optional chaining for safety while preserving the
dummy-id pattern and hook ordering (e.g., change uses of sessionRow.event_id /
sessionRow.user_id / sessionRow.someProp to sessionRow?.event_id ||
"dummy-event-id" etc.), ensuring every persisted.UI.useRow call still receives a
string id even when sessionRow is undefined.
| if ( | ||
| name && !name.toLowerCase().includes(normalizedQuery) | ||
| && (!email || !email.toLowerCase().includes(normalizedQuery)) | ||
| ) { | ||
| return; | ||
| } | ||
|
|
There was a problem hiding this comment.
Fix participant search filter (currently includes non-matches).
Rows with no name are always included even if email doesn’t match. Require a positive match on at least one field.
- if (
- name && !name.toLowerCase().includes(normalizedQuery)
- && (!email || !email.toLowerCase().includes(normalizedQuery))
- ) {
- return;
- }
+ const matches =
+ (name && name.toLowerCase().includes(normalizedQuery)) ||
+ (email && email.toLowerCase().includes(normalizedQuery)) ||
+ (job_title && job_title.toLowerCase().includes(normalizedQuery)) ||
+ (linkedin_username && linkedin_username.toLowerCase().includes(normalizedQuery));
+ if (!matches) return;🤖 Prompt for AI Agents
In apps/desktop2/src/components/main/body/sessions/outer-header/metadata.tsx
around lines 135-141, the current filter lets rows through when name is missing
even if email doesn't match; change the logic to require at least one positive
match: compute nameMatches = name?.toLowerCase().includes(normalizedQuery) and
emailMatches = email?.toLowerCase().includes(normalizedQuery) (safely handling
undefined), then return early only when both are false (if (!nameMatches &&
!emailMatches) return;). Ensure to normalize before checking and avoid
short-circuiting that treats missing name as an automatic match.
| const handleToggleLock = () => { | ||
| setIsLocked(!isLocked); | ||
| // TODO: Implement lock note functionality | ||
| console.log("Toggle lock"); | ||
| }; |
There was a problem hiding this comment.
Fix double toggle and keep menu open for “Lock note”.
Clicking the Switch can fire both the item onClick and the Switch handler, toggling twice and closing the menu. Bind the state only to the Switch and prevent the item from closing the menu.
Apply:
- const handleToggleLock = () => {
- setIsLocked(!isLocked);
- // TODO: Implement lock note functionality
- console.log("Toggle lock");
- };
+ // Lock toggled via Switch in-menu to avoid double-trigger.- <DropdownMenuItem onClick={handleToggleLock}>
+ <DropdownMenuItem onSelect={(e) => e.preventDefault()}>
<LockIcon />
<span>Lock note</span>
- <Switch checked={isLocked} onCheckedChange={handleToggleLock} className="ml-auto" />
+ <Switch checked={isLocked} onCheckedChange={setIsLocked} className="ml-auto" />
</DropdownMenuItem>Also applies to: 108-112
| className={clsx([ | ||
| "group", | ||
| "relative", | ||
| "absolute left-1/2 transform -translate-x-1/2", | ||
| "bg-white hover:bg-gray-50", | ||
| "border border-gray-200", | ||
| "rounded-full shadow-lg", | ||
| "text-gray-700 z-40", | ||
| "rounded-full bg-white hover:bg-gray-50", | ||
| "text-gray-700 border border-gray-200", | ||
| "z-40 flex items-center gap-1", | ||
| "shadow-[inset_0_-4px_6px_-1px_rgba(255,0,0,0.1),inset_0_-2px_4px_-2px_rgba(255,0,0,0.1)]", | ||
| isScrolledPastToday ? "top-2" : "bottom-2", | ||
| isScrolledPastToday | ||
| ? "shadow-[0_-2px_8px_rgba(248,113,113,0.3)] hover:shadow-[0_-2px_12px_rgba(239,68,68,0.4)]" | ||
| : "shadow-[0_2px_8px_rgba(248,113,113,0.3)] hover:shadow-[0_2px_12px_rgba(239,68,68,0.4)]", | ||
| ])} |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use cn instead of clsx for consistency.
The file uses cn for className composition elsewhere (lines 44, 55), but this Button uses clsx. Per the coding guidelines, when composing many Tailwind classNames, use the cn utility imported from @hypr/ui/lib/utils.
As per coding guidelines
Apply this diff:
- className={clsx([
+ className={cn([
"absolute left-1/2 transform -translate-x-1/2",
"rounded-full bg-white hover:bg-gray-50",
"text-gray-700 border border-gray-200",
"z-40 flex items-center gap-1",
"shadow-[inset_0_-4px_6px_-1px_rgba(255,0,0,0.1),inset_0_-2px_4px_-2px_rgba(255,0,0,0.1)]",
isScrolledPastToday ? "top-2" : "bottom-2",
])}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| className={clsx([ | |
| "group", | |
| "relative", | |
| "absolute left-1/2 transform -translate-x-1/2", | |
| "bg-white hover:bg-gray-50", | |
| "border border-gray-200", | |
| "rounded-full shadow-lg", | |
| "text-gray-700 z-40", | |
| "rounded-full bg-white hover:bg-gray-50", | |
| "text-gray-700 border border-gray-200", | |
| "z-40 flex items-center gap-1", | |
| "shadow-[inset_0_-4px_6px_-1px_rgba(255,0,0,0.1),inset_0_-2px_4px_-2px_rgba(255,0,0,0.1)]", | |
| isScrolledPastToday ? "top-2" : "bottom-2", | |
| isScrolledPastToday | |
| ? "shadow-[0_-2px_8px_rgba(248,113,113,0.3)] hover:shadow-[0_-2px_12px_rgba(239,68,68,0.4)]" | |
| : "shadow-[0_2px_8px_rgba(248,113,113,0.3)] hover:shadow-[0_2px_12px_rgba(239,68,68,0.4)]", | |
| ])} | |
| className={cn([ | |
| "absolute left-1/2 transform -translate-x-1/2", | |
| "rounded-full bg-white hover:bg-gray-50", | |
| "text-gray-700 border border-gray-200", | |
| "z-40 flex items-center gap-1", | |
| "shadow-[inset_0_-4px_6px_-1px_rgba(255,0,0,0.1),inset_0_-2px_4px_-2px_rgba(255,0,0,0.1)]", | |
| isScrolledPastToday ? "top-2" : "bottom-2", | |
| ])} |
🤖 Prompt for AI Agents
In apps/desktop2/src/components/main/sidebar/timeline/index.tsx around lines 88
to 95, the Button's className is composed with clsx but the project standard
uses the cn utility; replace clsx([...]) with cn([...]) for consistency, ensure
cn is imported from "@hypr/ui/lib/utils" at the top of the file, and remove the
clsx import if it becomes unused.
| handleKeyDown: (_, event) => { | ||
| if ((event.metaKey || event.ctrlKey) && (event.key === "w" || event.key === "n" || event.key === "t")) { | ||
| return false; | ||
| } | ||
| return false; | ||
| }, |
There was a problem hiding this comment.
Critical: This blocks ALL keyboard input in the editor.
Line 70 unconditionally returns false for every key event, preventing all typing (letters, numbers, punctuation, etc.). The special-key guard on line 67 becomes meaningless.
Apply this diff to return false only for the targeted shortcuts:
handleKeyDown: (_, event) => {
if ((event.metaKey || event.ctrlKey) && (event.key === "w" || event.key === "n" || event.key === "t")) {
+ event.preventDefault();
return false;
}
- return false;
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| handleKeyDown: (_, event) => { | |
| if ((event.metaKey || event.ctrlKey) && (event.key === "w" || event.key === "n" || event.key === "t")) { | |
| return false; | |
| } | |
| return false; | |
| }, | |
| handleKeyDown: (_, event) => { | |
| if ((event.metaKey || event.ctrlKey) && (event.key === "w" || event.key === "n" || event.key === "t")) { | |
| event.preventDefault(); | |
| return false; | |
| } | |
| }, |
🤖 Prompt for AI Agents
In packages/tiptap/src/editor/index.tsx around lines 66 to 71, the handler
currently returns false for every key event which blocks all typing; change it
so the function only returns false when the event is a targeted shortcut
(metaKey or ctrlKey AND key is "w", "n", or "t"), and otherwise allow normal
handling by returning true (or not returning false) so regular keystrokes are
not blocked.
| if ((e.metaKey || e.ctrlKey) && (e.key === "w" || e.key === "n" || e.key === "t")) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Returning early doesn't prevent the browser shortcuts.
The handler needs to call e.preventDefault() to actually block Cmd/Ctrl+W (close tab), Cmd/Ctrl+N (new window), and Cmd/Ctrl+T (new tab).
Apply this diff:
-if ((e.metaKey || e.ctrlKey) && (e.key === "w" || e.key === "n" || e.key === "t")) {
- return;
-}
+if ((e.metaKey || e.ctrlKey) && (e.key === "w" || e.key === "n" || e.key === "t")) {
+ e.preventDefault();
+ return;
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ((e.metaKey || e.ctrlKey) && (e.key === "w" || e.key === "n" || e.key === "t")) { | |
| return; | |
| } | |
| if ((e.metaKey || e.ctrlKey) && (e.key === "w" || e.key === "n" || e.key === "t")) { | |
| e.preventDefault(); | |
| return; | |
| } |
🤖 Prompt for AI Agents
In packages/tiptap/src/editor/index.tsx around lines 106 to 108, the keyboard
handler currently returns early for (e.metaKey || e.ctrlKey) with keys "w", "n",
or "t" but does not call e.preventDefault(), so the browser shortcuts still run;
update the handler to call e.preventDefault() (and optionally
e.stopPropagation()) before returning when those key combos are detected to
block the browser's default behavior.
| function ParticipantChip({ participant, currentUserId, onClick, onRemove }: ParticipantChipProps) { | ||
| const getInitials = (name: string) => { | ||
| return name | ||
| .split(" ") | ||
| .map((n) => n[0]) | ||
| .join("") | ||
| .toUpperCase() | ||
| .slice(0, 2); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Deduplicate and harden getInitials.
This helper appears elsewhere and can be more robust with whitespace and multi‑space names. Either extract to a shared util in packages/ui (preferred) or minimally update here.
- const getInitials = (name: string) => {
- return name
- .split(" ")
- .map((n) => n[0])
- .join("")
- .toUpperCase()
- .slice(0, 2);
- };
+ const getInitials = (name: string) => {
+ const parts = name.trim().split(/\s+/).filter(Boolean);
+ return parts.slice(0, 2).map(n => n[0]!.toUpperCase()).join("");
+ };Run to locate duplicates and potential consolidation targets:
🏁 Script executed:
#!/bin/bash
# Find repeated getInitials implementations across TS/TSX
rg -nP --type=ts --type=tsx -C2 '\b(getInitials)\b\s*=\s*\('Length of output: 27
I need to fix the file type issue and re-search for duplicates. Let me run corrected shell scripts:
Now let me search for getInitials implementations with corrected commands:
🏁 Script executed:
#!/bin/bash
# Search for getInitials in TypeScript files with corrected approach
rg -nP -C2 '\bgetInitials\b' --type ts
rg -nP -C2 '\bgetInitials\b' -g '*.tsx'Length of output: 13081
Now let me inspect the existing shared getInitials implementation to compare it with what's in the two chip files:
🏁 Script executed:
#!/bin/bash
# View the existing shared getInitials implementation
sed -n '5,15p' apps/desktop2/src/components/main/body/contacts/shared.tsxLength of output: 193
Extract getInitials to packages/ui shared util and consolidate duplicate implementations.
Two identical implementations exist in the same package and should be consolidated. Create a shared utility in packages/ui with the more robust implementation that handles whitespace and multi-space names properly.
Locations requiring refactoring:
packages/ui/src/components/block/meeting-metadata-chip.tsx:377–384packages/ui/src/components/block/participants-chip.tsx:179–186
Suggested implementation:
export const getInitials = (name: string) => {
const parts = name.trim().split(/\s+/).filter(Boolean);
return parts.slice(0, 2).map(n => n[0]!.toUpperCase()).join("");
};🤖 Prompt for AI Agents
In packages/ui/src/components/block/meeting-metadata-chip.tsx around lines
376–384, the local getInitials implementation duplicates another in
participants-chip and doesn't handle extra whitespace or multi-space names;
extract a shared util in packages/ui (e.g., src/utils/string.ts or
src/lib/getInitials.ts) exporting getInitials that trims, splits on /\s+/,
filters empty parts, takes up to two parts and returns their uppercase first
letters; replace the local function here and in
packages/ui/src/components/block/participants-chip.tsx (lines ~179–186) with an
import of the new utility, remove the duplicate implementations, update
imports/exports and run TypeScript compile/tests to ensure no typing or import
errors.
| <a | ||
| href={`https://linkedin.com/in/${participant.linkedin_username}`} | ||
| onClick={(e) => e.stopPropagation()} | ||
| className="text-neutral-400 transition-colors hover:text-neutral-600 p-1 rounded-full hover:bg-neutral-200" | ||
| className="text-color3 transition-colors hover:text-color4 p-1 rounded-full hover:bg-neutral-200" | ||
| > | ||
| <LinkedinIcon className="size-3.5" /> | ||
| <LinkedInIcon className="size-3.5" /> | ||
| </a> | ||
| )} |
There was a problem hiding this comment.
External link hardening (tabnabbing).
Open the LinkedIn profile in a new tab with rel to prevent reverse‑tabnabbing and match MeetingMetadataChip.
- <a
- href={`https://linkedin.com/in/${participant.linkedin_username}`}
- onClick={(e) => e.stopPropagation()}
- className="text-color3 transition-colors hover:text-color4 p-1 rounded-full hover:bg-neutral-200"
- >
+ <a
+ href={`https://linkedin.com/in/${participant.linkedin_username}`}
+ onClick={(e) => e.stopPropagation()}
+ target="_blank"
+ rel="noopener noreferrer"
+ className="text-color3 transition-colors hover:text-color4 p-1 rounded-full hover:bg-neutral-200"
+ >📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <a | |
| href={`https://linkedin.com/in/${participant.linkedin_username}`} | |
| onClick={(e) => e.stopPropagation()} | |
| className="text-neutral-400 transition-colors hover:text-neutral-600 p-1 rounded-full hover:bg-neutral-200" | |
| className="text-color3 transition-colors hover:text-color4 p-1 rounded-full hover:bg-neutral-200" | |
| > | |
| <LinkedinIcon className="size-3.5" /> | |
| <LinkedInIcon className="size-3.5" /> | |
| </a> | |
| )} | |
| <a | |
| href={`https://linkedin.com/in/${participant.linkedin_username}`} | |
| onClick={(e) => e.stopPropagation()} | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| className="text-color3 transition-colors hover:text-color4 p-1 rounded-full hover:bg-neutral-200" | |
| > | |
| <LinkedInIcon className="size-3.5" /> | |
| </a> |
🤖 Prompt for AI Agents
In packages/ui/src/components/block/participants-chip.tsx around lines 238 to
245, the LinkedIn anchor opens an external page without tabnabbing protection;
update the anchor to open in a new tab and include rel attributes like
MeetingMetadataChip does (e.g., add target="_blank" and rel="noopener
noreferrer") so the external link is hardened against reverse-tabnabbing while
preserving the existing stopPropagation and styling.
I should shoot PRs more often