Contacts page initial migration #1549
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds a new Contacts tab (Organizations, People, Details) with UI, edit/create flows, and store wiring; extends human schema/DB and seed data with job_title/linkedin_username/is_user; adds frontend deps; updates ports and tauri/vite config; duplicates analytics init in Tauri builder. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Sidebar as Sidebar Profile
participant Tabs as Tabs Store (zustand)
participant Main as Main Body
participant Contacts as Contacts View
participant Persist as Persisted Store (tinybase)
User->>Sidebar: Click "Contacts"
Sidebar->>Tabs: openNew({ type: "contacts", active: true, state: {...} })
Tabs-->>Main: tabs updated (parsed via tabSchema)
Main->>Contacts: Render TabContentContact
Contacts->>Persist: Query visibleOrganizations, visibleHumans
Persist-->>Contacts: Orgs, Humans
Contacts-->>User: Render Organizations / People / Details
rect rgba(220,240,255,0.35)
note right of Contacts: Edit/create flows
User->>Contacts: Edit person/org
Contacts->>Persist: Update rows
Persist-->>Contacts: Updated data
Contacts-->>User: Updated UI
end
sequenceDiagram
autonumber
participant Tauri as Tauri Builder
Tauri->>Tauri: tauri_plugin_tracing::init()
Tauri->>Tauri: tauri_plugin_analytics::init()
Tauri->>Tauri: tauri_plugin_analytics::init() %% duplicate init
Tauri->>Tauri: tauri_plugin_listener::init()
note over Tauri: Analytics plugin initialized twice
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/desktop2/src-tauri/src/lib.rs (1)
42-54: Critical: Remove duplicate plugin initializations.The builder chain contains duplicate plugin initializations:
tauri_plugin_analytics::init()is called twice (lines 42 and 45)tauri_plugin_tracing::init()is called twice (lines 44 and 54)These duplicates can cause initialization errors, runtime conflicts, or undefined behavior. Remove the duplicate calls on lines 45 and 54.
Apply this diff to remove the duplicates:
builder = builder .plugin(tauri_plugin_opener::init()) .plugin(tauri_plugin_auth::init()) .plugin(tauri_plugin_analytics::init()) .plugin(tauri_plugin_db2::init()) .plugin(tauri_plugin_tracing::init()) - .plugin(tauri_plugin_analytics::init()) .plugin(tauri_plugin_listener::init()) .plugin(tauri_plugin_local_stt::init()) .plugin(tauri_plugin_updater::Builder::new().build()) .plugin(tauri_plugin_deep_link::init()) .plugin(tauri_plugin_sentry::init_with_no_injection(&sentry_client)) .plugin(tauri_plugin_os::init()) .plugin(tauri_plugin_fs::init()) .plugin(tauri_plugin_process::init()) - .plugin(tauri_plugin_tracing::init()) .plugin(tauri_plugin_tray::init()) .plugin(tauri_plugin_store::Builder::default().build()) .plugin(tauri_plugin_windows::init());apps/desktop2/src/components/main/body/shared.tsx (1)
21-29: Restore keyboard accessibility for TabItemBaseSwitching the outer element to a plain
divremoved the built-in keyboard and focus behavior that the old button provided. Tabs are now inaccessible via keyboard and screen readers. Please add the necessary role, tabIndex, and key handling (or revert to a semantic button without nesting another button).- <div - onClick={handleSelect} + <div + role="tab" + tabIndex={0} + onClick={handleSelect} + onKeyDown={(event) => { + if (event.key === "Enter" || event.key === " ") { + event.preventDefault(); + handleSelect(); + } + }} className={clsx([ "flex items-center gap-2 cursor-pointer",apps/desktop2/src/components/main/body/contacts/organizations.tsx (1)
474-489: Implement the “Create …” actionThe “Create “{searchTerm}”” button is rendered when no matches are found, but its
onClickis empty. That leaves the user with no way to create an organization from this control. Please hook it up (e.g., viauseAddRowCallback) or hide it until the flow works.
🧹 Nitpick comments (4)
apps/desktop2/src/components/main/sidebar/profile/banner.tsx (1)
11-23: Nice refactor to button—improves accessibility.Converting from
divtobuttonwith proper click handling is the right approach for an interactive element. The hover states and transitions are well-implemented.Consider adding an accessible label.
While the button has text content, adding an explicit
aria-labelcould improve clarity for screen readers:<button + aria-label="Start Hyprnote Pro free trial" onClick={handleClickTryPro}Optional: Simplify width calculation.
Line 15 uses
mx-4(1rem horizontal margin) withw-[calc(100%-2rem)], which effectively cancels out. Consider usingw-fullinstead:className={clsx( "group", - "mx-4 mb-1.5 w-[calc(100%-2rem)]", + "mx-4 mb-1.5 w-full",apps/desktop2/src/components/main/body/contacts/index.tsx (1)
124-133: Reuse the sharedgetInitialshelperYou’re re‑implementing initials logic here even though
packages/utils/src/string.tsalready exportsgetInitials. Importing that helper keeps behavior consistent with the rest of the app and avoids duplication.-import { Contact2Icon } from "lucide-react"; - -import * as persisted from "../../../../store/tinybase/persisted"; +import { Contact2Icon } from "lucide-react"; + +import { getInitials } from "@hypr/utils/string"; +import * as persisted from "../../../../store/tinybase/persisted"; ... - const getInitials = (name: string | null) => { - if (!name) { - return "?"; - } - return name - .split(" ") - .map(n => n[0]) - .join("") - .toUpperCase() - .slice(0, 2); - }; - return (apps/desktop2/src/components/main/body/contacts/details.tsx (1)
175-185: Reuse the sharedgetInitialshelper
EditPersonFormredefinesgetInitials, duplicating logic that already lives in@hypr/utils/string. Import and reuse the shared helper to keep behavior aligned across components.-import React, { useState } from "react"; - -import { Button } from "@hypr/ui/components/ui/button"; +import React, { useState } from "react"; + +import { getInitials } from "@hypr/utils/string"; +import { Button } from "@hypr/ui/components/ui/button"; ... - const getInitials = (name: string | null) => { - if (!name) { - return "?"; - } - return name - .split(" ") - .map(n => n[0]) - .join("") - .toUpperCase() - .slice(0, 2); - }; -apps/desktop2/src/store/zustand/tabs.ts (1)
108-120: Consider genericizing the tab state updater logic.The implementation is correct and follows the same pattern as
updateSessionTabState. However, the logic is duplicated. You could extract a genericupdateTabState<T extends Tab["type"]>helper to reduce repetition.Example approach:
const createTabStateUpdater = <T extends Tab["type"]>(tabType: T) => (tab: Tab, state: Extract<Tab, { type: T }>["state"]) => { const { tabs, currentTab } = get(); const nextTabs = tabs.map((t) => isSameTab(t, tab) && t.type === tabType ? { ...t, state } : t ); const nextCurrentTab = currentTab && isSameTab(currentTab, tab) && currentTab.type === tabType ? { ...currentTab, state } : currentTab; set({ tabs: nextTabs, currentTab: nextCurrentTab }); };This is optional—the current explicit implementation is clear and maintainable.
📜 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 (16)
apps/desktop2/package.json(1 hunks)apps/desktop2/src-tauri/src/lib.rs(1 hunks)apps/desktop2/src-tauri/tauri.conf.json(1 hunks)apps/desktop2/src/components/main/body/contacts/details.tsx(1 hunks)apps/desktop2/src/components/main/body/contacts/index.tsx(1 hunks)apps/desktop2/src/components/main/body/contacts/organizations.tsx(1 hunks)apps/desktop2/src/components/main/body/contacts/people.tsx(1 hunks)apps/desktop2/src/components/main/body/index.tsx(2 hunks)apps/desktop2/src/components/main/body/shared.tsx(2 hunks)apps/desktop2/src/components/main/sidebar/profile/banner.tsx(1 hunks)apps/desktop2/src/components/main/sidebar/profile/index.tsx(1 hunks)apps/desktop2/src/store/seed.ts(4 hunks)apps/desktop2/src/store/tinybase/persisted.ts(4 hunks)apps/desktop2/src/store/zustand/tabs.ts(8 hunks)apps/desktop2/vite.config.ts(1 hunks)packages/db/src/schema.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{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-tauri/src/lib.rspackages/db/src/schema.tsapps/desktop2/src/components/main/body/contacts/details.tsxapps/desktop2/src/components/main/body/contacts/index.tsxapps/desktop2/src/components/main/sidebar/profile/banner.tsxapps/desktop2/src/components/main/body/shared.tsxapps/desktop2/vite.config.tsapps/desktop2/src/store/zustand/tabs.tsapps/desktop2/src/components/main/sidebar/profile/index.tsxapps/desktop2/src/components/main/body/contacts/organizations.tsxapps/desktop2/src/store/tinybase/persisted.tsapps/desktop2/src/store/seed.tsapps/desktop2/src/components/main/body/index.tsxapps/desktop2/src/components/main/body/contacts/people.tsx
🧬 Code graph analysis (8)
apps/desktop2/src-tauri/src/lib.rs (2)
plugins/tracing/src/lib.rs (1)
init(30-65)plugins/db2/src/lib.rs (1)
init(31-42)
apps/desktop2/src/components/main/body/contacts/details.tsx (3)
packages/db/src/schema.ts (1)
organizations(24-27)packages/utils/src/string.ts (1)
getInitials(1-12)packages/ui/src/components/ui/button.tsx (1)
Button(37-89)
apps/desktop2/src/components/main/body/contacts/index.tsx (6)
apps/desktop2/src/components/main/body/shared.tsx (2)
TabItem(5-9)TabItemBase(11-53)apps/desktop2/src/store/zustand/tabs.ts (2)
Tab(176-176)useTabs(31-121)packages/utils/src/string.ts (1)
getInitials(1-12)apps/desktop2/src/components/main/body/contacts/organizations.tsx (1)
OrganizationsColumn(7-96)apps/desktop2/src/components/main/body/contacts/people.tsx (1)
PeopleColumn(6-85)apps/desktop2/src/components/main/body/contacts/details.tsx (1)
DetailsColumn(9-146)
apps/desktop2/src/components/main/body/contacts/organizations.tsx (2)
packages/ui/src/lib/utils.ts (1)
cn(4-6)packages/db/src/schema.ts (1)
organizations(24-27)
apps/desktop2/src/store/tinybase/persisted.ts (1)
packages/db/src/schema.ts (1)
humanSchema(136-136)
apps/desktop2/src/store/seed.ts (1)
packages/db/src/schema.ts (1)
humans(13-21)
apps/desktop2/src/components/main/body/index.tsx (2)
apps/desktop2/src/components/main/body/calendars.tsx (2)
TabItemCalendar(10-20)TabContentCalendar(22-60)apps/desktop2/src/components/main/body/contacts/index.tsx (2)
TabItemContact(10-20)TabContentContact(22-32)
apps/desktop2/src/components/main/body/contacts/people.tsx (2)
packages/ui/src/lib/utils.ts (1)
cn(4-6)packages/utils/src/string.ts (1)
getInitials(1-12)
🪛 Biome (2.1.2)
apps/desktop2/src/components/main/body/contacts/index.tsx
[error] 39-39: 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] 67-67: 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] 68-68: 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] 69-69: 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] 72-72: 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)
🪛 GitHub Actions: .github/workflows/desktop2_ci.yaml
apps/desktop2/src/components/main/sidebar/profile/index.tsx
[error] 52-52: TypeScript error TS2345: Argument of type '{ type: "contacts"; active: true; }' is not assignable to parameter of type '{ active: boolean; type: "sessions"; id: string; state: { editor: "transcript" | "raw" | "enhanced"; }; } | { active: boolean; type: "contacts"; state: { selectedOrganization: string | null; ... 4 more ...; sortOption: "alphabetical" | ... 1 more ... | "newest"; }; } | ... 4 more ... | { ...; }'.
🔇 Additional comments (24)
apps/desktop2/src/components/main/sidebar/profile/banner.tsx (2)
6-8: Verify the placeholder onClick handler.The click handler currently only logs to the console. Given this is a "Contacts page initial migration" PR, confirm whether this placeholder is intentional for this phase or if the actual Pro trial flow implementation is expected before merge.
31-34: Good use of group hover for the arrow animation.The subtle translate effect on hover (
group-hover:translate-x-0.5) provides nice visual feedback that the button is interactive.packages/db/src/schema.ts (1)
18-20: LGTM! Schema extension looks correct.The three new nullable columns (job_title, linkedin_username, is_user) are properly defined and follow the existing schema conventions. The nullable approach is appropriate for optional human attributes.
apps/desktop2/vite.config.ts (1)
27-34: LGTM! Port configuration updated correctly.The port changes (1420→1422 for server, 1421→1423 for HMR) align with the corresponding updates in
apps/desktop2/src-tauri/tauri.conf.json.apps/desktop2/src-tauri/tauri.conf.json (1)
9-9: LGTM! Development URL updated consistently.The devUrl change to
http://localhost:1422correctly aligns with the Vite server port update inapps/desktop2/vite.config.ts.apps/desktop2/src/store/tinybase/persisted.ts (4)
41-46: LGTM! Schema extension properly handles nullable fields.The humanSchema extension correctly uses
z.preprocessto map null values to undefined for optional fields (job_title, linkedin_username, is_user), which ensures consistent handling of nullable database columns.
156-158: LGTM! Table schema updated consistently.The SCHEMA.table.humans definition correctly includes the new fields with appropriate types matching the humanSchema definition.
408-428: LGTM! Query definitions provide focused data selection.The new visibleHumans and visibleOrganizations queries select the appropriate fields for UI display, including the newly added human attributes. The query structure follows the existing pattern.
495-496: LGTM! Query constants properly registered.The QUERIES object correctly includes the new query names for easy reference throughout the codebase.
apps/desktop2/src/store/seed.ts (4)
41-78: LGTM! Human creation enhanced with realistic data.The createHuman function now supports:
- User designation via isUser parameter
- Realistic job titles from a predefined list
- LinkedIn usernames with 70% probability
- All changes align with the extended human schema
368-387: LGTM! Current user creation logic is well-structured.Creating the current user first in the first organization before generating other humans ensures proper data initialization and makes the seed data more realistic.
511-512: LGTM! Seed data scale increased appropriately.The increase to 8 organizations with 5-12 humans per organization provides better test data coverage for the new Contacts UI.
535-542: LGTM! Enhanced logging provides useful metrics.The additional logging for total organizations, humans, and current users helps verify the seed data generation is working correctly.
apps/desktop2/src/components/main/body/index.tsx (4)
2-2: LGTM! Icon import consolidated.Replacing the duplicate icon import with a direct import from lucide-react improves code organization.
9-9: LGTM! Contacts tab components imported.The imports for TabContentContact and TabItemContact align with the existing pattern for other tab types.
95-100: LGTM! Contacts tab rendering added to TabItem.The contacts tab case is properly integrated into the TabItem switch logic, following the same pattern as other tab types.
119-124: LGTM! Contacts tab content rendering added.The contacts tab case is properly integrated into the Content switch logic, following the same pattern as other tab types.
apps/desktop2/src/store/zustand/tabs.ts (7)
12-22: LGTM! Type definition improvements.The renaming from
TabUpdatortoTabUpdaterfixes the typo, and explicitly definingTabUpdaterwith the newsetTabsmethod improves type clarity.
34-37: LGTM! Parsing ensures consistent defaults.The implementation correctly applies defaults to all tabs via
tabSchema.parse()before storing them, ensuring consistent state initialization.
38-60: LGTM! Correct handling of tab replacement and deduplication.The implementation correctly parses the incoming tab to apply defaults before comparison, properly handles both cases (active tab present or absent), and ensures duplicate tabs are filtered out while maintaining active state.
62-70: LGTM! Clean implementation.The method correctly parses the incoming tab, filters duplicates, and sets the new tab as active.
135-152: LGTM! Comprehensive state definition for contacts tab.The schema correctly defines the contacts tab with a detailed state object. The double specification of defaults (both on individual fields and on the state object) is necessary for Zod to properly type and apply defaults to nested objects.
189-190: LGTM! Correct handling of the contacts tab type.The implementation correctly treats
contactsas aninvalid_resourceinrowIdfromTab(since it's not a resource-based tab) and returns a unique identifier"contacts"inuniqueIdfromTab(since there's only one contacts tab instance).Also applies to: 211-212
170-173: LGTM! Calendar tab definition in place.The calendars tab schema is correctly defined with a month field. The positioning in the discriminated union doesn't affect functionality.
No description provided.