-
Notifications
You must be signed in to change notification settings - Fork 1.1k
improvement: make mode selection simpler, cleaner, and update button styles of desktop #965
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughUpdated button variant system and replaced many usages with new variants; refactored mode selection to render icons via functions instead of image assets; adjusted some CSS shadow rules and a strict-equality check; removed a few unused imports. No exported/public API signatures changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant ModeSelect as ModeSelect (parent)
participant ModeOption as ModeOption (item)
participant IconFn as icon(props) → JSX
rect rgba(0,128,128,0.06)
ModeSelect->>ModeOption: render list of options with isSelected
ModeOption->>IconFn: call icon({ class, style })
IconFn-->>ModeOption: returns SVG/JSX element
ModeOption-->>ModeSelect: rendered option (with ring/opacity/style)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (4)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/desktop/src/routes/(window-chrome)/new-main/index.tsx (1)
371-374: Fix typo in user-facing text (“Singning” → “Signing”)Minor UX polish.
Apply this diff:
- <span>Singning In...</span> + <span>Signing In...</span>packages/ui-solid/src/Button.stories.tsx (1)
34-35: Fix lingering “secondary” usage and column label.Hover row still uses variant="secondary", which no longer exists. Also rename header to “Gray” for accuracy.
Apply:
- <th>Secondary</th> + <th>Gray</th>- <td> - <Button - variant="secondary" - size={size()} - class="sb-pseudo--hover" - > - Button - </Button> - </td> + <td> + <Button + variant="gray" + size={size()} + class="sb-pseudo--hover" + > + Button + </Button> + </td>Also applies to: 69-76
apps/desktop/src/components/Mode.tsx (1)
86-88: Leftover ring-blue-10 here; likely an oversight given other changesThis still uses ring-blue-10 while the others use ring-blue-500.
Apply the same change as above for consistency and to avoid mixing design token systems.
🧹 Nitpick comments (5)
packages/ui-solid/src/Button.tsx (1)
12-29: Unify disabled affordance and clean up minor class conflicts.
- Several variants lack disabled:cursor-not-allowed; UX inconsistency.
- radialblue sets both border and border-0/border-transparent; last-wins conflict.
- ghost has no base text color; can be unreadable on light backgrounds.
Apply this diff to tighten things up:
- primary: - "bg-gray-12 dark-button-shadow text-gray-1 disabled:bg-gray-6 disabled:text-gray-9", + primary: + "bg-gray-12 dark-button-shadow text-gray-1 disabled:bg-gray-6 disabled:text-gray-9 disabled:cursor-not-allowed", - blue: "bg-blue-600 text-white border border-blue-800 shadow-[0_1.50px_0_0_rgba(255,255,255,0.20)_inset] hover:bg-blue-700 disabled:bg-gray-6 disabled:text-gray-9", + blue: + "bg-blue-600 text-white border border-blue-800 shadow-[0_1.50px_0_0_rgba(255,255,255,0.20)_inset] hover:bg-blue-700 disabled:bg-gray-6 disabled:text-gray-9 disabled:cursor-not-allowed", - destructive: - "bg-red-500 text-white hover:bg-red-600 disabled:bg-red-200", + destructive: + "bg-red-500 text-white hover:bg-red-600 disabled:bg-red-200 disabled:cursor-not-allowed", - outline: - "border border-gray-4 hover:border-gray-12 hover:bg-gray-12 hover:text-gray-1 text-gray-12 disabled:bg-gray-8", + outline: + "border border-gray-4 hover:border-gray-12 hover:bg-gray-12 hover:text-gray-1 text-gray-12 disabled:bg-gray-8 disabled:cursor-not-allowed", - white: - "bg-gray-1 border border-gray-6 text-gray-12 hover:bg-gray-3 disabled:bg-gray-8", + white: + "bg-gray-1 border border-gray-6 text-gray-12 hover:bg-gray-3 disabled:bg-gray-8 disabled:cursor-not-allowed", - ghost: "hover:bg-white/20 hover:text-white", + ghost: + "text-gray-12 hover:bg-white/20 hover:text-white disabled:cursor-not-allowed", - gray: "bg-gray-5 data-[selected=true]:!bg-gray-8 dark:data-[selected=true]:!bg-gray-9 hover:bg-gray-7 border border-gray-6 gray-button-shadow text-gray-12 disabled:bg-gray-8 disabled:text-gray-9", + gray: + "bg-gray-5 data-[selected=true]:!bg-gray-8 dark:data-[selected=true]:!bg-gray-9 hover:bg-gray-7 border border-gray-6 gray-button-shadow text-gray-12 disabled:bg-gray-8 disabled:text-gray-9 disabled:cursor-not-allowed", - dark: "bg-gray-12 dark-button-shadow hover:bg-gray-11 border border-gray-12 text-gray-1 disabled:cursor-not-allowed disabled:text-gray-10 disabled:bg-gray-7 disabled:border-gray-8", + dark: + "bg-gray-12 dark-button-shadow hover:bg-gray-11 border border-gray-12 text-gray-1 disabled:cursor-not-allowed disabled:text-gray-10 disabled:bg-gray-7 disabled:border-gray-8", - darkgradient: - "bg-gradient-to-t button-gradient-border from-[#0f0f0f] to-[#404040] shadow-[0_0_0_1px] hover:brightness-110 shadow-[#383838] text-gray-50 hover:bg-[#383838] disabled:bg-[#383838] border-transparent", + darkgradient: + "bg-gradient-to-t button-gradient-border from-[#0f0f0f] to-[#404040] shadow-[0_0_0_1px] hover:brightness-110 shadow-[#383838] text-gray-50 hover:bg-[#383838] disabled:bg-[#383838] disabled:cursor-not-allowed border-transparent", - radialblue: - "text-gray-50 border button-gradient-border shadow-[0_0_0_1px] shadow-blue-400 disabled:bg-gray-1 border-0 [background:radial-gradient(90%_100%_at_15%_12%,#9BC4FF_0%,#3588FF_100%)] border-transparent hover:opacity-80", + radialblue: + "text-gray-50 button-gradient-border shadow-[0_0_0_1px] shadow-blue-400 disabled:bg-gray-1 [background:radial-gradient(90%_100%_at_15%_12%,#9BC4FF_0%,#3588FF_100%)] border-transparent hover:opacity-80 disabled:cursor-not-allowed",apps/desktop/src/routes/(window-chrome)/settings/license.tsx (1)
261-266: Use aria-busy for pending state.Small a11y win while the mutation runs.
- <Button - onClick={() => openCommercialCheckout.mutate()} - disabled={openCommercialCheckout.isPending} - variant="dark" + <Button + onClick={() => openCommercialCheckout.mutate()} + disabled={openCommercialCheckout.isPending} + aria-busy={openCommercialCheckout.isPending} + variant="dark"apps/desktop/src/routes/(window-chrome)/settings/integrations/s3-config.tsx (1)
98-103: Timeout message mismatches actual timeout (5500ms).The message says “after 5 seconds” but the timer is 5.5s. Update text to avoid misleading users.
Apply this diff:
- "Connection test timed out after 5 seconds. Please check your endpoint URL and network connection.", + "Connection test timed out after ~5 seconds. Please check your endpoint URL and network connection.",apps/desktop/src/components/Mode.tsx (1)
35-35: Unify ring color token; one path still uses the old tokenMoved to ring-blue-500 here; please update the remaining instance below to avoid mixed token sets.
The lingering usage is at Line 87.
Apply this diff to keep all four states consistent:
- ? "ring-2 ring-offset-1 ring-offset-gray-1 bg-gray-5 hover:bg-gray-7 ring-blue-10" + ? "ring-2 ring-offset-1 ring-offset-gray-1 bg-gray-5 hover:bg-gray-7 ring-blue-500"Also applies to: 57-57, 74-74
apps/desktop/src/routes/editor/ui.tsx (1)
436-436: Avoid any in public component prop typesReplace
anyin ComingSoonTooltip props with a precise type.Apply:
-export function ComingSoonTooltip( - props: ComponentProps<typeof KTooltip> & any, -) { +export function ComingSoonTooltip( + props: ComponentProps<typeof KTooltip> & { as?: ValidComponent }, +) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (20)
apps/desktop/src/components/CapErrorBoundary.tsx(1 hunks)apps/desktop/src/components/Mode.tsx(3 hunks)apps/desktop/src/components/ModeSelect.tsx(1 hunks)apps/desktop/src/routes/(window-chrome)/new-main/index.tsx(1 hunks)apps/desktop/src/routes/(window-chrome)/settings.tsx(1 hunks)apps/desktop/src/routes/(window-chrome)/settings/feedback.tsx(1 hunks)apps/desktop/src/routes/(window-chrome)/settings/general.tsx(1 hunks)apps/desktop/src/routes/(window-chrome)/settings/integrations/s3-config.tsx(1 hunks)apps/desktop/src/routes/(window-chrome)/settings/license.tsx(1 hunks)apps/desktop/src/routes/(window-chrome)/setup.tsx(1 hunks)apps/desktop/src/routes/(window-chrome)/upgrade.tsx(2 hunks)apps/desktop/src/routes/editor/Editor.tsx(1 hunks)apps/desktop/src/routes/editor/ExportDialog.tsx(8 hunks)apps/desktop/src/routes/editor/Header.tsx(1 hunks)apps/desktop/src/routes/editor/ShareButton.tsx(2 hunks)apps/desktop/src/routes/editor/ui.tsx(1 hunks)apps/desktop/src/styles/theme.css(2 hunks)apps/web/app/globals.css(1 hunks)packages/ui-solid/src/Button.stories.tsx(3 hunks)packages/ui-solid/src/Button.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/desktop/**/*.{ts,tsx}: In the desktop app, rely on unplugin-icons auto-imports; do not manually import icon modules
Use generated tauri_specta commands/events (commands, events) in the desktop frontend; listen to generated events directly
Use @tanstack/solid-query for server state in the desktop app
Files:
apps/desktop/src/routes/(window-chrome)/setup.tsxapps/desktop/src/routes/(window-chrome)/settings/feedback.tsxapps/desktop/src/routes/(window-chrome)/settings.tsxapps/desktop/src/routes/(window-chrome)/settings/integrations/s3-config.tsxapps/desktop/src/routes/editor/ui.tsxapps/desktop/src/routes/editor/Header.tsxapps/desktop/src/components/CapErrorBoundary.tsxapps/desktop/src/routes/editor/ExportDialog.tsxapps/desktop/src/routes/editor/Editor.tsxapps/desktop/src/routes/(window-chrome)/settings/general.tsxapps/desktop/src/routes/editor/ShareButton.tsxapps/desktop/src/routes/(window-chrome)/new-main/index.tsxapps/desktop/src/components/ModeSelect.tsxapps/desktop/src/components/Mode.tsxapps/desktop/src/routes/(window-chrome)/settings/license.tsxapps/desktop/src/routes/(window-chrome)/upgrade.tsx
{apps/desktop,packages/ui-solid}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Component naming (Solid): components in PascalCase; hooks/utilities in camelCase starting with 'use' where applicable
Files:
apps/desktop/src/routes/(window-chrome)/setup.tsxapps/desktop/src/routes/(window-chrome)/settings/feedback.tsxapps/desktop/src/routes/(window-chrome)/settings.tsxapps/desktop/src/routes/(window-chrome)/settings/integrations/s3-config.tsxapps/desktop/src/routes/editor/ui.tsxapps/desktop/src/routes/editor/Header.tsxapps/desktop/src/components/CapErrorBoundary.tsxapps/desktop/src/routes/editor/ExportDialog.tsxapps/desktop/src/routes/editor/Editor.tsxapps/desktop/src/routes/(window-chrome)/settings/general.tsxapps/desktop/src/routes/editor/ShareButton.tsxapps/desktop/src/routes/(window-chrome)/new-main/index.tsxapps/desktop/src/components/ModeSelect.tsxapps/desktop/src/components/Mode.tsxapps/desktop/src/routes/(window-chrome)/settings/license.tsxapps/desktop/src/routes/(window-chrome)/upgrade.tsxpackages/ui-solid/src/Button.stories.tsxpackages/ui-solid/src/Button.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; prefer shared types from packages
Files:
apps/desktop/src/routes/(window-chrome)/setup.tsxapps/desktop/src/routes/(window-chrome)/settings/feedback.tsxapps/desktop/src/routes/(window-chrome)/settings.tsxapps/desktop/src/routes/(window-chrome)/settings/integrations/s3-config.tsxapps/desktop/src/routes/editor/ui.tsxapps/desktop/src/routes/editor/Header.tsxapps/desktop/src/components/CapErrorBoundary.tsxapps/desktop/src/routes/editor/ExportDialog.tsxapps/desktop/src/routes/editor/Editor.tsxapps/desktop/src/routes/(window-chrome)/settings/general.tsxapps/desktop/src/routes/editor/ShareButton.tsxapps/desktop/src/routes/(window-chrome)/new-main/index.tsxapps/desktop/src/components/ModeSelect.tsxapps/desktop/src/components/Mode.tsxapps/desktop/src/routes/(window-chrome)/settings/license.tsxapps/desktop/src/routes/(window-chrome)/upgrade.tsxpackages/ui-solid/src/Button.stories.tsxpackages/ui-solid/src/Button.tsx
🧬 Code graph analysis (5)
apps/desktop/src/routes/(window-chrome)/settings.tsx (1)
packages/ui-solid/src/Button.tsx (1)
Button(40-50)
apps/desktop/src/routes/(window-chrome)/settings/integrations/s3-config.tsx (2)
packages/ui-solid/src/Button.tsx (1)
Button(40-50)apps/desktop/src/utils/tauri.ts (1)
events(271-313)
apps/desktop/src/routes/editor/ui.tsx (1)
packages/ui-solid/src/Button.tsx (1)
Button(40-50)
apps/desktop/src/routes/editor/ExportDialog.tsx (2)
apps/desktop/src/components/SignInButton.tsx (1)
SignInButton(6-29)packages/ui-solid/src/Button.tsx (1)
Button(40-50)
apps/desktop/src/components/ModeSelect.tsx (2)
apps/desktop/src-tauri/src/recording.rs (1)
mode(125-130)apps/desktop/src/utils/tauri.ts (1)
RecordingMode(419-419)
⏰ 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). (3)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (21)
apps/desktop/src/routes/(window-chrome)/setup.tsx (1)
492-500: LGTM: migrated to Button variant="gray"Matches the new design token set and usage elsewhere in the PR.
apps/desktop/src/components/CapErrorBoundary.tsx (1)
27-33: LGTM: Reload now uses variant="gray"Consistent with the project-wide variant migration.
apps/desktop/src/routes/(window-chrome)/new-main/index.tsx (1)
376-382: LGTM: Cancel button migrated to variant="gray"Aligns with the updated UI vocabulary.
apps/desktop/src/routes/(window-chrome)/upgrade.tsx (2)
5-6: LGTM: import cleanup with type-only importLean and correct; Accessor is used later in Props.
424-431: LGTM: Purchase button moved to variant="dark"Matches the dark card and new theme shadows.
apps/desktop/src/styles/theme.css (1)
142-149: LGTM: shadow token adjustmentsUpdated .dark-button-shadow values look consistent with the cross-app shadow consolidation.
apps/web/app/globals.css (1)
75-77: Duplication removed; token placement looks good.Single top-level .dark-button-shadow keeps web styles consistent with desktop overrides via .dark .dark-button-shadow. No further action.
packages/ui-solid/src/Button.stories.tsx (2)
29-29: Non-functional class reorder; fine to keep.
47-50: Good: migrate Secondary → Gray in Default/Disabled rows.Aligns with new tokens.
Also applies to: 95-98
apps/desktop/src/routes/(window-chrome)/settings/general.tsx (1)
536-543: Token update to dark looks correct.Matches new Button variants; no other changes needed here.
apps/desktop/src/routes/(window-chrome)/settings.tsx (1)
90-92: Sign Out: variant change aligns with new design tokens (LGTM).Using variant="dark" is consistent with the PR-wide button variant migration. No behavioral changes; keep as-is.
apps/desktop/src/routes/(window-chrome)/settings/integrations/s3-config.tsx (1)
267-269: Secondary action uses variant="gray" (LGTM).Matches the new palette and leaves primary/destructive semantics intact.
apps/desktop/src/routes/editor/ShareButton.tsx (2)
193-195: Share button variant migrated to "dark" (LGTM).Consistent with global style updates; no behavior impact.
314-321: Strict equality fix is correct (LGTM).Avoids unintended type coercion in status checks.
apps/desktop/src/components/ModeSelect.tsx (1)
30-41: Icon render prop migration looks clean (LGTM).Works with unplugin-icons auto-imports and keeps ModeOption generic.
apps/desktop/src/routes/editor/ui.tsx (1)
192-195: Cancel button variant migration looks goodSwitch to variant="gray" aligns with the new tokens overhaul.
apps/desktop/src/routes/editor/Header.tsx (1)
145-156: Export button variant updated to dark — consistent with PR theme changesNo functional impact; styling-only change is consistent.
apps/desktop/src/routes/editor/Editor.tsx (1)
388-409: Rule-of-Thirds toggle uses variant="gray" — consistent with neutral/secondary actionsLooks correct and consistent with the new token set.
apps/desktop/src/routes/editor/ExportDialog.tsx (3)
492-496: Option groups migrated to variant="gray" correctlyExport To, Format, Compression, and Resolution controls now use the neutral "gray" variant consistently.
Also applies to: 510-547, 632-637, 665-668
898-908: Completion actions use variant="dark" — consistent with primary emphasis"Open Link", "Open File", and "Copy to Clipboard" adopt the emphasized style appropriately.
Also applies to: 919-931, 932-957
387-405: Replace deprecated Button variants in SignInButton
Inapps/desktop/src/components/SignInButton.tsx, changevariant={signIn.isPending ? "secondary" : "primary"}to
variant={signIn.isPending ? "gray" : "dark"}No other
variant="secondary"orvariant="lightdark"occurrences remain in the app code.
| `p-4 space-y-3 rounded-lg bg-gray-2 transition-all duration-200`, | ||
| { | ||
| "ring-2 ring-offset-2 hover:bg-gray-2 cursor-default ring-blue-9 ring-offset-gray-100": | ||
| props.isSelected, | ||
| "ring-2 ring-transparent ring-offset-transparent hover:bg-gray-3 cursor-pointer": | ||
| !props.isSelected, | ||
| }, | ||
| )} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Make the option a real button and fix ring offset token.
Improves accessibility (keyboard/focus semantics) and uses an existing gray token (1–12 scale) instead of likely-nonexistent gray-100.
Apply this diff:
- <div
- onClick={() => props.onSelect(props.mode)}
- class={cx(
- `p-4 space-y-3 rounded-lg bg-gray-2 transition-all duration-200`,
- {
- "ring-2 ring-offset-2 hover:bg-gray-2 cursor-default ring-blue-9 ring-offset-gray-100":
- props.isSelected,
- "ring-2 ring-transparent ring-offset-transparent hover:bg-gray-3 cursor-pointer":
- !props.isSelected,
- },
- )}
- >
+ <button
+ type="button"
+ onClick={() => props.onSelect(props.mode)}
+ aria-pressed={props.isSelected}
+ class={cx(
+ `p-4 space-y-3 rounded-lg bg-gray-2 transition-all duration-200`,
+ {
+ "ring-2 ring-offset-2 ring-blue-9 ring-offset-gray-2 hover:bg-gray-2 cursor-default":
+ props.isSelected,
+ "ring-2 ring-transparent ring-offset-transparent hover:bg-gray-3 cursor-pointer":
+ !props.isSelected,
+ },
+ )}
+ >And close with:
- </div>
+ </button>📝 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-4 space-y-3 rounded-lg bg-gray-2 transition-all duration-200`, | |
| { | |
| "ring-2 ring-offset-2 hover:bg-gray-2 cursor-default ring-blue-9 ring-offset-gray-100": | |
| props.isSelected, | |
| "ring-2 ring-transparent ring-offset-transparent hover:bg-gray-3 cursor-pointer": | |
| !props.isSelected, | |
| }, | |
| )} | |
| <button | |
| type="button" | |
| onClick={() => props.onSelect(props.mode)} | |
| aria-pressed={props.isSelected} | |
| class={cx( | |
| `p-4 space-y-3 rounded-lg bg-gray-2 transition-all duration-200`, | |
| { | |
| "ring-2 ring-offset-2 ring-blue-9 ring-offset-gray-2 hover:bg-gray-2 cursor-default": | |
| props.isSelected, | |
| "ring-2 ring-transparent ring-offset-transparent hover:bg-gray-3 cursor-pointer": | |
| !props.isSelected, | |
| }, | |
| )} | |
| > | |
| { /* ...option content here... */ } | |
| </button> |
🤖 Prompt for AI Agents
In apps/desktop/src/components/ModeSelect.tsx around lines 20 to 27, change the
option container to be a real <button> element (use type="button", forward
onClick, onKeyDown if present, and set accessible state like
aria-pressed={props.isSelected}) so keyboard/focus semantics are correct, and
adjust classNames: replace the non-existent ring-offset-gray-100 token with the
existing token (e.g., ring-offset-gray-2) and keep the conditional cursor
classes (cursor-default when selected, cursor-pointer when not) while preserving
the ring/ring-offset/ring-blue-9 logic.
| <Button | ||
| type="submit" | ||
| size="md" | ||
| variant="dark" | ||
| disabled={!feedback().trim() || feedback().trim().length < 0} | ||
| class="mt-2 bg-primary text-primary" | ||
| class="mt-2" | ||
| > |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Disabled condition is ineffective; length check is always false.
feedback().trim().length < 0 can never be true. Enforce the same 10-char minimum as the textarea.
Apply this diff:
- disabled={!feedback().trim() || feedback().trim().length < 0}
+ disabled={feedback().trim().length < 10}📝 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 | |
| type="submit" | |
| size="md" | |
| variant="dark" | |
| disabled={!feedback().trim() || feedback().trim().length < 0} | |
| class="mt-2 bg-primary text-primary" | |
| class="mt-2" | |
| > | |
| <Button | |
| type="submit" | |
| size="md" | |
| variant="dark" | |
| disabled={feedback().trim().length < 10} | |
| class="mt-2" | |
| > |
🤖 Prompt for AI Agents
In apps/desktop/src/routes/(window-chrome)/settings/feedback.tsx around lines 65
to 71, the Button's disabled expression uses feedback().trim().length < 0 which
is always false; change the condition to enforce the same 10-character minimum
as the textarea by disabling when feedback is empty or trimmed length is less
than 10 (e.g., use disabled={!feedback().trim() || feedback().trim().length <
10}).
This PR:
Summary by CodeRabbit
New Features
Style
Bug Fixes
Chores