Conversation
ComputelessComputer
force-pushed
the
jj-branch-25
branch
from
3d8d0e7 to
08625f1
Compare
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughThis PR refactors the template editing system by introducing navigation-based template selection, restructuring the template editor with a new sections list component, and adding filter/search UI to both the integrations and templates settings pages. Icon props are simplified in the floating listen component. Changes
Sequence DiagramsequenceDiagram
participant User
participant SettingsLayout as Settings Layout
participant TemplatesPage as Templates Page
participant TemplateEditor as Template Editor
participant SectionsList as Sections List
User->>SettingsLayout: Navigate to Templates tab
SettingsLayout->>TemplatesPage: Render with navigate prop
TemplatesPage->>TemplatesPage: Render template list/search
User->>TemplatesPage: Click template card
TemplatesPage->>SettingsLayout: navigate({search: {templateId}})
SettingsLayout->>SettingsLayout: Detect templateId in search
SettingsLayout->>TemplateEditor: Render TemplateEditor(id)
User->>TemplateEditor: Edit template
TemplateEditor->>SectionsList: Render sections
User->>SectionsList: Reorder/edit/add sections
SectionsList->>SectionsList: Update local items + IDs
SectionsList->>TemplateEditor: onChange(sections)
TemplateEditor->>TemplateEditor: Update template state
User->>TemplateEditor: Click back
TemplateEditor->>SettingsLayout: Navigation back
SettingsLayout->>SettingsLayout: Clear templateId
SettingsLayout->>TemplatesPage: Render template list
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes The changes span multiple files with mixed complexity: signature changes to exported components ( Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (7)
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 |
ComputelessComputer
force-pushed
the
jj-branch-25
branch
from
08625f1 to
1ef803f
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
apps/desktop/src/components/main/body/sessions/floating/listen.tsx (1)
131-131: Consider standardizing icon sizing in DuringMeetingButton.Line 131 still uses
className="w-5 h-5 mt-1.5"for the stop icon. For consistency with the refactored icons above, consider extracting the margin utility separately or updating this to use thesizeprop as well (note: this is out of scope for the current PR but worth addressing in a follow-up).apps/desktop/src/components/settings/template/editor.tsx (1)
9-11: Use explicit type instead ofany.The sections parameter is typed as
any[]. Consider using thepersisted.TemplateSectiontype for better type safety.Apply this diff:
- const handleSectionsChange = (sections: any[]) => { + const handleSectionsChange = (sections: (persisted.TemplateSection & { id: string })[]) => { handle.setField("sections", sections.map(({ id, ...rest }) => rest)); };apps/desktop/src/routes/app/settings/_layout.tsx (2)
62-65: Placeholder implementation for creating templates.The create template logic is not yet implemented. This should be completed before the PR is merged to provide a full feature.
Do you want me to help generate the create template implementation, or would you like me to open an issue to track this work?
234-243: Placeholder implementations for duplicate and delete actions.Both handlers are incomplete. The delete action should include a confirmation dialog to prevent accidental deletions.
Would you like me to generate the implementation for these actions, including a confirmation dialog for delete?
apps/desktop/src/components/settings/template/index.tsx (1)
23-26: Consider improving type safety.The double type cast (
as unknown as Record<string, persisted.Template>) suggests a type mismatch between TinyBase's return type and the expected template structure. While this might be necessary for the current integration, consider adding proper type definitions to TinyBase queries or creating a typed wrapper to avoid unsafe casts.apps/desktop/src/components/settings/template/sections-list.tsx (1)
95-122: Type dragControls properly.The
dragControlsprop is typed asanyon line 100. Use the proper type from the motion library for better type safety.Apply this diff:
+import type { DragControls } from "motion/react"; + interface SectionItemProps { disabled: boolean; item: ReorderItem & { id: string }; onChange: (item: ReorderItem & { id: string }) => void; onDelete: (itemId: string) => void; - dragControls: any; + dragControls: DragControls; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/desktop/src/components/main/body/sessions/floating/listen.tsx(4 hunks)apps/desktop/src/components/settings/integrations.tsx(3 hunks)apps/desktop/src/components/settings/template/editor.tsx(1 hunks)apps/desktop/src/components/settings/template/index.tsx(2 hunks)apps/desktop/src/components/settings/template/sections-list.tsx(1 hunks)apps/desktop/src/devtool/seed/shared.ts(2 hunks)apps/desktop/src/routes/app/settings/_layout.index.tsx(2 hunks)apps/desktop/src/routes/app/settings/_layout.tsx(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
apps/desktop/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (apps/desktop/.cursor/rules/style.mdc)
apps/desktop/**/*.{tsx,jsx}: When there are many Tailwind classNames with conditional logic, use the utilitycnimported asimport { cn } from "@hypr/utils"
When usingcnfor Tailwind classNames, always pass an array
Group Tailwind classNames by logical sections when usingcn(split array items by grouping)
Files:
apps/desktop/src/components/main/body/sessions/floating/listen.tsxapps/desktop/src/routes/app/settings/_layout.index.tsxapps/desktop/src/components/settings/template/editor.tsxapps/desktop/src/components/settings/integrations.tsxapps/desktop/src/components/settings/template/index.tsxapps/desktop/src/components/settings/template/sections-list.tsxapps/desktop/src/routes/app/settings/_layout.tsx
🧬 Code graph analysis (5)
apps/desktop/src/routes/app/settings/_layout.index.tsx (3)
plugins/windows/src/ext.rs (1)
navigate(19-41)apps/desktop/src/routes/app/settings/_layout.tsx (1)
Route(49-52)apps/desktop/src/components/settings/template/index.tsx (1)
SettingsTemplates(19-117)
apps/desktop/src/components/settings/template/editor.tsx (2)
apps/desktop/src/components/settings/shared.tsx (1)
useUpdateTemplate(41-58)apps/desktop/src/components/settings/template/sections-list.tsx (1)
SectionsList(19-93)
apps/desktop/src/components/settings/template/index.tsx (3)
apps/desktop/src/contexts/search/ui.tsx (1)
useSearch(219-225)packages/db/src/schema.ts (1)
templates(100-105)apps/desktop/src/components/settings/template/editor.tsx (1)
TemplateEditor(6-47)
apps/desktop/src/devtool/seed/shared.ts (3)
apps/desktop/src/store/tinybase/persisted.ts (1)
TemplateStorage(128-128)packages/db/src/schema.ts (1)
templates(100-105)apps/desktop/src/utils/index.ts (1)
id(3-3)
apps/desktop/src/routes/app/settings/_layout.tsx (1)
apps/desktop/src/components/settings/shared.tsx (1)
useUpdateTemplate(41-58)
🪛 Biome (2.1.2)
apps/desktop/src/components/settings/template/index.tsx
[error] 38-38: filtered is assigned to itself.
This is where is assigned.
Self assignments have no effect and can be removed.
(lint/correctness/noSelfAssign)
⏰ 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 (29)
apps/desktop/src/components/main/body/sessions/floating/listen.tsx (4)
57-61: Improved Icon sizing using the appropriate prop.Converting from Tailwind className to the Icon component's native
sizeprop is the correct approach and more maintainable.
64-73: Improved Icon sizing using the appropriate prop.Converting from Tailwind className to the Icon component's native
sizeprop is the correct approach and more maintainable.
75-84: Improved Icon sizing using the appropriate prop.Converting from Tailwind className to the Icon component's native
sizeprop is the correct approach and more maintainable.
86-95: Improved Icon sizing using the appropriate prop.Converting from Tailwind className to the Icon component's native
sizeprop is the correct approach and more maintainable.apps/desktop/src/components/settings/integrations.tsx (4)
2-2: LGTM - ButtonGroup import added correctly.The import is properly specified and used in the filter controls below.
88-92: LGTM - Filter state management is well-typed.The
FilterStatustype clearly defines the three filter options, and the state is properly initialized to show all integrations by default.
96-116: Excellent implementation of combined filtering.The two-stage filtering approach is clean and efficient:
- Status filter is applied first with correct logic for all three states
- Search filter follows with case-insensitive matching on both name and description
- Dependencies in
useMemoare complete and correctThe implementation is maintainable and performs well.
132-156: LGTM - Filter controls are well-implemented.The
ButtonGroupprovides clear visual feedback with the conditional variant logic correctly showing the active filter state. The layout and spacing are appropriate, and the button labels are clear and user-friendly.apps/desktop/src/devtool/seed/shared.ts (1)
237-312: No issues found with JSON serialization approach.After examining the codebase, the
JSON.stringify()call on line 308 is correct and necessary. This codebase uses TinyBase (not Drizzle ORM), where the schema explicitly stores JSON fields as strings (sections: { type: "string" }).The
jsonObjecthelper is designed to accept either strings or objects, automatically callingJSON.parse()when the input is a string. The data flows correctly:
- Seed creates sections as an array and stringifies it before storage
- TinyBase stores the stringified JSON
- When retrieved, the
jsonObjectschema automatically parses it back to an object- The UI accesses
sectionsas an arrayThe existing pattern in the codebase is consistent—other components like
apps/desktop/src/components/settings/shared.tsx:50use the sameJSON.stringify()approach for template sections.apps/desktop/src/routes/app/settings/_layout.index.tsx (2)
17-17: LGTM!The navigate hook is correctly obtained from the router and will be used to enable template navigation flows.
26-26: LGTM!The navigate prop is correctly passed to SettingsTemplates, enabling the component to handle template-specific navigation.
apps/desktop/src/components/settings/template/editor.tsx (3)
6-6: LGTM! Simplified signature aligns with navigation-based flow.The removal of the
onCloseprop simplifies the component, with navigation now handled through the routing layer (via the back button in TemplateEditorHeader).
7-7: LGTM!The useUpdateTemplate hook correctly provides type-safe template state management.
13-46: LGTM! Clean and declarative UI.The simplified editor structure with Title, Description, and Sections is easy to understand and maintain. The inline styles and placeholders provide good UX.
apps/desktop/src/routes/app/settings/_layout.tsx (4)
2-47: LGTM!The new imports and search validation schema are correctly structured to support template editing functionality.
78-104: LGTM! Clean conditional rendering.The header correctly switches between the standard settings header and the template editor header based on the presence of a templateId.
264-287: LGTM! Well-implemented inline title editing.The title editing UX is intuitive with good keyboard support (Enter to save, blur to exit) and appropriate fallback text.
291-323: LGTM! Clean action controls.The Edit button and dropdown menu provide clear access to template actions, with appropriate styling for the destructive delete action.
apps/desktop/src/components/settings/template/index.tsx (6)
13-17: LGTM! Clear type definitions.The FilterStatus and SettingsTemplatesProps types are well-defined and support the component's navigation and filtering requirements.
54-56: LGTM!The conditional rendering correctly switches to the TemplateEditor when a templateId is present, using the updated component signature.
58-79: LGTM! Clear filter UI.The header and filter buttons provide an intuitive interface for switching between all and favorite templates.
81-90: LGTM!The search input is well-implemented with proper icon positioning and clear placeholder text.
92-116: LGTM! Good UX for empty and populated states.The empty state provides helpful feedback, and the template list correctly navigates to individual templates when clicked.
119-143: LGTM! Well-styled template card.The TemplateCard layout and styling provide a clean, clickable interface with appropriate hover states and text sizing. The use of
cn()follows the coding guidelines.As per coding guidelines.
apps/desktop/src/components/settings/template/sections-list.tsx (5)
1-17: LGTM! Clean imports and type definitions.The component dependencies and type definitions are well-organized and appropriate for the drag-and-drop functionality.
24-28: LGTM! Stable keys for reordering.Using
crypto.randomUUID()to generate stable IDs for drag-and-drop is appropriate. Since this is a Tauri desktop app, the Web Crypto API will be available in the embedded Chromium runtime.
30-59: LGTM! Well-structured change handlers.The handlers correctly manage local state and propagate changes via the
onChangecallback. The disabled check inhandleReorderproperly prevents unwanted reordering.
61-92: LGTM! Proper drag-and-drop setup.The Reorder.Group and Reorder.Item components are correctly configured with stable keys and proper event handling. The Add Section button is appropriately placed and respects the disabled state.
130-172: LGTM! Well-implemented section item controls.The drag handle and delete button with hover-based visibility provide a clean UX. The Input and Textarea components are properly configured with focus handlers and helpful placeholders.
ComputelessComputer
force-pushed
the
jj-branch-25
branch
from
77add89 to
18365ca
Compare
2 participants
No description provided.