Conversation
📝 WalkthroughWalkthroughRefactors Contacts to derive data from persisted TinyBase IDs/hooks instead of props, removes several contacts fields from the tabs state, adds a TinyBase index Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ParticipantComp as Participant component
participant TabsStore as useTabs store
participant ContactsView as Contacts dialog/view
participant Persisted as TinyBase persisted store
User->>ParticipantComp: Click participant row
ParticipantComp->>TabsStore: openNew({ type: "contacts", selectedPerson: id, selectedOrganization: null, active: true })
TabsStore-->>ContactsView: open Contacts with selectedHumanId
ContactsView->>Persisted: query INDEXES.sessionsByHuman for selectedHumanId
Persisted-->>ContactsView: return mapping ids → sessions
ContactsView->>TabsStore: openNew({ type: "sessions", raw: sessionState }) on session click
ContactsView-->>User: render contact details and sessions list
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 2 inconclusive)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (2)apps/desktop2/**/*.{tsx,jsx}📄 CodeRabbit inference engine (apps/desktop2/.cursor/rules/style.mdc)
Files:
**/*.{js,ts,tsx,rs}⚙️ CodeRabbit configuration file
Files:
🧬 Code graph analysis (1)apps/desktop2/src/components/main/body/contacts/organizations.tsx (2)
⏰ 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)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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/body/contacts/index.tsx (1)
34-112: Critical: Hooks called after conditional return.All hooks in the
ContactViewcomponent are called after the early return at lines 35-36. React requires hooks to be called unconditionally at the top level of the component to maintain consistent ordering across renders.Move all hooks before the conditional return:
function ContactView({ tab }: { tab: Tab }) { - if (tab.type !== "contacts") { - return null; - } - const updateContactsTabState = useTabs((state) => state.updateContactsTabState); const { openNew } = useTabs(); - - const { selectedOrganization, selectedPerson, editingPerson, editingOrg, showNewOrg, sortOption } = tab.state; - const setSelectedOrganization = (value: string | null) => { updateContactsTabState(tab, { ...tab.state, selectedOrganization: value }); }; - const setSelectedPerson = (value: string | null) => { updateContactsTabState(tab, { ...tab.state, selectedPerson: value }); }; - const setEditingPerson = (value: string | null) => { updateContactsTabState(tab, { ...tab.state, editingPerson: value }); }; - const setEditingOrg = (value: string | null) => { updateContactsTabState(tab, { ...tab.state, editingOrg: value }); }; - const setShowNewOrg = (value: boolean) => { updateContactsTabState(tab, { ...tab.state, showNewOrg: value }); }; - const setSortOption = (value: "alphabetical" | "oldest" | "newest") => { updateContactsTabState(tab, { ...tab.state, sortOption: value }); }; - const organizationsData = persisted.UI.useResultTable(persisted.QUERIES.visibleOrganizations, persisted.STORE_ID); const humansData = persisted.UI.useResultTable(persisted.QUERIES.visibleHumans, persisted.STORE_ID); - const selectedPersonData = persisted.UI.useRow("humans", selectedPerson ?? "", persisted.STORE_ID); + const selectedPersonData = persisted.UI.useRow("humans", tab.type === "contacts" ? tab.state.selectedPerson ?? "" : "", persisted.STORE_ID); const allSessions = persisted.UI.useTable("sessions", persisted.STORE_ID); - - // Get humans by organization if one is selected const humanIdsByOrg = persisted.UI.useSliceRowIds( persisted.INDEXES.humansByOrg, - selectedOrganization ?? "", + tab.type === "contacts" ? tab.state.selectedOrganization ?? "" : "", persisted.STORE_ID, ); - const mappingIdsByHuman = persisted.UI.useSliceRowIds( persisted.INDEXES.sessionsByHuman, - selectedPerson ?? "", + tab.type === "contacts" ? tab.state.selectedPerson ?? "" : "", persisted.STORE_ID, ); - const allMappings = persisted.UI.useTable("mapping_session_participant", persisted.STORE_ID); - const personSessions = useMemo(() => { if (!mappingIdsByHuman || mappingIdsByHuman.length === 0) { return []; } return mappingIdsByHuman .map((mappingId: string) => { const mapping = allMappings[mappingId]; if (!mapping || !mapping.session_id) { return null; } const sessionId = mapping.session_id as string; const session = allSessions[sessionId]; if (!session) { return null; } return { id: sessionId, ...session, }; }) .filter((session: any): session is NonNullable<typeof session> => session !== null); }, [mappingIdsByHuman, allMappings, allSessions]); - - console.log("personSessions", personSessions); + + if (tab.type !== "contacts") { + return null; + } + + const { selectedOrganization, selectedPerson, editingPerson, editingOrg, showNewOrg, sortOption } = tab.state; // Convert to arrays for rendering
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/desktop2/src/components/main/body/contacts/index.tsx(5 hunks)apps/desktop2/src/store/tinybase/persisted.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
apps/desktop2/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/desktop2/.cursor/rules/style.mdc)
apps/desktop2/**/*.{ts,tsx,js,jsx}: Use the cn utility for conditional Tailwind classNames, importing it as: import { cn } from "@hypr/ui/lib/utils"; avoid using clsx/classnames directly.
When calling cn, always pass an array of class segments (e.g., cn(["base", condition && "modifier"]))
Files:
apps/desktop2/src/components/main/body/contacts/index.tsxapps/desktop2/src/store/tinybase/persisted.ts
**/*.{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/contacts/index.tsxapps/desktop2/src/store/tinybase/persisted.ts
🧬 Code graph analysis (1)
apps/desktop2/src/components/main/body/contacts/index.tsx (1)
apps/desktop2/src/store/zustand/tabs.ts (1)
useTabs(31-121)
🪛 Biome (2.1.2)
apps/desktop2/src/components/main/body/contacts/index.tsx
[error] 40-40: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 71-71: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 80-80: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 86-86: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 88-88: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
⏰ 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: ci (windows, windows-latest)
- GitHub Check: ci (macos, macos-14)
🔇 Additional comments (2)
apps/desktop2/src/store/tinybase/persisted.ts (1)
436-436: LGTM!The new
sessionsByHumanindex is correctly defined and enables efficient lookup of sessions by participant. The implementation follows the established pattern for index definitions.Also applies to: 520-520
apps/desktop2/src/components/main/body/contacts/index.tsx (1)
40-40: Confirm session click opens in a new tab
contacts/index.tsx:144 now callsopenNew(always new) instead ofopenCurrent(replace current tab); ensure this aligns with the intended UX.
There was a problem hiding this comment.
Actionable comments posted: 0
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/body/contacts/index.tsx (1)
34-40: Move early return after all hook calls.All hooks (lines 40, 71, 80, 86, 88) are called after the early return on lines 35-37, which violates the Rules of Hooks. Hooks must be called unconditionally and in the same order on every render.
Move the type guard check to after all hook calls:
function ContactView({ tab }: { tab: Tab }) { - if (tab.type !== "contacts") { - return null; - } - const updateContactsTabState = useTabs((state) => state.updateContactsTabState); const { openNew } = useTabs(); + + if (tab.type !== "contacts") { + return null; + }Then move all remaining hook calls (lines 68-112) above the type guard as well.
🧹 Nitpick comments (1)
apps/desktop2/src/components/main/body/contacts/index.tsx (1)
88-112: Consider a more specific type annotation.The memoization logic correctly maps session data through the mapping table and filters out nulls. However, the type annotation on line 111 uses
any.If the session shape is defined elsewhere, consider using it:
- .filter((session: any): session is NonNullable<typeof session> => session !== null); + .filter((session): session is NonNullable<typeof session> => session !== null);Or define an explicit return type for the useMemo callback.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop2/src/components/main/body/contacts/index.tsx(5 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
apps/desktop2/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/desktop2/.cursor/rules/style.mdc)
apps/desktop2/**/*.{ts,tsx,js,jsx}: Use the cn utility for conditional Tailwind classNames, importing it as: import { cn } from "@hypr/ui/lib/utils"; avoid using clsx/classnames directly.
When calling cn, always pass an array of class segments (e.g., cn(["base", condition && "modifier"]))
Files:
apps/desktop2/src/components/main/body/contacts/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/body/contacts/index.tsx
🧬 Code graph analysis (1)
apps/desktop2/src/components/main/body/contacts/index.tsx (1)
apps/desktop2/src/store/zustand/tabs.ts (1)
useTabs(31-121)
🪛 Biome (2.1.2)
apps/desktop2/src/components/main/body/contacts/index.tsx
[error] 40-40: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 71-71: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 80-80: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 86-86: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 88-88: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
⏰ 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: ci (windows, windows-latest)
- GitHub Check: ci (macos, macos-14)
🔇 Additional comments (1)
apps/desktop2/src/components/main/body/contacts/index.tsx (1)
142-142: LGTM!The implementation correctly uses
openNewto open the session in a new tab with the raw editor state. This properly replaces the previous debug console.log.
There was a problem hiding this comment.
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 (1)
apps/desktop2/src/components/main/body/contacts/organizations.tsx (1)
15-66: Sorting option is never applied
sortOption/setSortOptionare wired into the header, but the list rendering still iterates the raworganizationIdsarray. Changing the dropdown has no effect, so the sort control is now a dead UI element. Please either drive the query/input that feedsorganizationIdswithsortOption, or locally reorder the IDs before rendering so the list reflects the selected sort.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/desktop2/.cursor/rules/correctness.mdc(1 hunks)apps/desktop2/src/components/main/body/contacts/details.tsx(5 hunks)apps/desktop2/src/components/main/body/contacts/index.tsx(2 hunks)apps/desktop2/src/components/main/body/contacts/organizations.tsx(3 hunks)apps/desktop2/src/components/main/body/contacts/people.tsx(1 hunks)apps/desktop2/src/components/main/body/contacts/shared.tsx(1 hunks)apps/desktop2/src/components/main/body/sessions/outer-header/participant.tsx(3 hunks)apps/desktop2/src/components/main/sidebar/profile/index.tsx(1 hunks)apps/desktop2/src/store/zustand/tabs.ts(0 hunks)
💤 Files with no reviewable changes (1)
- apps/desktop2/src/store/zustand/tabs.ts
✅ Files skipped from review due to trivial changes (1)
- apps/desktop2/.cursor/rules/correctness.mdc
🧰 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/participant.tsxapps/desktop2/src/components/main/body/contacts/organizations.tsxapps/desktop2/src/components/main/body/contacts/shared.tsxapps/desktop2/src/components/main/sidebar/profile/index.tsxapps/desktop2/src/components/main/body/contacts/people.tsxapps/desktop2/src/components/main/body/contacts/index.tsxapps/desktop2/src/components/main/body/contacts/details.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/participant.tsxapps/desktop2/src/components/main/body/contacts/organizations.tsxapps/desktop2/src/components/main/body/contacts/shared.tsxapps/desktop2/src/components/main/sidebar/profile/index.tsxapps/desktop2/src/components/main/body/contacts/people.tsxapps/desktop2/src/components/main/body/contacts/index.tsxapps/desktop2/src/components/main/body/contacts/details.tsx
🧬 Code graph analysis (4)
apps/desktop2/src/components/main/body/sessions/outer-header/participant.tsx (1)
apps/desktop2/src/store/zustand/tabs.ts (1)
useTabs(31-121)
apps/desktop2/src/components/main/body/contacts/organizations.tsx (2)
apps/desktop2/src/components/main/body/contacts/shared.tsx (2)
SortOption(17-17)ColumnHeader(49-74)packages/ui/src/lib/utils.ts (1)
cn(4-6)
apps/desktop2/src/components/main/body/contacts/people.tsx (2)
apps/desktop2/src/components/main/body/contacts/shared.tsx (3)
SortOption(17-17)ColumnHeader(49-74)getInitials(5-15)packages/ui/src/lib/utils.ts (1)
cn(4-6)
apps/desktop2/src/components/main/body/contacts/index.tsx (4)
apps/desktop2/src/store/zustand/tabs.ts (1)
useTabs(31-121)apps/desktop2/src/components/main/body/contacts/organizations.tsx (1)
OrganizationsColumn(8-71)apps/desktop2/src/components/main/body/contacts/people.tsx (1)
PeopleColumn(7-50)apps/desktop2/src/components/main/body/contacts/details.tsx (1)
DetailsColumn(10-162)
🪛 Biome (2.1.2)
apps/desktop2/src/components/main/body/contacts/index.tsx
[error] 41-41: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 45-45: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 49-49: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 53-53: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 57-57: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
🔇 Additional comments (1)
apps/desktop2/src/components/main/sidebar/profile/index.tsx (1)
127-127: LGTM! Presentational update aligns with PR objectives.The styling change from
text-slate-900 font-semiboldtotext-blackis a deliberate presentational polish that removes font weight and adjusts color. This aligns with similar updates across the profile UI mentioned in the PR summary.
There was a problem hiding this comment.
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/body/contacts/index.tsx (1)
35-61: Move hooks above the early return; avoid conditional hooksHooks are called after an early return, violating rules-of-hooks and flagged by the linter. Also select openNew via a selector to avoid extra re-renders.
function ContactView({ tab }: { tab: Tab }) { - if (tab.type !== "contacts") { - return null; - } - - const updateContactsTabState = useTabs((state) => state.updateContactsTabState); - const { openNew } = useTabs(); + const updateContactsTabState = useTabs((state) => state.updateContactsTabState); + const openNew = useTabs((state) => state.openNew); + + if (tab.type !== "contacts") { + return null; + } @@ - const handleSessionClick = useCallback((id: string) => { - openNew({ type: "sessions", id, active: true, state: { editor: "raw" } }); - }, [openNew]); + const handleSessionClick = useCallback( + (id: string) => { + openNew({ type: "sessions", id, active: true, state: { editor: "raw" } }); + }, + [openNew], + );As per coding guidelines
🧹 Nitpick comments (7)
apps/desktop2/src/components/main/body/contacts/shared.tsx (3)
27-33: Fix Select onValueChange typingSelect’s onValueChange typically passes a string. Narrowing the param to SortOption can cause TS incompatibility. Cast inside the handler.
- <Select - value={sortOption} - onValueChange={(value: SortOption) => setSortOption(value)} - > + <Select + value={sortOption} + onValueChange={(value) => setSortOption(value as SortOption)} + >
5-15: Make getInitials resilient to extra whitespaceTrim and split on whitespace; filter empties. Same output, safer for inputs like " Ada Lovelace ".
-export const getInitials = (name?: string | null) => { +export const getInitials = (name?: string | null) => { if (!name) { return "?"; } - return name - .split(" ") - .map(n => n[0]) + return name + .trim() + .split(/\s+/) + .filter(Boolean) + .map((n) => n[0]) .join("") .toUpperCase() .slice(0, 2); };
49-73: Consider making onAdd optional to hide the Add button when not neededPeopleColumn currently passes a no-op onAdd. Allow ColumnHeader to omit the Add button unless onAdd is provided.
export function ColumnHeader({ title, sortOption, setSortOption, onAdd, }: { title: string; sortOption?: SortOption; setSortOption?: (option: SortOption) => void; - onAdd: () => void; + onAdd?: () => void; }) { return ( <div className="px-3 py-2 border-b border-neutral-200 flex items-center justify-between h-12"> <h3 className="text-xs font-medium text-neutral-600">{title}</h3> <div className="flex items-center gap-1"> {sortOption && setSortOption && <SortDropdown sortOption={sortOption} setSortOption={setSortOption} />} - <button - onClick={onAdd} - className="p-0.5 rounded hover:bg-neutral-100 transition-colors" - > - <Plus className="h-3 w-3 text-neutral-500" /> - </button> + {onAdd && ( + <button + onClick={onAdd} + className="p-0.5 rounded hover:bg-neutral-100 transition-colors" + > + <Plus className="h-3 w-3 text-neutral-500" /> + </button> + )} </div> </div> ); }apps/desktop2/src/components/main/body/contacts/organizations.tsx (1)
112-118: Align edit handler prop with actual usageThe handler ignores the id argument at call site. Simplify type to () => void and call without args.
function OrganizationItem({ @@ - handleEditOrganization: (id: string) => void; + handleEditOrganization: () => void; }) { @@ - onClick={(e) => { + onClick={(e) => { e.stopPropagation(); - handleEditOrganization(organizationId); + handleEditOrganization(); }}Also applies to: 136-141
apps/desktop2/src/components/main/body/contacts/people.tsx (2)
20-26: Avoid rendering an inert Add buttononAdd is a no-op; hide the button by omitting onAdd if not used (pairs with making onAdd optional in ColumnHeader).
- <ColumnHeader - title="People" - sortOption={sortOption} - setSortOption={setSortOption} - onAdd={() => { - }} - /> + <ColumnHeader + title="People" + sortOption={sortOption} + setSortOption={setSortOption} + />
71-88: Optimize filter to O(n) using a Setincludes in a loop is O(n^2). Use a Set for thisOrgHumanIds.
const thisOrgHumanIds = persisted.UI.useSliceRowIds( persisted.INDEXES.humansByOrg, currentOrgId ?? "", persisted.STORE_ID, ); - const humanIds = currentOrgId - ? (sortOption === "alphabetical" - ? allAlphabeticalIds - : sortOption === "newest" - ? allNewestIds - : allOldestIds).filter((id) => thisOrgHumanIds.includes(id)) - : (sortOption === "alphabetical" - ? allAlphabeticalIds - : sortOption === "newest" - ? allNewestIds - : allOldestIds); + const humanIds = currentOrgId + ? (() => { + const set = new Set(thisOrgHumanIds); + const base = + sortOption === "alphabetical" + ? allAlphabeticalIds + : sortOption === "newest" + ? allNewestIds + : allOldestIds; + return base.filter((id) => set.has(id)); + })() + : sortOption === "alphabetical" + ? allAlphabeticalIds + : sortOption === "newest" + ? allNewestIds + : allOldestIds;apps/desktop2/src/components/main/body/contacts/details.tsx (1)
476-492: Remove or implement the “Create” action (currently a no‑op)The button does nothing; this confuses users. Either wire it to an add‑org callback or hide it until implemented.
- {organizations.length === 0 && ( - <button - type="button" - className="flex items-center px-3 py-2 text-sm text-left hover:bg-gray-100 transition-colors w-full" - onClick={() => {}} - > - <span className="flex-shrink-0 size-5 flex items-center justify-center mr-2 bg-gray-200 rounded-full"> - <span className="text-xs">+</span> - </span> - <span className="flex items-center gap-1 font-medium text-gray-600"> - Create - <span className="text-gray-900 truncate max-w-[140px]"> - "{searchTerm.trim()}" - </span> - </span> - </button> - )} + {/* Intentionally hide Create action until implemented */}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/desktop2/.cursor/rules/correctness.mdc(1 hunks)apps/desktop2/src/components/main/body/contacts/details.tsx(5 hunks)apps/desktop2/src/components/main/body/contacts/index.tsx(2 hunks)apps/desktop2/src/components/main/body/contacts/organizations.tsx(3 hunks)apps/desktop2/src/components/main/body/contacts/people.tsx(1 hunks)apps/desktop2/src/components/main/body/contacts/shared.tsx(1 hunks)apps/desktop2/src/components/main/body/sessions/outer-header/participant.tsx(3 hunks)apps/desktop2/src/components/main/sidebar/profile/index.tsx(1 hunks)apps/desktop2/src/store/zustand/tabs.ts(0 hunks)
💤 Files with no reviewable changes (1)
- apps/desktop2/src/store/zustand/tabs.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/desktop2/src/components/main/body/sessions/outer-header/participant.tsx
- apps/desktop2/src/components/main/sidebar/profile/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/contacts/organizations.tsxapps/desktop2/src/components/main/body/contacts/details.tsxapps/desktop2/src/components/main/body/contacts/people.tsxapps/desktop2/src/components/main/body/contacts/index.tsxapps/desktop2/src/components/main/body/contacts/shared.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/contacts/organizations.tsxapps/desktop2/src/components/main/body/contacts/details.tsxapps/desktop2/src/components/main/body/contacts/people.tsxapps/desktop2/src/components/main/body/contacts/index.tsxapps/desktop2/src/components/main/body/contacts/shared.tsx
🧬 Code graph analysis (4)
apps/desktop2/src/components/main/body/contacts/organizations.tsx (2)
apps/desktop2/src/components/main/body/contacts/shared.tsx (2)
ColumnHeader(49-74)SortOption(17-17)packages/ui/src/lib/utils.ts (1)
cn(4-6)
apps/desktop2/src/components/main/body/contacts/people.tsx (1)
apps/desktop2/src/components/main/body/contacts/shared.tsx (3)
ColumnHeader(49-74)SortOption(17-17)getInitials(5-15)
apps/desktop2/src/components/main/body/contacts/index.tsx (4)
apps/desktop2/src/store/zustand/tabs.ts (1)
useTabs(31-121)apps/desktop2/src/components/main/body/contacts/organizations.tsx (1)
OrganizationsColumn(8-69)apps/desktop2/src/components/main/body/contacts/people.tsx (1)
PeopleColumn(7-41)apps/desktop2/src/components/main/body/contacts/details.tsx (1)
DetailsColumn(10-162)
apps/desktop2/src/components/main/body/contacts/shared.tsx (1)
packages/ui/src/components/ui/select.tsx (5)
Select(174-174)SelectTrigger(174-174)SelectValue(174-174)SelectContent(174-174)SelectItem(174-174)
🪛 Biome (2.1.2)
apps/desktop2/src/components/main/body/contacts/index.tsx
[error] 41-41: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 45-45: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 49-49: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 53-53: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 57-57: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
🔇 Additional comments (1)
apps/desktop2/src/components/main/body/contacts/people.tsx (1)
101-125: Guard against missing person rowDirectly reading person.name/email may throw if the row is undefined.
function PersonItem({ @@ }) { - const person = persisted.UI.useRow("humans", humanId, persisted.STORE_ID); + const person = persisted.UI.useRow("humans", humanId, persisted.STORE_ID); + if (!person) { + return null; + } @@ - {getInitials(person.name || person.email)} + {getInitials(person.name || person.email)} @@ - {person.name || person.email || "Unnamed"} + {person.name || person.email || "Unnamed"} - {person.is_user && <span className="text-xs bg-blue-100 text-blue-700 px-1.5 py-0.5 rounded-full">You</span>} + {person.is_user && <span className="text-xs bg-blue-100 text-blue-700 px-1.5 py-0.5 rounded-full">You</span>}Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/desktop2/src/components/main/body/sessions/outer-header/participant.tsx (1)
8-8: Prefer a selector to avoid unnecessary re-renders.Destructuring
{ openNew } = useTabs()subscribes the component to the entire tabs store, causing re-renders whenever any part of the store changes (e.g.,currentTab,tabsarray). Since the component only needsopenNew, use a selector instead.Based on learnings
Apply this diff to use a selector:
-import { useTabs } from "../../../../../store/zustand/tabs"; +import { useTabs } from "../../../../../store/zustand/tabs";- const { openNew } = useTabs(); + const openNew = useTabs((state) => state.openNew);Also applies to: 18-18
📜 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/participant.tsx(3 hunks)apps/desktop2/src/components/main/sidebar/timeline.tsx(1 hunks)apps/desktop2/src/store/tinybase/persisted.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop2/src/store/tinybase/persisted.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/body/sessions/outer-header/participant.tsxapps/desktop2/src/components/main/sidebar/timeline.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/participant.tsxapps/desktop2/src/components/main/sidebar/timeline.tsx
🧬 Code graph analysis (1)
apps/desktop2/src/components/main/body/sessions/outer-header/participant.tsx (1)
apps/desktop2/src/store/zustand/tabs.ts (1)
useTabs(31-121)
⏰ 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: ci (windows, windows-latest)
- GitHub Check: ci (macos, macos-14)
🔇 Additional comments (2)
apps/desktop2/src/components/main/sidebar/timeline.tsx (1)
55-55: LGTM! Good UX improvement.Defaulting to "Untitled" instead of an empty string improves the user experience by ensuring timeline items and newly created sessions always display a meaningful title.
apps/desktop2/src/components/main/body/sessions/outer-header/participant.tsx (1)
197-206: LGTM!The implementation correctly replaces the console.log with proper navigation logic. Opening a new contacts tab with the selected participant is the right UX pattern, and the state structure matches the expected contacts tab schema.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop2/src/components/main/body/contacts/organizations.tsx(3 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/body/contacts/organizations.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/contacts/organizations.tsx
🧬 Code graph analysis (1)
apps/desktop2/src/components/main/body/contacts/organizations.tsx (2)
apps/desktop2/src/components/main/body/contacts/shared.tsx (2)
ColumnHeader(49-74)SortOption(17-17)packages/ui/src/lib/utils.ts (1)
cn(4-6)
⏰ 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: ci (windows, windows-latest)
- GitHub Check: ci (macos, macos-14)
2 participants
No description provided.