ui polishes #1653
ui polishes #1653
Conversation
📝 WalkthroughWalkthroughThreads model-configuration awareness through chat and AI settings: adds an Changes
Sequence Diagram(s)sequenceDiagram
participant View as ChatView
participant Body as ChatBody
participant Empty as ChatBodyEmpty
participant Store as Config/Providers Store
participant Settings as Settings Panel
View->>Body: render(messages, status, isModelConfigured=!!model)
Body->>Empty: render(isModelConfigured)
alt isModelConfigured == false
Empty->>Store: evaluate provider/model/auth/config (useHasLLMModel/useHasSTTModel)
Store-->>Empty: hasModel=false + message
Empty->>View: render alert + "Go to settings" button
Empty->>Settings: open settings (on click)
else isModelConfigured == true
Empty->>Empty: render Hyprnote AI greeting + quick-action chips
Empty->>View: on quick action -> handle/log action
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
🔇 Additional comments (1)
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: 2
🧹 Nitpick comments (4)
apps/desktop/src/components/onboarding/welcome.tsx (1)
37-49: Remove commented-out code.Commented-out code should be removed since version control maintains the history. If this feature may return, consider adding a TODO comment with a tracking issue instead.
Apply this diff to remove the commented code:
- { - /*<div - className={cn([ - "flex flex-row items-center gap-1", - "text-neutral-400 transition-colors hover:text-neutral-800", - ])} - > - <button className="text-sm underline" onClick={() => onNext({ local: true })}> - Or proceed without an account - </button> - <CircleQuestionMarkIcon className="h-4 w-4 cursor-help" /> - </div>*/ - }apps/desktop/src/components/chat/body/empty.tsx (1)
16-19: TODO: Implement quick action handler.The
handleQuickActionfunction currently only logs the action. Consider implementing the actual functionality or adding a TODO comment to track this work.Do you want me to generate an implementation for the quick action handler or open a new issue to track this task?
apps/desktop/src/components/chat/input.tsx (1)
46-59: TODO: Implement file attachment, screenshot, and voice input handlers.These handlers are currently stubbed with console.log statements. Consider implementing the actual functionality or opening issues to track these features.
Do you want me to help generate implementation code for these handlers or open issues to track them?
apps/desktop/src/components/settings/ai/stt/no-model-banner.tsx (1)
39-46: Optimize query execution to run conditionally.These four model download queries execute unconditionally but are only used when
current_stt_provider === "hyprnote"(lines 59-72). For all other providers, these queries waste resources.Consider restructuring to conditionally execute these queries:
function useHasSTTModel(): { hasModel: boolean; message: string } { const { current_stt_provider, current_stt_model } = useConfigValues( ["current_stt_provider", "current_stt_model"] as const, ); const configuredProviders = main.UI.useResultTable(main.QUERIES.sttProviders, main.STORE_ID); - const [p2, p3, tinyEn, smallEn] = useQueries({ - queries: [ - sttModelQueries.isDownloaded("am-parakeet-v2"), - sttModelQueries.isDownloaded("am-parakeet-v3"), - sttModelQueries.isDownloaded("QuantizedTinyEn"), - sttModelQueries.isDownloaded("QuantizedSmallEn"), - ], - }); + const shouldCheckHyprnoteModels = current_stt_provider === "hyprnote"; + const [p2, p3, tinyEn, smallEn] = useQueries({ + queries: shouldCheckHyprnoteModels ? [ + sttModelQueries.isDownloaded("am-parakeet-v2"), + sttModelQueries.isDownloaded("am-parakeet-v3"), + sttModelQueries.isDownloaded("QuantizedTinyEn"), + sttModelQueries.isDownloaded("QuantizedSmallEn"), + ] : [], + }); if (!current_stt_provider || !current_stt_model) { return { hasModel: false, message: "Please select a provider and model" }; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (17)
apps/desktop/src-tauri/icons/nightly/128x128.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/[email protected]is excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/16.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/32x32.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/64x64.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square107x107Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square142x142Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square150x150Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square284x284Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square30x30Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square310x310Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square44x44Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square71x71Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square89x89Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/StoreLogo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/icon.icois excluded by!**/*.icoapps/desktop/src-tauri/icons/nightly/icon.pngis excluded by!**/*.png
📒 Files selected for processing (31)
apps/desktop/src/components/chat/body/empty.tsx(1 hunks)apps/desktop/src/components/chat/body/index.tsx(3 hunks)apps/desktop/src/components/chat/header.tsx(1 hunks)apps/desktop/src/components/chat/index.tsx(1 hunks)apps/desktop/src/components/chat/input.tsx(3 hunks)apps/desktop/src/components/chat/interactive.tsx(1 hunks)apps/desktop/src/components/chat/trigger.tsx(1 hunks)apps/desktop/src/components/chat/view.tsx(1 hunks)apps/desktop/src/components/main/body/index.tsx(2 hunks)apps/desktop/src/components/main/body/search.tsx(2 hunks)apps/desktop/src/components/main/body/sessions/floating/listen.tsx(1 hunks)apps/desktop/src/components/main/body/shared.tsx(2 hunks)apps/desktop/src/components/main/sidebar/banner/registry.tsx(3 hunks)apps/desktop/src/components/onboarding/calendar.tsx(0 hunks)apps/desktop/src/components/onboarding/permissions.tsx(2 hunks)apps/desktop/src/components/onboarding/shared.tsx(2 hunks)apps/desktop/src/components/onboarding/welcome.tsx(3 hunks)apps/desktop/src/components/settings/ai/llm/configure.tsx(1 hunks)apps/desktop/src/components/settings/ai/llm/index.tsx(1 hunks)apps/desktop/src/components/settings/ai/llm/no-model-banner.tsx(1 hunks)apps/desktop/src/components/settings/ai/llm/select.tsx(4 hunks)apps/desktop/src/components/settings/ai/stt/configure.tsx(2 hunks)apps/desktop/src/components/settings/ai/stt/index.tsx(1 hunks)apps/desktop/src/components/settings/ai/stt/no-model-banner.tsx(1 hunks)apps/desktop/src/components/settings/ai/stt/select.tsx(3 hunks)apps/desktop/src/components/settings/ai/stt/shared.tsx(1 hunks)apps/desktop/src/components/settings/memory/custom-vocabulary.tsx(8 hunks)apps/desktop/src/routes/app.tsx(1 hunks)apps/desktop/src/routes/app/main/_layout.index.tsx(1 hunks)apps/desktop/src/routes/app/onboarding/index.tsx(1 hunks)package.json(1 hunks)
💤 Files with no reviewable changes (1)
- apps/desktop/src/components/onboarding/calendar.tsx
🧰 Additional context used
🧬 Code graph analysis (12)
apps/desktop/src/components/settings/ai/stt/no-model-banner.tsx (4)
packages/utils/src/cn.ts (1)
cn(20-22)apps/desktop/src/config/use-config.ts (1)
useConfigValues(32-53)apps/desktop/src/components/settings/ai/stt/shared.tsx (1)
sttModelQueries(248-275)apps/desktop/src/store/tinybase/schema-internal.ts (1)
AIProviderStorage(40-40)
apps/desktop/src/components/settings/ai/llm/index.tsx (1)
apps/desktop/src/components/settings/ai/llm/no-model-banner.tsx (1)
NoModelBanner(11-31)
apps/desktop/src/components/chat/view.tsx (1)
apps/desktop/src/components/chat/input.tsx (1)
ChatMessageInput(12-135)
apps/desktop/src/components/main/body/search.tsx (3)
apps/desktop/src/contexts/search/ui.tsx (1)
useSearch(236-242)apps/desktop/src/hooks/useCmdKeyPressed.tsx (1)
useCmdKeyPressed(3-35)packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/settings/ai/llm/no-model-banner.tsx (3)
packages/utils/src/cn.ts (1)
cn(20-22)apps/desktop/src/auth.tsx (1)
useAuth(177-185)apps/desktop/src/config/use-config.ts (1)
useConfigValues(32-53)
apps/desktop/src/components/settings/ai/stt/index.tsx (1)
apps/desktop/src/components/settings/ai/stt/no-model-banner.tsx (1)
NoModelBanner(10-30)
apps/desktop/src/components/chat/input.tsx (1)
packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/chat/body/index.tsx (1)
apps/desktop/src/components/chat/body/empty.tsx (1)
ChatBodyEmpty(4-68)
apps/desktop/src/components/onboarding/permissions.tsx (2)
apps/desktop/src/components/onboarding/shared.tsx (1)
OnboardingContainer(21-43)packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/settings/ai/stt/select.tsx (2)
packages/utils/src/cn.ts (1)
cn(20-22)apps/desktop/src/components/settings/ai/stt/shared.tsx (3)
ProviderId(8-8)PROVIDERS(46-104)displayModelId(22-44)
apps/desktop/src/components/main/body/shared.tsx (1)
packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/settings/memory/custom-vocabulary.tsx (1)
packages/utils/src/cn.ts (1)
cn(20-22)
🔇 Additional comments (37)
apps/desktop/src/components/chat/trigger.tsx (1)
1-3: Import reordering looks good.The reorganization of imports aligns with the PR's broader effort to standardize imports across chat components. The component logic remains sound and unchanged.
apps/desktop/src/components/onboarding/welcome.tsx (1)
29-35: LGTM! Button styling improvements enhance the CTA.The full-width button with large size is appropriate for a welcome screen's primary action.
apps/desktop/src/routes/app.tsx (1)
1-1: LGTM! Import organization improvement.Moving the TooltipProvider import to the top of the file improves code organization and readability.
apps/desktop/src/routes/app/main/_layout.index.tsx (1)
19-19: LGTM! Padding simplified to uniform value.The change from
pl-1 pr-0 pt-1 pb-1top-1adds right padding (pr-1) where there was previously none (pr-0), creating uniform padding on all sides. This appears to be an intentional layout simplification aligned with the broader UI refinements in this PR.apps/desktop/src/components/main/body/index.tsx (1)
79-79: LGTM! Consistent icon color styling.The addition of
text-neutral-600to both the PanelLeftOpenIcon and the button with PlusIcon provides consistent visual styling across the UI. These are purely cosmetic changes with no behavioral impact.Also applies to: 154-156
apps/desktop/src/components/settings/ai/llm/configure.tsx (1)
1-3: LGTM! Duplicate imports eliminated.Consolidating the Accordion components and
cnimports at the top of the file eliminates duplication and follows standard import organization practices.apps/desktop/src/components/chat/index.tsx (1)
1-2: LGTM! Import moved to top.The
windowsCommandsimport has been appropriately relocated to the top of the file, improving code organization.apps/desktop/src/components/main/body/sessions/floating/listen.tsx (1)
1-6: LGTM! Imports consolidated at top.Multiple imports (miscCommands, windowsCommands, Button, Popover, Spinner, Tooltip) have been properly relocated to the top of the file, improving code organization and readability.
apps/desktop/src/routes/app/onboarding/index.tsx (1)
41-42: LGTM! Onboarding layout restructured.The layout has been restructured to use a flex column container with uniform padding and an absolute-positioned drag region at the top. This provides better structure for the onboarding flow and ensures the drag region is properly positioned for window interactions in the Tauri application.
package.json (1)
5-5: No breaking changes detected in [email protected] upgrade.There are no new breaking changes listed for pnpm 10.20 or 10.21. The update from [email protected] to [email protected] consists only of minor version bumps with additive features (Node.js runtime installation support, trustPolicy setting) and patch fixes. These changes are backward compatible and won't impact the existing build or development workflow.
apps/desktop/src/components/settings/ai/stt/shared.tsx (1)
1-2: LGTM! Import consolidation improves code organization.The local-stt imports are now at the top of the file, eliminating duplicate imports later in the file. This improves readability and follows best practices.
apps/desktop/src/components/chat/interactive.tsx (1)
1-2: LGTM! Import consolidation aligns with PR-wide refactoring.Moving the
cnimport to the top of the file removes duplicate imports and improves consistency across the codebase.apps/desktop/src/components/settings/ai/stt/configure.tsx (2)
267-267: LGTM! Visual enhancement for progress overlay.The semi-transparent black background (
bg-black/50) improves contrast and readability during download progress.
379-383: LGTM! Proper loading state handling prevents premature reset.The additional check for
!isDownloading.isLoadingprevents the task state from being reset while the query is still loading its initial state. The dependency array is correctly updated to includeisDownloading.isLoading.apps/desktop/src/components/main/sidebar/banner/registry.tsx (3)
1-1: LGTM! Icons better represent their respective features.
AudioLinesIconis more semantically appropriate for STT (speech-to-text), andSparklesIconbetter represents LLM capabilities compared to the previous icons.
32-36: LGTM! Banner content is clearer and more actionable.The updated title "Configure STT model" and description clearly communicate what's needed. The action label "Go to settings" is concise and direct.
46-50: LGTM! Consistent improvements to LLM banner.The updates mirror the STT banner improvements with clear, actionable content and consistent styling.
apps/desktop/src/components/main/body/search.tsx (2)
61-70: LGTM! Button component adoption improves consistency.Replacing the native button with the UI
Buttoncomponent provides consistent styling across the application. The icon sizing updates using Tailwind classes (size-4) are cleaner than explicit width/height props.
78-91: LGTM! Responsive behavior enhances UX on narrow screens.The
hasSpacecondition ensures the search input maintains optimal width on narrow screens (alwaysw-[250px]), while allowing dynamic width changes on wider screens. The computedwidthvariable is clearer than inline conditionals.apps/desktop/src/components/main/body/shared.tsx (1)
45-156: LGTM! Hover interactions enhance tab UX.The hover state management adds polished visual feedback:
- Icons transition to close buttons on hover
- Active tabs show a pulsing red dot indicator
- Smooth opacity transitions create professional feel
The implementation correctly prevents event bubbling for the close buttons and maintains proper z-index layering for the overlays.
apps/desktop/src/components/chat/body/empty.tsx (2)
4-40: LGTM! Configuration-aware rendering improves onboarding.The
isModelConfiguredprop enables dynamic empty state rendering. When unconfigured, the component guides users to settings with a clear call-to-action. The default value oftruemaintains backward compatibility.
42-67: LGTM! Enhanced empty state provides better user guidance.The quick action buttons provide clear starting points for users. The visual design with the Hyprnote branding and animated icon creates a welcoming experience.
apps/desktop/src/components/settings/ai/llm/no-model-banner.tsx (2)
11-31: LGTM! Banner provides clear visual feedback for missing configuration.The red alert banner with icon effectively communicates configuration issues to users. Conditional rendering ensures it only appears when needed.
33-72: LGTM! Comprehensive validation logic with clear messaging.The hook systematically checks all configuration requirements:
- Provider and model selection
- Provider existence in registry
- Authentication for "hyprnote" provider
- Provider configuration (base URL)
- API key requirements
Each validation step provides a clear, actionable message. The
useMemooptimization with correct dependencies prevents unnecessary recalculations.apps/desktop/src/components/settings/ai/llm/index.tsx (1)
2-2: LGTM!The addition of the NoModelBanner component is clean and integrates well with the existing LLM settings UI.
Also applies to: 8-8
apps/desktop/src/components/settings/ai/stt/index.tsx (1)
2-2: LGTM!The NoModelBanner integration mirrors the LLM settings implementation, maintaining consistency across AI settings.
Also applies to: 8-8
apps/desktop/src/components/chat/header.tsx (1)
1-3: LGTM!The import reorganization improves code structure by consolidating related imports.
apps/desktop/src/components/chat/view.tsx (2)
107-107: LGTM!The isModelConfigured prop properly propagates model configuration state to ChatBody, enabling model-aware UI rendering.
110-110: LGTM!The simplified disabled logic correctly prevents input when the model is unavailable or the chat is not ready.
apps/desktop/src/components/chat/input.tsx (3)
35-44: LGTM!The useEffect properly guards against uninitialized or destroyed editor instances before focusing.
63-132: LGTM!The UI restructure provides a comprehensive input interface with attachment, screenshot, voice, and send controls. The disabled state handling and visual feedback are well-implemented.
154-159: LGTM!The ChatPlaceholder implementation correctly uses the PlaceholderFunction type and provides contextual placeholder text.
apps/desktop/src/components/settings/ai/stt/no-model-banner.tsx (2)
10-30: LGTM!The NoModelBanner component provides clear visual feedback when no STT model is configured.
48-89: LGTM!The validation logic provides comprehensive checks with clear, actionable error messages for different scenarios.
apps/desktop/src/components/chat/body/index.tsx (3)
1-1: LGTM!The cn import consolidation improves code organization.
17-17: LGTM!The isModelConfigured prop is properly typed with a sensible default that maintains backward compatibility.
Also applies to: 24-24
39-39: LGTM!The layout changes with the spacer div properly align the chat content, and the isModelConfigured prop propagation enables model-aware empty state rendering.
Also applies to: 43-43, 45-45
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 (2)
apps/desktop/src/components/settings/ai/llm/select.tsx(4 hunks)apps/desktop/src/components/settings/ai/stt/select.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/components/settings/ai/llm/select.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
apps/desktop/src/components/settings/ai/stt/select.tsx (2)
packages/utils/src/cn.ts (1)
cn(20-22)apps/desktop/src/components/settings/ai/stt/shared.tsx (3)
ProviderId(8-8)PROVIDERS(46-104)displayModelId(22-44)
| setTimeout(() => { | ||
| form.setFieldValue("model", availableModels[0].id); | ||
| }, 0); |
There was a problem hiding this comment.
Prevent stale auto-selection when provider toggles quickly.
Queuing the auto-fill with setTimeout lets the callback fire even after the user jumps to another provider (e.g., Custom). That leaves us persisting a Deepgram model ID while current_stt_provider is already Custom, breaking STT configuration. Guard the deferred update (or make it synchronous) so we only seed the model when the provider still matches.
Apply this diff to guard against the stale-provider race:
- if (availableModels.length > 0) {
- setTimeout(() => {
- form.setFieldValue("model", availableModels[0].id);
- }, 0);
- }
+ if (availableModels.length > 0) {
+ const [firstModel] = availableModels;
+ setTimeout(() => {
+ if (form.getFieldValue("provider") !== providerId) {
+ return;
+ }
+ form.setFieldValue("model", firstModel.id);
+ }, 0);
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/desktop/src/components/settings/ai/stt/select.tsx around lines 80 to 82,
the deferred setTimeout that auto-selects availableModels[0].id can run after
the user has switched providers and thus seed a stale Deepgram model; change the
logic so the deferred update is guarded or made synchronous: either remove the
setTimeout and set the model immediately when the provider change handler runs,
or if you must defer, store the provider expected at scheduling time and inside
the timeout verify form.getFieldValue("current_stt_provider") (or equivalent
state) still equals that expected provider before calling
form.setFieldValue("model", ...); also ensure any previously scheduled timeout
is cleared/cancelled when the provider changes rapidly.
ComputelessComputer
force-pushed
the
john-desktop-wip
branch
2 times, most recently
from
dcfbf03 to
1345b59
Compare
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/components/settings/ai/llm/select.tsx (1)
192-194: Do not log API credentials.This
console.logprints the provider’s API key to the developer console. Logging secrets is a critical security leak—remove the log (or, if absolutely needed, omit the key).- console.log(provider.id, baseUrl, apiKey);
♻️ Duplicate comments (1)
apps/desktop/src/components/settings/ai/stt/select.tsx (1)
74-83: Guard the auto-select against stale providers.The deferred
setTimeoutcan still run after the user switches providers, seeding a model from the previous provider (e.g., Hyprnote → Deepgram ends up with a Hyprnote model). Keep the provider/model pair consistent by verifying the provider inside the timeout before writing.if (availableModels.length > 0) { - setTimeout(() => { - form.setFieldValue("model", availableModels[0].id); - }, 0); + const [firstModel] = availableModels; + setTimeout(() => { + if (form.getFieldValue("provider") !== providerId) { + return; + } + form.setFieldValue("model", firstModel.id); + }, 0);
🧹 Nitpick comments (4)
apps/desktop/src/components/chat/input.tsx (1)
46-59: Verify TODO placeholders are intentional for this PR.Three action handlers (attach file, screenshot, voice input) are stubbed with console.log statements. Ensure these placeholders align with the PR's "ui polishes" scope.
Do these TODO implementations need to be tracked in a follow-up issue?
apps/desktop/src/components/chat/body/empty.tsx (2)
9-19: Quick actions are hardcoded.Consider whether the
quickActionsarray should be configurable or derived from available features rather than being hardcoded.If these actions will vary based on context or user preferences, consider making them props or deriving them from configuration.
16-19: Verify console.log placeholder is intentional.The
handleQuickActionfunction only logs the action. Ensure this placeholder aligns with the PR's scope, similar to the TODO handlers ininput.tsx.Do these placeholders need to be implemented in this PR or tracked separately?
apps/desktop/src/components/settings/memory/custom-vocabulary.tsx (1)
194-222: LGTM! Item numbering implementation is clean.The item number display with conditional visibility during search provides good UX. The layout structure clearly separates the numeric indicator from the content.
Optional: The conditional class on line 195 could be simplified to:
- <span className={cn(["text-sm text-neutral-400 w-4 flex-shrink-0 text-center", isSearching && "invisible"])}> + <span className={cn("text-sm text-neutral-400 w-4 flex-shrink-0 text-center", isSearching && "invisible")}>
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (17)
apps/desktop/src-tauri/icons/nightly/128x128.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/[email protected]is excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/16.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/32x32.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/64x64.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square107x107Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square142x142Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square150x150Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square284x284Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square30x30Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square310x310Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square44x44Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square71x71Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square89x89Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/StoreLogo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/icon.icois excluded by!**/*.icoapps/desktop/src-tauri/icons/nightly/icon.pngis excluded by!**/*.png
📒 Files selected for processing (31)
apps/desktop/src/components/chat/body/empty.tsx(1 hunks)apps/desktop/src/components/chat/body/index.tsx(3 hunks)apps/desktop/src/components/chat/header.tsx(1 hunks)apps/desktop/src/components/chat/index.tsx(1 hunks)apps/desktop/src/components/chat/input.tsx(3 hunks)apps/desktop/src/components/chat/interactive.tsx(1 hunks)apps/desktop/src/components/chat/trigger.tsx(1 hunks)apps/desktop/src/components/chat/view.tsx(1 hunks)apps/desktop/src/components/main/body/index.tsx(5 hunks)apps/desktop/src/components/main/body/search.tsx(2 hunks)apps/desktop/src/components/main/body/sessions/floating/listen.tsx(1 hunks)apps/desktop/src/components/main/body/shared.tsx(2 hunks)apps/desktop/src/components/main/sidebar/banner/registry.tsx(3 hunks)apps/desktop/src/components/onboarding/calendar.tsx(0 hunks)apps/desktop/src/components/onboarding/permissions.tsx(2 hunks)apps/desktop/src/components/onboarding/shared.tsx(2 hunks)apps/desktop/src/components/onboarding/welcome.tsx(3 hunks)apps/desktop/src/components/settings/ai/llm/configure.tsx(1 hunks)apps/desktop/src/components/settings/ai/llm/index.tsx(1 hunks)apps/desktop/src/components/settings/ai/llm/no-model-banner.tsx(1 hunks)apps/desktop/src/components/settings/ai/llm/select.tsx(3 hunks)apps/desktop/src/components/settings/ai/stt/configure.tsx(2 hunks)apps/desktop/src/components/settings/ai/stt/index.tsx(1 hunks)apps/desktop/src/components/settings/ai/stt/no-model-banner.tsx(1 hunks)apps/desktop/src/components/settings/ai/stt/select.tsx(3 hunks)apps/desktop/src/components/settings/ai/stt/shared.tsx(1 hunks)apps/desktop/src/components/settings/memory/custom-vocabulary.tsx(8 hunks)apps/desktop/src/routes/app.tsx(1 hunks)apps/desktop/src/routes/app/main/_layout.index.tsx(1 hunks)apps/desktop/src/routes/app/onboarding/index.tsx(1 hunks)package.json(1 hunks)
💤 Files with no reviewable changes (1)
- apps/desktop/src/components/onboarding/calendar.tsx
🚧 Files skipped from review as they are similar to previous changes (15)
- apps/desktop/src/components/settings/ai/llm/configure.tsx
- apps/desktop/src/components/settings/ai/stt/index.tsx
- apps/desktop/src/components/settings/ai/llm/index.tsx
- apps/desktop/src/routes/app/main/_layout.index.tsx
- apps/desktop/src/components/settings/ai/stt/configure.tsx
- apps/desktop/src/components/onboarding/shared.tsx
- package.json
- apps/desktop/src/components/onboarding/welcome.tsx
- apps/desktop/src/components/chat/body/index.tsx
- apps/desktop/src/components/main/sidebar/banner/registry.tsx
- apps/desktop/src/components/main/body/sessions/floating/listen.tsx
- apps/desktop/src/components/chat/header.tsx
- apps/desktop/src/routes/app.tsx
- apps/desktop/src/components/chat/interactive.tsx
- apps/desktop/src/components/settings/ai/stt/shared.tsx
🧰 Additional context used
🧬 Code graph analysis (11)
apps/desktop/src/components/settings/ai/llm/no-model-banner.tsx (3)
packages/utils/src/cn.ts (1)
cn(20-22)apps/desktop/src/auth.tsx (1)
useAuth(177-185)apps/desktop/src/config/use-config.ts (1)
useConfigValues(32-53)
apps/desktop/src/components/main/body/index.tsx (1)
plugins/windows/src/ext.rs (1)
close(56-62)
apps/desktop/src/components/main/body/shared.tsx (1)
packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/chat/view.tsx (1)
apps/desktop/src/components/chat/input.tsx (1)
ChatMessageInput(12-135)
apps/desktop/src/components/settings/ai/llm/select.tsx (3)
packages/utils/src/cn.ts (1)
cn(20-22)apps/desktop/src/components/settings/ai/llm/shared.tsx (1)
PROVIDERS(6-63)apps/desktop/src/components/settings/ai/shared/model-combobox.tsx (1)
ModelCombobox(45-233)
apps/desktop/src/components/settings/ai/stt/no-model-banner.tsx (4)
packages/utils/src/cn.ts (1)
cn(20-22)apps/desktop/src/config/use-config.ts (1)
useConfigValues(32-53)apps/desktop/src/components/settings/ai/stt/shared.tsx (1)
sttModelQueries(248-275)apps/desktop/src/store/tinybase/schema-internal.ts (1)
AIProviderStorage(40-40)
apps/desktop/src/components/main/body/search.tsx (3)
apps/desktop/src/contexts/search/ui.tsx (1)
useSearch(236-242)apps/desktop/src/hooks/useCmdKeyPressed.tsx (1)
useCmdKeyPressed(3-35)packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/onboarding/permissions.tsx (2)
apps/desktop/src/components/onboarding/shared.tsx (1)
OnboardingContainer(21-43)packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/settings/memory/custom-vocabulary.tsx (1)
packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/chat/input.tsx (1)
packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/settings/ai/stt/select.tsx (2)
packages/utils/src/cn.ts (1)
cn(20-22)apps/desktop/src/components/settings/ai/stt/shared.tsx (3)
ProviderId(8-8)PROVIDERS(46-104)displayModelId(22-44)
🔇 Additional comments (24)
apps/desktop/src/components/chat/trigger.tsx (1)
3-4: LGTM! Fixes missing import.This adds the necessary import for
createPortalwhich is already being used in the component (line 6). Without this import, the component would throw a ReferenceError at runtime.apps/desktop/src/routes/app/onboarding/index.tsx (1)
41-42: Verify drag region remains interactive if content overlaps.The separation of the drag region from the content container is a good pattern. However, without an explicit
z-indexon the drag region, tall content that extends into the top 56px could overlap and potentially obscure the draggable area depending on stacking context.Consider adding
z-index: 10(or similar) to the drag region div to ensure it remains interactive:- <div data-tauri-drag-region className="h-14 w-full absolute top-0 left-0 right-0" /> + <div data-tauri-drag-region className="h-14 w-full absolute top-0 left-0 right-0 z-10" />Alternatively, verify that the onboarding content is never tall enough to extend into the top region.
apps/desktop/src/components/chat/index.tsx (1)
1-2: LGTM! Clean import organization.Relocating the import to the top improves readability and eliminates the duplicate import that was likely present later in the file.
apps/desktop/src/components/chat/input.tsx (3)
41-44: Focus management looks correct.The conditional focus logic properly prevents focusing the editor when disabled, avoiding potential UI issues.
63-132: Consistent action bar UI with proper disabled states.The button layout and disabled styling are well-structured. The use of
cn()with conditional classes ensures consistent visual feedback.
154-159: Placeholder component implementation is correct.The
ChatPlaceholderfunction properly implements thePlaceholderFunctionsignature and only renders for the first paragraph node.apps/desktop/src/components/chat/view.tsx (2)
107-107: Configuration awareness properly threaded.The
isModelConfiguredprop correctly derives from the model's existence and will enable the empty state to render configuration-aware UI.
110-110: Simplified disabled logic is cleaner.The boolean expression
!model || status !== "ready"is more maintainable than the previous conditional object structure, while still accepting the union type defined inChatMessageInput.apps/desktop/src/components/chat/body/empty.tsx (3)
4-4: Prop signature enables configuration-aware rendering.The
isModelConfiguredprop with a sensible default allows the component to gracefully handle both configured and unconfigured states.
21-40: Configuration prompt UI looks good.The unconfigured state properly guides users to settings with clear messaging and an actionable button.
27-27: Asset path verified and valid.The
dynamic.giffile exists atapps/desktop/public/assets/dynamic.gif, which is correctly referenced by the path/assets/dynamic.gifin the code. No action required.apps/desktop/src/components/main/body/index.tsx (3)
1-2: LGTM! Import consolidation aligns with UI standardization.The import changes replace local/alternative components with standardized UI components from
@hypr/uiand utilities from@hypr/utils, which is consistent with the broader UI polish objectives of this PR.
374-385: Simplified mod+n behavior looks correct.The updated logic creates a note in the current tab when it's empty, otherwise creates a new note. The dependency array correctly includes
currentTab,newNote, andnewNoteCurrent.
394-403: Verify that removing app window close is intentional.The mod+w shortcut no longer closes the application window when the last tab is closed. This is a user-facing behavior change that some users might find unexpected if they're accustomed to closing the app with mod+w.
Please confirm this is the intended behavior.
apps/desktop/src/components/main/body/search.tsx (2)
56-72: LGTM! Button component provides better consistency.The refactor replaces a native button with the standardized
Buttoncomponent and updates icon sizing to use Tailwind'ssize-4utility class. This aligns with the PR's UI polish objectives and improves consistency across the codebase.
74-92: Responsive width logic enhances narrow-screen UX.The responsive behavior ensures the search input is appropriately sized on both wide and narrow screens. On narrow screens, the search bar maintains full width when expanded, while on wide screens it dynamically adjusts based on focus state.
apps/desktop/src/components/main/body/shared.tsx (4)
45-45: LGTM! Clean hover state implementation.The hover state tracking using a wrapper div is a good approach for managing the tab item's hover effects.
Also applies to: 68-72
73-91: InteractiveButton refactor maintains functionality with enhanced structure.The refactored structure uses the
asChildpattern and adds transition classes for smooth hover effects. The border and color logic for active/selected states is correctly preserved.
92-144: Well-implemented hover-driven close button overlay.The overlay logic smoothly transitions between showing the tab icon/indicator and the close button on hover. Key aspects:
- Correct use of
stopPropagationprevents unintended tab selection when clicking close- Smooth opacity transitions between normal and hover states
- Different visual treatments for active vs inactive tabs enhance clarity
The nested interactive structure is acceptable here since event propagation is properly managed.
147-154: Keyboard shortcut positioning updated correctly.The shortcut badge positioning and styling adapt to the new layout, with
pointer-events-noneensuring it doesn't interfere with interactions.apps/desktop/src/components/onboarding/permissions.tsx (2)
1-28: LGTM!The imports, hook usage, and permission status logic are well-structured and correct.
130-134: LGTM!The continue button correctly enforces that all permissions must be granted before proceeding, and provides clear feedback to the user about what's needed.
apps/desktop/src/components/settings/memory/custom-vocabulary.tsx (2)
50-55: LGTM! Clean header simplification.The streamlined header with just title and description improves clarity and aligns well with the UI polish objectives.
64-90: LGTM! Search UI enhancements improve usability.The addition of the Search icon and styling updates create a more polished search experience. The Add button now has more visual emphasis without the ghost variant.
| <p className="text-sm text-neutral-700 mb-2"> | ||
| Hey! I can help you a lot of cool stuff :) | ||
| </p> |
There was a problem hiding this comment.
Fix grammar in greeting message.
The text "I can help you a lot of cool stuff" is grammatically incorrect.
Apply this diff:
- Hey! I can help you a lot of cool stuff :)
+ Hey! I can help you with a lot of cool stuff :)📝 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.
| <p className="text-sm text-neutral-700 mb-2"> | |
| Hey! I can help you a lot of cool stuff :) | |
| </p> | |
| <p className="text-sm text-neutral-700 mb-2"> | |
| Hey! I can help you with a lot of cool stuff :) | |
| </p> |
🤖 Prompt for AI Agents
In apps/desktop/src/components/chat/body/empty.tsx around lines 50 to 52, the
greeting text "Hey! I can help you a lot of cool stuff :)" is grammatically
incorrect; update the string to a correct phrasing such as "Hey! I can help you
with a lot of cool things :)" (i.e., insert "with" and change "stuff" to
"things" or use "a lot of cool stuff" with "with": "I can help you with a lot of
cool stuff") so the sentence reads naturally.
| <div className="flex items-center justify-between"> | ||
| <div className="gap-2"> | ||
| <div | ||
| className={cn([ | ||
| "flex items-center gap-2", | ||
| micPermissionStatus.data !== "authorized" ? "text-red-500" : "text-neutral-900", | ||
| ])} | ||
| > | ||
| {micPermissionStatus.data !== "authorized" && <AlertCircleIcon className="size-4" />} | ||
| <span className="text-base font-medium">Microphone</span> | ||
| </div> | ||
| <p className="text-sm text-neutral-500"> | ||
| {micPermissionStatus.data === "authorized" ? "Good to go :)" : "To capture your voice"} | ||
| </p> | ||
| </div> | ||
| <Button | ||
| variant={micPermissionStatus.data === "authorized" ? "outline" : "default"} | ||
| size="icon" | ||
| onClick={handleMicPermissionAction} | ||
| disabled={micPermission.isPending || micPermissionStatus.data === "authorized"} | ||
| className={cn(["size-8", micPermissionStatus.data === "authorized" && "bg-stone-100 text-stone-800"])} | ||
| > | ||
| {micPermissionStatus.data === "authorized" | ||
| ? <CheckIcon className="size-5" /> | ||
| : <ArrowRightIcon className="size-5" />} | ||
| </Button> | ||
| </div> | ||
|
|
||
| {/* System Audio */} | ||
| <div className="flex items-center justify-between"> | ||
| <div className="gap-2"> | ||
| <div | ||
| className={cn([ | ||
| "flex items-center gap-2", | ||
| systemAudioPermissionStatus.data !== "authorized" ? "text-red-500" : "text-neutral-900", | ||
| ])} | ||
| > | ||
| {systemAudioPermissionStatus.data !== "authorized" && <AlertCircleIcon className="size-4" />} | ||
| <span className="text-base font-medium">System audio</span> | ||
| </div> | ||
| <p className="text-sm text-neutral-500"> | ||
| {systemAudioPermissionStatus.data === "authorized" | ||
| ? "Good to go :)" | ||
| : "To capture what other people are saying"} | ||
| </p> | ||
| </div> | ||
| <Button | ||
| variant={systemAudioPermissionStatus.data === "authorized" ? "outline" : "default"} | ||
| size="icon" | ||
| onClick={handleSystemAudioPermissionAction} | ||
| disabled={systemAudioPermission.isPending || systemAudioPermissionStatus.data === "authorized"} | ||
| className={cn([ | ||
| "size-8", | ||
| systemAudioPermissionStatus.data === "authorized" && "bg-stone-100 text-stone-800", | ||
| ])} | ||
| > | ||
| {systemAudioPermissionStatus.data === "authorized" | ||
| ? <CheckIcon className="size-5" /> | ||
| : <ArrowRightIcon className="size-5" />} | ||
| </Button> | ||
| </div> | ||
|
|
||
| {/* Accessibility */} | ||
| <div className="flex items-center justify-between"> | ||
| <div className="gap-2"> | ||
| <div | ||
| className={cn([ | ||
| "flex items-center gap-2", | ||
| accessibilityPermissionStatus.data !== "authorized" ? "text-red-500" : "text-neutral-900", | ||
| ])} | ||
| > | ||
| {accessibilityPermissionStatus.data !== "authorized" && <AlertCircleIcon className="size-4" />} | ||
| <span className="text-base font-medium">Accessibility</span> | ||
| </div> | ||
| <p className="text-sm text-neutral-500"> | ||
| {accessibilityPermissionStatus.data === "authorized" | ||
| ? "Good to go :)" | ||
| : "To sync mic inputs & mute from meetings"} | ||
| </p> | ||
| </div> | ||
| <Button | ||
| variant={accessibilityPermissionStatus.data === "authorized" ? "outline" : "default"} | ||
| size="icon" | ||
| onClick={handleAccessibilityPermissionAction} | ||
| disabled={accessibilityPermission.isPending || accessibilityPermissionStatus.data === "authorized"} | ||
| className={cn([ | ||
| "size-8", | ||
| accessibilityPermissionStatus.data === "authorized" && "bg-stone-100 text-stone-800", | ||
| ])} | ||
| > | ||
| {accessibilityPermissionStatus.data === "authorized" | ||
| ? <CheckIcon className="size-5" /> | ||
| : <ArrowRightIcon className="size-5" />} | ||
| </Button> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Extract repeated permission blocks into a reusable component.
All three permission blocks follow an identical pattern with only the permission name, status, handler, and descriptions varying. This duplication makes the code harder to maintain and more error-prone.
Consider extracting a PermissionBlock component:
type PermissionBlockProps = {
name: string;
status: string | undefined;
description: { authorized: string; unauthorized: string };
isPending: boolean;
onAction: () => void;
};
function PermissionBlock({ name, status, description, isPending, onAction }: PermissionBlockProps) {
const isAuthorized = status === "authorized";
return (
<div className="flex items-center justify-between">
<div className="flex flex-col gap-2">
<div className={cn([
"flex items-center gap-2",
!isAuthorized ? "text-red-500" : "text-neutral-900",
])}>
{!isAuthorized && <AlertCircleIcon className="size-4" />}
<span className="text-base font-medium">{name}</span>
</div>
<p className="text-sm text-neutral-500">
{isAuthorized ? description.authorized : description.unauthorized}
</p>
</div>
<Button
variant={isAuthorized ? "outline" : "default"}
size="icon"
onClick={onAction}
disabled={isPending || isAuthorized}
className={cn(["size-8", isAuthorized && "bg-stone-100 text-stone-800"])}
aria-label={isAuthorized ? `${name} permission granted` : `Request ${name.toLowerCase()} permission`}
>
{isAuthorized ? <CheckIcon className="size-5" /> : <ArrowRightIcon className="size-5" />}
</Button>
</div>
);
}Then use it like:
<PermissionBlock
name="Microphone"
status={micPermissionStatus.data}
description={{ authorized: "Good to go :)", unauthorized: "To capture your voice" }}
isPending={micPermission.isPending}
onAction={handleMicPermissionAction}
/>🤖 Prompt for AI Agents
In apps/desktop/src/components/onboarding/permissions.tsx around lines 33 to
127, there is repeated JSX for three permission blocks (Microphone, System
audio, Accessibility); extract a reusable PermissionBlock component that accepts
props for name, status, description (authorized/unauthorized), isPending, and
onAction, then replace each repeated block with that component to remove
duplication. Implement the component to compute isAuthorized from status, render
the label/icon/text conditionally, set Button props (variant, size, onClick,
disabled, className) based on isAuthorized and isPending, and include an
accessible aria-label; finally update the three usages to pass the appropriate
values (names, handlers, statuses, descriptions, and isPending flags).
| <div className="gap-2"> | ||
| <div | ||
| className={cn([ | ||
| "flex items-center gap-2", | ||
| micPermissionStatus.data !== "authorized" ? "text-red-500" : "text-neutral-900", | ||
| ])} | ||
| > | ||
| {micPermissionStatus.data !== "authorized" && <AlertCircleIcon className="size-4" />} | ||
| <span className="text-base font-medium">Microphone</span> | ||
| </div> | ||
| <p className="text-sm text-neutral-500"> | ||
| {micPermissionStatus.data === "authorized" ? "Good to go :)" : "To capture your voice"} | ||
| </p> | ||
| </div> |
There was a problem hiding this comment.
Fix layout: gap-2 requires flex or grid container.
The gap-2 utility on line 34 has no effect because the div isn't using display: flex or display: grid.
Apply this diff to fix the layout:
- <div className="flex items-center justify-between">
- <div className="gap-2">
+ <div className="flex items-center justify-between">
+ <div className="flex flex-col gap-2">Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/desktop/src/components/onboarding/permissions.tsx around lines 34 to 47,
the outer div uses the Tailwind gap-2 utility but isn't a flex or grid container
so the gap has no effect; update the div's className to include a display
utility (e.g., add "flex flex-col" or "grid") so gap-2 works as intended (for
example change className to "flex flex-col gap-2" to keep vertical stacking and
spacing).
| <Button | ||
| variant={micPermissionStatus.data === "authorized" ? "outline" : "default"} | ||
| size="icon" | ||
| onClick={handleMicPermissionAction} | ||
| disabled={micPermission.isPending || micPermissionStatus.data === "authorized"} | ||
| className={cn(["size-8", micPermissionStatus.data === "authorized" && "bg-stone-100 text-stone-800"])} | ||
| > | ||
| {micPermissionStatus.data === "authorized" | ||
| ? <CheckIcon className="size-5" /> | ||
| : <ArrowRightIcon className="size-5" />} | ||
| </Button> |
There was a problem hiding this comment.
Add aria-label for screen reader accessibility.
The icon-only button should include an aria-label to ensure screen readers can convey the button's purpose to users.
Apply this diff:
<Button
variant={micPermissionStatus.data === "authorized" ? "outline" : "default"}
size="icon"
onClick={handleMicPermissionAction}
disabled={micPermission.isPending || micPermissionStatus.data === "authorized"}
className={cn(["size-8", micPermissionStatus.data === "authorized" && "bg-stone-100 text-stone-800"])}
+ aria-label={micPermissionStatus.data === "authorized" ? "Microphone permission granted" : "Request microphone permission"}
>📝 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.
| <Button | |
| variant={micPermissionStatus.data === "authorized" ? "outline" : "default"} | |
| size="icon" | |
| onClick={handleMicPermissionAction} | |
| disabled={micPermission.isPending || micPermissionStatus.data === "authorized"} | |
| className={cn(["size-8", micPermissionStatus.data === "authorized" && "bg-stone-100 text-stone-800"])} | |
| > | |
| {micPermissionStatus.data === "authorized" | |
| ? <CheckIcon className="size-5" /> | |
| : <ArrowRightIcon className="size-5" />} | |
| </Button> | |
| <Button | |
| variant={micPermissionStatus.data === "authorized" ? "outline" : "default"} | |
| size="icon" | |
| onClick={handleMicPermissionAction} | |
| disabled={micPermission.isPending || micPermissionStatus.data === "authorized"} | |
| className={cn(["size-8", micPermissionStatus.data === "authorized" && "bg-stone-100 text-stone-800"])} | |
| aria-label={micPermissionStatus.data === "authorized" ? "Microphone permission granted" : "Request microphone permission"} | |
| > | |
| {micPermissionStatus.data === "authorized" | |
| ? <CheckIcon className="size-5" /> | |
| : <ArrowRightIcon className="size-5" />} | |
| </Button> |
🤖 Prompt for AI Agents
In apps/desktop/src/components/onboarding/permissions.tsx around lines 48 to 58,
the icon-only Button is missing an aria-label which prevents screen readers from
conveying its purpose; add an aria-label prop to the Button that changes with
micPermissionStatus.data (for example when authorized use "Microphone permission
granted" or "Microphone enabled", otherwise use "Grant microphone permission" or
"Enable microphone") so the label clearly describes the action/state for
assistive tech.
| <div className="gap-2"> | ||
| <div | ||
| className={cn([ | ||
| "flex items-center gap-2", | ||
| systemAudioPermissionStatus.data !== "authorized" ? "text-red-500" : "text-neutral-900", | ||
| ])} | ||
| > | ||
| {systemAudioPermissionStatus.data !== "authorized" && <AlertCircleIcon className="size-4" />} | ||
| <span className="text-base font-medium">System audio</span> | ||
| </div> | ||
| <p className="text-sm text-neutral-500"> | ||
| {systemAudioPermissionStatus.data === "authorized" | ||
| ? "Good to go :)" | ||
| : "To capture what other people are saying"} | ||
| </p> | ||
| </div> |
There was a problem hiding this comment.
Fix layout: gap-2 requires flex or grid container.
Same issue as the Microphone block—the gap-2 utility has no effect without flex or grid.
Apply this diff:
- <div className="flex items-center justify-between">
- <div className="gap-2">
+ <div className="flex items-center justify-between">
+ <div className="flex flex-col gap-2">Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/desktop/src/components/onboarding/permissions.tsx around lines 63 to 78,
the outer div uses the Tailwind utility gap-2 but is not a flex or grid
container so gap has no effect; update the div's className to make it a flex
column (e.g., change "gap-2" to "flex flex-col gap-2") so spacing between the
title and description works as intended, matching the Microphone block layout.
| <Button | ||
| variant={systemAudioPermissionStatus.data === "authorized" ? "outline" : "default"} | ||
| size="icon" | ||
| onClick={handleSystemAudioPermissionAction} | ||
| disabled={systemAudioPermission.isPending || systemAudioPermissionStatus.data === "authorized"} | ||
| className={cn([ | ||
| "size-8", | ||
| systemAudioPermissionStatus.data === "authorized" && "bg-stone-100 text-stone-800", | ||
| ])} | ||
| > | ||
| {systemAudioPermissionStatus.data === "authorized" | ||
| ? <CheckIcon className="size-5" /> | ||
| : <ArrowRightIcon className="size-5" />} | ||
| </Button> |
There was a problem hiding this comment.
Add aria-label for screen reader accessibility.
The icon-only button needs an aria-label.
Apply this diff:
<Button
variant={systemAudioPermissionStatus.data === "authorized" ? "outline" : "default"}
size="icon"
onClick={handleSystemAudioPermissionAction}
disabled={systemAudioPermission.isPending || systemAudioPermissionStatus.data === "authorized"}
className={cn([
"size-8",
systemAudioPermissionStatus.data === "authorized" && "bg-stone-100 text-stone-800",
])}
+ aria-label={systemAudioPermissionStatus.data === "authorized" ? "System audio permission granted" : "Request system audio permission"}
>📝 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.
| <Button | |
| variant={systemAudioPermissionStatus.data === "authorized" ? "outline" : "default"} | |
| size="icon" | |
| onClick={handleSystemAudioPermissionAction} | |
| disabled={systemAudioPermission.isPending || systemAudioPermissionStatus.data === "authorized"} | |
| className={cn([ | |
| "size-8", | |
| systemAudioPermissionStatus.data === "authorized" && "bg-stone-100 text-stone-800", | |
| ])} | |
| > | |
| {systemAudioPermissionStatus.data === "authorized" | |
| ? <CheckIcon className="size-5" /> | |
| : <ArrowRightIcon className="size-5" />} | |
| </Button> | |
| <Button | |
| variant={systemAudioPermissionStatus.data === "authorized" ? "outline" : "default"} | |
| size="icon" | |
| onClick={handleSystemAudioPermissionAction} | |
| disabled={systemAudioPermission.isPending || systemAudioPermissionStatus.data === "authorized"} | |
| className={cn([ | |
| "size-8", | |
| systemAudioPermissionStatus.data === "authorized" && "bg-stone-100 text-stone-800", | |
| ])} | |
| aria-label={systemAudioPermissionStatus.data === "authorized" ? "System audio permission granted" : "Request system audio permission"} | |
| > | |
| {systemAudioPermissionStatus.data === "authorized" | |
| ? <CheckIcon className="size-5" /> | |
| : <ArrowRightIcon className="size-5" />} | |
| </Button> |
🤖 Prompt for AI Agents
In apps/desktop/src/components/onboarding/permissions.tsx around lines 79 to 92,
the icon-only Button is missing an aria-label which is required for screen
reader accessibility; add an aria-label prop that reflects the current
action/state (e.g. when systemAudioPermissionStatus.data === "authorized" use
something like "System audio permission granted" or "System audio permission
authorized", otherwise "Grant system audio permission" or "Enable system audio
permission") and compute it the same way the icon and variant are computed so
the label matches the visual affordance; ensure the aria-label is present even
when the button is disabled.
| <div className="gap-2"> | ||
| <div | ||
| className={cn([ | ||
| "flex items-center gap-2", | ||
| accessibilityPermissionStatus.data !== "authorized" ? "text-red-500" : "text-neutral-900", | ||
| ])} | ||
| > | ||
| {accessibilityPermissionStatus.data !== "authorized" && <AlertCircleIcon className="size-4" />} | ||
| <span className="text-base font-medium">Accessibility</span> | ||
| </div> | ||
| <p className="text-sm text-neutral-500"> | ||
| {accessibilityPermissionStatus.data === "authorized" | ||
| ? "Good to go :)" | ||
| : "To sync mic inputs & mute from meetings"} | ||
| </p> | ||
| </div> |
There was a problem hiding this comment.
Fix layout: gap-2 requires flex or grid container.
Same layout issue—gap-2 has no effect without flex or grid.
Apply this diff:
- <div className="flex items-center justify-between">
- <div className="gap-2">
+ <div className="flex items-center justify-between">
+ <div className="flex flex-col gap-2">Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/desktop/src/components/onboarding/permissions.tsx around lines 97 to
112, the outer div uses className="gap-2" but gap utilities only apply to flex
or grid containers; update the outer div to be a flex (vertical) container so
the gap takes effect — e.g., add "flex flex-col" to the className (preserving
any existing classes) so the inner title row and paragraph are spaced correctly.
| <Button | ||
| variant={accessibilityPermissionStatus.data === "authorized" ? "outline" : "default"} | ||
| size="icon" | ||
| onClick={handleAccessibilityPermissionAction} | ||
| disabled={accessibilityPermission.isPending || accessibilityPermissionStatus.data === "authorized"} | ||
| className={cn([ | ||
| "size-8", | ||
| accessibilityPermissionStatus.data === "authorized" && "bg-stone-100 text-stone-800", | ||
| ])} | ||
| > | ||
| {accessibilityPermissionStatus.data === "authorized" | ||
| ? <CheckIcon className="size-5" /> | ||
| : <ArrowRightIcon className="size-5" />} | ||
| </Button> |
There was a problem hiding this comment.
Add aria-label for screen reader accessibility.
The icon-only button needs an aria-label.
Apply this diff:
<Button
variant={accessibilityPermissionStatus.data === "authorized" ? "outline" : "default"}
size="icon"
onClick={handleAccessibilityPermissionAction}
disabled={accessibilityPermission.isPending || accessibilityPermissionStatus.data === "authorized"}
className={cn([
"size-8",
accessibilityPermissionStatus.data === "authorized" && "bg-stone-100 text-stone-800",
])}
+ aria-label={accessibilityPermissionStatus.data === "authorized" ? "Accessibility permission granted" : "Request accessibility permission"}
>📝 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.
| <Button | |
| variant={accessibilityPermissionStatus.data === "authorized" ? "outline" : "default"} | |
| size="icon" | |
| onClick={handleAccessibilityPermissionAction} | |
| disabled={accessibilityPermission.isPending || accessibilityPermissionStatus.data === "authorized"} | |
| className={cn([ | |
| "size-8", | |
| accessibilityPermissionStatus.data === "authorized" && "bg-stone-100 text-stone-800", | |
| ])} | |
| > | |
| {accessibilityPermissionStatus.data === "authorized" | |
| ? <CheckIcon className="size-5" /> | |
| : <ArrowRightIcon className="size-5" />} | |
| </Button> | |
| <Button | |
| variant={accessibilityPermissionStatus.data === "authorized" ? "outline" : "default"} | |
| size="icon" | |
| onClick={handleAccessibilityPermissionAction} | |
| disabled={accessibilityPermission.isPending || accessibilityPermissionStatus.data === "authorized"} | |
| className={cn([ | |
| "size-8", | |
| accessibilityPermissionStatus.data === "authorized" && "bg-stone-100 text-stone-800", | |
| ])} | |
| aria-label={accessibilityPermissionStatus.data === "authorized" ? "Accessibility permission granted" : "Request accessibility permission"} | |
| > | |
| {accessibilityPermissionStatus.data === "authorized" | |
| ? <CheckIcon className="size-5" /> | |
| : <ArrowRightIcon className="size-5" />} | |
| </Button> |
🤖 Prompt for AI Agents
In apps/desktop/src/components/onboarding/permissions.tsx around lines 113 to
126, the icon-only Button lacks an aria-label which is required for screen
reader accessibility; add an appropriate aria-label prop to the Button that
varies by state (e.g., "Accessibility permission granted" or "Request
accessibility permission") so screen readers get context, ensure the label
reflects the authorized vs. non-authorized state and keep it consistent with the
button action.
| filteredItems.map((item: VocabItem) => { | ||
| const itemIndex = vocabItems.findIndex((v) => v.rowId === item.rowId); | ||
| return ( | ||
| <VocabularyItem | ||
| key={item.rowId} | ||
| item={item} | ||
| itemNumber={itemIndex + 1} | ||
| vocabItems={vocabItems} | ||
| isEditing={editingId === item.rowId} | ||
| isSearching={searchValue.trim().length > 0} | ||
| onStartEdit={() => setEditingId(item.rowId)} | ||
| onCancelEdit={() => setEditingId(null)} | ||
| onUpdate={mutations.update} | ||
| onRemove={() => mutations.delete(item.rowId)} | ||
| /> | ||
| ); | ||
| }) |
There was a problem hiding this comment.
Optimize itemNumber calculation to avoid O(n²) complexity.
The findIndex call inside the map loop creates quadratic time complexity. With a large vocabulary list, this could cause noticeable performance degradation during rendering.
Consider creating an index map before the render loop:
const filteredItems = useMemo(() => {
if (!searchValue.trim()) {
return vocabItems;
}
const query = searchValue.toLowerCase();
return vocabItems.filter((item) => item.text.toLowerCase().includes(query));
}, [vocabItems, searchValue]);
+ const itemIndexMap = useMemo(() => {
+ return new Map(vocabItems.map((item, index) => [item.rowId, index + 1]));
+ }, [vocabItems]);
const allTexts = vocabItems.map((item) => item.text.toLowerCase());
const exactMatch = allTexts.includes(searchValue.toLowerCase());
const showAddButton = searchValue.trim() && !exactMatch;
return (
<div>
{/* ... */}
<div className="max-h-[300px] overflow-y-auto">
{filteredItems.length === 0
? (
<div className="px-4 py-8 text-center text-sm text-neutral-400">
{searchValue.trim() ? "No matching terms" : "No custom vocabulary added"}
</div>
)
: (
- filteredItems.map((item: VocabItem) => {
- const itemIndex = vocabItems.findIndex((v) => v.rowId === item.rowId);
+ filteredItems.map((item) => {
return (
<VocabularyItem
key={item.rowId}
item={item}
- itemNumber={itemIndex + 1}
+ itemNumber={itemIndexMap.get(item.rowId)!}
vocabItems={vocabItems}
isEditing={editingId === item.rowId}
isSearching={searchValue.trim().length > 0}
onStartEdit={() => setEditingId(item.rowId)}
onCancelEdit={() => setEditingId(null)}
onUpdate={mutations.update}
onRemove={() => mutations.delete(item.rowId)}
/>
);
})
)}
</div>Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/desktop/src/components/settings/memory/custom-vocabulary.tsx around
lines 100 to 116, the code calls findIndex inside the map which causes O(n²)
rendering; precompute a lookup (e.g., a Map or object) that maps rowId to its
index before rendering, then use that lookup inside the map to compute
itemNumber (lookup.get(item.rowId) + 1) so the render loop becomes O(n).
coderabbit
ComputelessComputer
force-pushed
the
john-desktop-wip
branch
from
1345b59 to
593bbc1
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/desktop/src/components/onboarding/welcome.tsx (1)
37-49: Remove commented code instead of leaving it in the codebase.Commented-out code should be removed entirely. If this functionality might be needed in the future, rely on git history to retrieve it or create an issue to track the potential feature restoration.
Apply this diff to remove the commented code:
- { - /*<div - className={cn([ - "flex flex-row items-center gap-1", - "text-neutral-400 transition-colors hover:text-neutral-800", - ])} - > - <button className="text-sm underline" onClick={() => onNext({ local: true })}> - Or proceed without an account - </button> - <CircleQuestionMarkIcon className="h-4 w-4 cursor-help" /> - </div>*/ - }apps/desktop/src/components/settings/ai/llm/select.tsx (1)
192-192: Remove debug logging of sensitive data.The
console.logstatement logs the API key, which could expose sensitive credentials in production. Remove this debug statement before merging.Apply this diff:
- console.log(provider.id, baseUrl, apiKey);
♻️ Duplicate comments (9)
apps/desktop/src/components/onboarding/permissions.tsx (7)
33-125: Consider extracting repeated permission blocks into a reusable component.All three permission blocks follow an identical pattern. Extracting a
PermissionBlockcomponent would reduce duplication and improve maintainability.
34-47: Fix layout:gap-2requires flex or grid container.The
gap-2utility on line 34 has no effect because the div isn't usingdisplay: flexordisplay: grid.Apply this diff to fix the layout:
- <div className="gap-2"> + <div className="flex flex-col gap-2">
48-58: Addaria-labelfor screen reader accessibility.The icon-only button should include an
aria-labelto ensure screen readers can convey the button's purpose to users.Apply this diff:
<Button variant={micPermissionStatus.data === "authorized" ? "outline" : "default"} size="icon" onClick={handleMicPermissionAction} disabled={micPermission.isPending || micPermissionStatus.data === "authorized"} className={cn(["size-8", micPermissionStatus.data === "authorized" && "bg-stone-100 text-stone-800"])} + aria-label={micPermissionStatus.data === "authorized" ? "Microphone permission granted" : "Request microphone permission"} >
62-77: Fix layout:gap-2requires flex or grid container.Same issue as the Microphone block—the
gap-2utility has no effect without flex or grid.Apply this diff:
- <div className="gap-2"> + <div className="flex flex-col gap-2">
78-91: Addaria-labelfor screen reader accessibility.The icon-only button needs an
aria-label.Apply this diff:
<Button variant={systemAudioPermissionStatus.data === "authorized" ? "outline" : "default"} size="icon" onClick={handleSystemAudioPermissionAction} disabled={systemAudioPermission.isPending || systemAudioPermissionStatus.data === "authorized"} className={cn([ "size-8", systemAudioPermissionStatus.data === "authorized" && "bg-stone-100 text-stone-800", ])} + aria-label={systemAudioPermissionStatus.data === "authorized" ? "System audio permission granted" : "Request system audio permission"} >
95-110: Fix layout:gap-2requires flex or grid container.Same layout issue—
gap-2has no effect without flex or grid.Apply this diff:
- <div className="gap-2"> + <div className="flex flex-col gap-2">
111-124: Addaria-labelfor screen reader accessibility.The icon-only button needs an
aria-label.Apply this diff:
<Button variant={accessibilityPermissionStatus.data === "authorized" ? "outline" : "default"} size="icon" onClick={handleAccessibilityPermissionAction} disabled={accessibilityPermission.isPending || accessibilityPermissionStatus.data === "authorized"} className={cn([ "size-8", accessibilityPermissionStatus.data === "authorized" && "bg-stone-100 text-stone-800", ])} + aria-label={accessibilityPermissionStatus.data === "authorized" ? "Accessibility permission granted" : "Request accessibility permission"} >apps/desktop/src/components/settings/memory/custom-vocabulary.tsx (1)
100-116: Performance issue: O(n²) complexity from findIndex in map loop.This performance concern was already identified in a previous review. The
findIndexcall on line 101 inside themaploop creates quadratic time complexity. Please refer to the earlier review comment for the detailed solution using a precomputed index map.apps/desktop/src/components/settings/ai/stt/select.tsx (1)
70-83: Guard the deferred model auto-fill.The deferred
setTimeoutcan still run after the user flips to another provider (e.g., Custom), so we end up persisting a Deepgram model while the selected provider is now Custom. That breaks the configuration flow and matches the race we previously chased. Please guard the timeout with the provider that scheduled it (or remove the defer entirely) so stale callbacks bail out.- if (availableModels.length > 0) { - setTimeout(() => { - form.setFieldValue("model", availableModels[0].id); - }, 0); - } + if (availableModels.length > 0) { + const [firstModel] = availableModels; + setTimeout(() => { + if (form.getFieldValue("provider") !== providerId) { + return; + } + form.setFieldValue("model", firstModel.id); + }, 0); + }
🧹 Nitpick comments (8)
apps/desktop/src/components/main/body/search.tsx (2)
78-78: Consider passinghasSpaceas a prop to avoid duplicate media queries.The
hasSpacemedia query is already computed in the parentSearchcomponent (line 13). Computing it again here creates a redundant subscription. Consider passing it as a prop toExpandedSearchinstead.Apply this diff to accept
hasSpaceas a prop:-function ExpandedSearch({ onFocus, onBlur }: { onFocus?: () => void; onBlur?: () => void }) { +function ExpandedSearch({ onFocus, onBlur, hasSpace }: { onFocus?: () => void; onBlur?: () => void; hasSpace: boolean }) { const { query, setQuery, isSearching, isIndexing, inputRef } = useSearch(); const [isFocused, setIsFocused] = useState(false); const isCmdPressed = useCmdKeyPressed(); - const hasSpace = useMediaQuery("(min-width: 900px)");And update the call site:
if (shouldShowExpanded) { - return <ExpandedSearch onFocus={handleExpandedFocus} onBlur={handleExpandedBlur} />; + return <ExpandedSearch onFocus={handleExpandedFocus} onBlur={handleExpandedBlur} hasSpace={hasSpace} />; }
95-97: Consider usingsize-4for icon consistency.The icons here still use
h-4 w-4classes, whileCollapsedSearchwas updated to use the more concisesize-4class (lines 68-69). For consistency, consider updating these icons to match.Apply this diff:
{showLoading - ? <Loader2Icon className={cn(["h-4 w-4 absolute left-3 text-neutral-400 animate-spin"])} /> - : <SearchIcon className={cn(["h-4 w-4 absolute left-3 text-neutral-400"])} />} + ? <Loader2Icon className={cn(["size-4 absolute left-3 text-neutral-400 animate-spin"])} /> + : <SearchIcon className={cn(["size-4 absolute left-3 text-neutral-400"])} />}apps/desktop/src/components/main/body/shared.tsx (1)
92-146: Consider extracting the icon/close button logic into sub-components.The nested conditional rendering for the icon and close button overlay is complex and spans many lines. Extracting this into smaller, focused components would improve readability and maintainability.
For example, you could create:
TabIconcomponent for the icon display logicTabCloseButtoncomponent for the close button overlayapps/desktop/src/components/settings/general/permissions.tsx (1)
38-72: Consider adding accessible labels for icons.The AlertCircleIcon (line 42) and Check icon (line 66) don't have accessible text, which could impact screen reader users.
Apply this diff to add accessibility:
- {!isAuthorized && <AlertCircleIcon className="size-4" />} + {!isAuthorized && <AlertCircleIcon className="size-4" aria-label="Warning" />}- {isAuthorized ? <Check size={16} /> : null} + {isAuthorized ? <Check size={16} aria-label="Granted" /> : null}apps/desktop/src/components/settings/template/shared.tsx (1)
49-56: LGTM with a minor note.The targets chip rendering is well-implemented with appropriate spacing and responsive layout. The optional chaining handles the null/undefined array correctly.
Note: While React gracefully handles null/undefined elements in the map, consider adding
target?.optional chaining for consistency with the filtering logic inremote.tsx(once that's fixed).If desired, apply this diff for defensive consistency:
<div className="flex items-center gap-2 flex-wrap"> - {targets?.map((target, index) => ( + {targets?.map((target, index) => target && ( <span key={index} className="text-xs text-neutral-600 bg-neutral-100 px-2 py-0.5 rounded"> {target} </span> ))} </div>apps/desktop/src/components/chat/input.tsx (1)
46-59: TODO: Implement stub handlersThe file attachment, screenshot, and voice input handlers are currently stubs with console.log placeholders. These features are wired to UI buttons but not yet functional.
Would you like me to help implement these handlers or open issues to track their completion?
apps/desktop/src/components/settings/ai/llm/select.tsx (2)
239-277: Consider usinguseMemofor status computation.The IIFE pattern works but recomputes on every render. Using
useMemowould make the intent clearer and follow React best practices by memoizing the computation based on dependencies.- const { status, message, textColor } = (() => { + const { status, message, textColor } = useMemo(() => { if (!model) { return { status: "No model configured", message: "Please configure a provider and model", textColor: "text-red-600", }; } if (text.isPending) { return { status: "Checking connection", message: "Testing model connection", textColor: "text-yellow-600", }; } if (text.isError) { return { status: "Connection failed", message: text.error?.message || "Unable to connect to the model", textColor: "text-red-600", }; } if (text.isSuccess) { return { status: "Connected!", message: "Model is ready to use", textColor: "text-green-600", }; } return { status: "Unknown status", message: "Connection status unknown", textColor: "text-red-600", }; - })(); + }, [model, text.isPending, text.isError, text.isSuccess, text.error]);
287-293: Loading animation uses inline styles.The dots animation works correctly but uses inline
styleattributes foranimationDelay, which is inconsistent with the Tailwind-first approach used throughout the file. Consider using Tailwind's animation utilities or defining a CSS animation if you want to maintain consistency.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (17)
apps/desktop/src-tauri/icons/nightly/128x128.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/[email protected]is excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/16.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/32x32.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/64x64.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square107x107Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square142x142Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square150x150Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square284x284Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square30x30Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square310x310Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square44x44Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square71x71Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square89x89Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/StoreLogo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/icon.icois excluded by!**/*.icoapps/desktop/src-tauri/icons/nightly/icon.pngis excluded by!**/*.png
📒 Files selected for processing (36)
apps/desktop/src/components/chat/body/empty.tsx(1 hunks)apps/desktop/src/components/chat/body/index.tsx(3 hunks)apps/desktop/src/components/chat/header.tsx(1 hunks)apps/desktop/src/components/chat/index.tsx(1 hunks)apps/desktop/src/components/chat/input.tsx(3 hunks)apps/desktop/src/components/chat/interactive.tsx(1 hunks)apps/desktop/src/components/chat/trigger.tsx(1 hunks)apps/desktop/src/components/chat/view.tsx(1 hunks)apps/desktop/src/components/main/body/index.tsx(5 hunks)apps/desktop/src/components/main/body/search.tsx(2 hunks)apps/desktop/src/components/main/body/sessions/floating/listen.tsx(1 hunks)apps/desktop/src/components/main/body/shared.tsx(2 hunks)apps/desktop/src/components/main/sidebar/banner/registry.tsx(3 hunks)apps/desktop/src/components/onboarding/calendar.tsx(0 hunks)apps/desktop/src/components/onboarding/permissions.tsx(2 hunks)apps/desktop/src/components/onboarding/shared.tsx(2 hunks)apps/desktop/src/components/onboarding/welcome.tsx(3 hunks)apps/desktop/src/components/settings/ai/llm/configure.tsx(1 hunks)apps/desktop/src/components/settings/ai/llm/index.tsx(1 hunks)apps/desktop/src/components/settings/ai/llm/no-model-banner.tsx(1 hunks)apps/desktop/src/components/settings/ai/llm/select.tsx(3 hunks)apps/desktop/src/components/settings/ai/shared/model-combobox.tsx(2 hunks)apps/desktop/src/components/settings/ai/stt/configure.tsx(2 hunks)apps/desktop/src/components/settings/ai/stt/index.tsx(1 hunks)apps/desktop/src/components/settings/ai/stt/no-model-banner.tsx(1 hunks)apps/desktop/src/components/settings/ai/stt/select.tsx(3 hunks)apps/desktop/src/components/settings/ai/stt/shared.tsx(1 hunks)apps/desktop/src/components/settings/general/permissions.tsx(2 hunks)apps/desktop/src/components/settings/memory/custom-vocabulary.tsx(8 hunks)apps/desktop/src/components/settings/template/remote.tsx(2 hunks)apps/desktop/src/components/settings/template/shared.tsx(2 hunks)apps/desktop/src/components/shared/permission-row.tsx(0 hunks)apps/desktop/src/routes/app.tsx(1 hunks)apps/desktop/src/routes/app/main/_layout.index.tsx(1 hunks)apps/desktop/src/routes/app/onboarding/index.tsx(1 hunks)package.json(1 hunks)
💤 Files with no reviewable changes (2)
- apps/desktop/src/components/onboarding/calendar.tsx
- apps/desktop/src/components/shared/permission-row.tsx
🚧 Files skipped from review as they are similar to previous changes (15)
- apps/desktop/src/components/settings/ai/stt/index.tsx
- apps/desktop/src/components/chat/body/empty.tsx
- apps/desktop/src/components/chat/interactive.tsx
- apps/desktop/src/routes/app/main/_layout.index.tsx
- apps/desktop/src/components/settings/ai/stt/shared.tsx
- apps/desktop/src/components/main/body/sessions/floating/listen.tsx
- apps/desktop/src/components/settings/ai/llm/configure.tsx
- apps/desktop/src/components/onboarding/shared.tsx
- apps/desktop/src/components/chat/index.tsx
- apps/desktop/src/components/chat/body/index.tsx
- apps/desktop/src/components/chat/header.tsx
- apps/desktop/src/routes/app/onboarding/index.tsx
- package.json
- apps/desktop/src/components/main/body/index.tsx
- apps/desktop/src/routes/app.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-12T01:18:52.105Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: packages/nango/.cursor/rules/nango.mdc:0-0
Timestamp: 2025-10-12T01:18:52.105Z
Learning: Applies to packages/nango/**/models.ts : Do not edit the models.ts file; it is auto-generated at compilation time
Applied to files:
apps/desktop/src/components/settings/ai/llm/index.tsx
🧬 Code graph analysis (11)
apps/desktop/src/components/settings/ai/stt/no-model-banner.tsx (4)
packages/utils/src/cn.ts (1)
cn(20-22)apps/desktop/src/config/use-config.ts (1)
useConfigValues(32-53)apps/desktop/src/components/settings/ai/stt/shared.tsx (1)
sttModelQueries(248-275)apps/desktop/src/store/tinybase/schema-internal.ts (1)
AIProviderStorage(40-40)
apps/desktop/src/components/main/body/search.tsx (3)
apps/desktop/src/contexts/search/ui.tsx (1)
useSearch(236-242)apps/desktop/src/hooks/useCmdKeyPressed.tsx (1)
useCmdKeyPressed(3-35)packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/settings/ai/llm/no-model-banner.tsx (3)
packages/utils/src/cn.ts (1)
cn(20-22)apps/desktop/src/auth.tsx (1)
useAuth(177-185)apps/desktop/src/config/use-config.ts (1)
useConfigValues(32-53)
apps/desktop/src/components/settings/general/permissions.tsx (1)
packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/settings/ai/stt/select.tsx (2)
packages/utils/src/cn.ts (1)
cn(20-22)apps/desktop/src/components/settings/ai/stt/shared.tsx (3)
ProviderId(8-8)PROVIDERS(46-104)displayModelId(22-44)
apps/desktop/src/components/chat/input.tsx (1)
packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/onboarding/permissions.tsx (2)
apps/desktop/src/components/onboarding/shared.tsx (1)
OnboardingContainer(21-43)packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/main/body/shared.tsx (1)
packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/settings/ai/llm/select.tsx (3)
packages/utils/src/cn.ts (1)
cn(20-22)apps/desktop/src/components/settings/ai/llm/shared.tsx (1)
PROVIDERS(6-63)apps/desktop/src/components/settings/ai/shared/model-combobox.tsx (1)
ModelCombobox(45-233)
apps/desktop/src/components/chat/view.tsx (1)
apps/desktop/src/components/chat/input.tsx (1)
ChatMessageInput(12-135)
apps/desktop/src/components/settings/ai/llm/index.tsx (1)
apps/desktop/src/components/settings/ai/llm/no-model-banner.tsx (1)
NoModelBanner(11-31)
🔇 Additional comments (27)
apps/desktop/src/components/settings/ai/shared/model-combobox.tsx (1)
2-2: LGTM! Icon change improves semantic clarity.The switch from
ChevronsUpDowntoChevronDownis more semantically appropriate for a combobox dropdown. The negative right margin (-mr-1) tightens the spacing between the icon and the button edge, which is a common design pattern for trailing icons in button components.Also applies to: 103-103
apps/desktop/src/components/onboarding/welcome.tsx (2)
12-12: LGTM: Clean refactor to React fragment.Removing the unnecessary wrapper div results in a cleaner DOM structure with no functional impact.
Also applies to: 50-50
31-32: LGTM: Button styling update.The larger size and full-width styling align with the UI polish objectives of this PR.
apps/desktop/src/components/main/sidebar/banner/registry.tsx (3)
1-1: LGTM! Improved icon semantics.The new icon choices are more semantically appropriate:
AudioLinesIconclearly represents audio/speech functionality for STT, andSparklesIconis a well-established choice for AI features.
32-38: LGTM! Clear and user-friendly STT banner.The updated content is more specific and informative. The title clearly indicates STT model configuration, the description explains why it's needed, and the action label "Go to settings" is concise and consistent.
46-52: LGTM! Clear and user-friendly LLM banner.The updated content effectively communicates the purpose of the LLM. The description clearly explains that it powers summarization and chat features, and the action label maintains consistency with the STT banner.
apps/desktop/src/components/main/body/search.tsx (1)
61-70: LGTM: Button component migration.The refactor to use the
Buttoncomponent with consistent icon sizing improves maintainability and aligns with the design system.apps/desktop/src/components/onboarding/permissions.tsx (3)
2-4: Imports look good.The
cnutility and new icon imports are appropriate for the status-aware permission blocks.
31-31: Title simplification improves clarity.The updated title is more concise and direct.
128-130: Excellent UX with state-aware button text.The Continue button provides clear feedback by showing why it's disabled and when users can proceed.
apps/desktop/src/components/settings/memory/custom-vocabulary.tsx (5)
1-8: LGTM: Import updates support UI enhancements.The new imports (
Button,cn,Searchicon) are properly utilized throughout the component for the UI polish improvements.
50-55: LGTM: Header simplification improves clarity.The streamlined header with clearer description text aligns well with the PR's UI polish objectives.
64-88: LGTM: Search UI enhancements improve user experience.The addition of the Search icon, background color update, and button styling refinements create a more polished interface.
126-146: LGTM: Props interface correctly extended.The
itemNumberandisSearchingprops are appropriately typed and support the new item numbering feature.
194-222: LGTM: Item numbering implementation enhances usability.The item number display with search-aware visibility provides clear visual ordering. Hiding numbers during search (line 195) is a good UX choice since filtered results may show non-consecutive indices.
apps/desktop/src/components/settings/general/permissions.tsx (2)
1-7: LGTM!The imports are properly organized and all necessary for the internal PermissionRow component implementation.
92-112: LGTM!The simplified permission titles are more concise while maintaining clarity, and all PermissionRow props are correctly passed.
apps/desktop/src/components/settings/template/remote.tsx (1)
32-33: LGTM!The new props are passed correctly to
TemplateCard, maintaining consistency with the extended template data structure.apps/desktop/src/components/settings/template/shared.tsx (2)
6-7: LGTM!The new props are properly typed with optional and nullable types, maintaining consistency with the existing prop definitions.
Also applies to: 12-13
41-48: LGTM!The inline category badge is well-implemented with appropriate layout changes and conditional rendering. The visual hierarchy is clear with the category displayed in a muted color.
apps/desktop/src/components/chat/trigger.tsx (1)
3-4: LGTM: Import reorderingThe import reordering is purely stylistic with no functional impact.
apps/desktop/src/components/chat/view.tsx (1)
107-110: LGTM: Model configuration awarenessThe changes cleanly thread model configuration state through the chat UI:
isModelConfigured={!!model}properly converts the model to a boolean for downstream components- The simplified
disabledlogic clearly gates input when either the model is missing or the chat status is not readyapps/desktop/src/components/chat/input.tsx (3)
41-44: Good improvement: Conditional focus behaviorGating the editor focus on the
disabledstate prevents stealing focus when the input is unavailable, improving the user experience.
154-159: LGTM: Well-implemented placeholder componentThe
ChatPlaceholderfunction correctly implements thePlaceholderFunctioninterface, properly checks for the first paragraph position, and provides friendly, contextually appropriate placeholder text.
77-131: LGTM: Clean multi-action button layoutThe new layout organizes actions logically (media operations on the left, communication on the right) with consistent styling and proper disabled state handling across all buttons.
apps/desktop/src/components/settings/ai/llm/index.tsx (1)
8-8: Banner placement looks good.Mounting
NoModelBannerahead of the selector keeps the warning visible without disrupting the existing layout.apps/desktop/src/components/settings/ai/llm/select.tsx (1)
84-86: Race condition fix correctly implemented!The guard check ensures that the model is only set if the provider hasn't changed since the fetch began. This prevents the invalid provider+model pairing that was flagged in the previous review.
| }} | ||
| className="flex items-center justify-center text-red-600 hover:text-red-700" | ||
| > | ||
| <span className="w-3 h-3 bg-red-600 hover:bg-red-700 rounded-none transition-colors" /> |
There was a problem hiding this comment.
Consider using a consistent close button icon.
Active tabs display a solid square on hover while non-active tabs show an X icon. This inconsistency might confuse users about the close action.
Consider using the X icon consistently for both states:
- <span className="w-3 h-3 bg-red-600 hover:bg-red-700 rounded-none transition-colors" />
+ <X size={16} />📝 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.
| <span className="w-3 h-3 bg-red-600 hover:bg-red-700 rounded-none transition-colors" /> | |
| <X size={16} /> |
🤖 Prompt for AI Agents
In apps/desktop/src/components/main/body/shared.tsx around line 124, the close
button for active tabs renders a solid square span which is inconsistent with
the X icon used on non-active tabs; replace the span with the same X icon
markup/component used elsewhere (matching size, hover/focus states and ARIA
attributes) so both active and non-active tabs show the same close icon, and
ensure classes and accessible title/aria-label remain consistent.
593bbc1 to
f2ecd16
Compare
ComputelessComputer
force-pushed
the
john-desktop-wip
branch
from
f2ecd16 to
0c1e4fe
Compare
No description provided.