Skip to content

Conversation

@ameer2468
Copy link
Contributor

@ameer2468 ameer2468 commented Aug 28, 2025

This PR:

  • Makes mode selection screen cleaner and simpler.
  • Adjusts button styles of desktop app to match web.
  • Adjusted ring colors of the old recording flow mode selection.

Summary by CodeRabbit

  • New Features

    • Mode selection now uses dynamic icons for clearer visuals.
    • Button styling variants expanded (gray, dark, ghost, darkgradient, radialblue).
  • Style

    • Refreshed button styles across the app for consistent dark/gray theming.
    • Unified ring and shadow styling for improved dark-theme appearance.
  • Bug Fixes

    • Fixed Share modal idle-state reporting.
    • Feedback submit now requires a minimum length (reduces accidental submits).
  • Chores

    • Removed duplicate CSS and unused imports to streamline assets.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 28, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

Updated 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

Cohort / File(s) Summary
Core Button Variants
packages/ui-solid/src/Button.tsx
Overhauled button variant styles: removed secondary/lightdark, added ghost/gray/dark/darkgradient/radialblue, revised primary/blue/destructive/white/outline. Component signature unchanged but accepted visual variants changed.
App-wide Button Usage Updates
apps/desktop/src/components/CapErrorBoundary.tsx, apps/desktop/src/routes/(window-chrome)/new-main/index.tsx, apps/desktop/src/routes/(window-chrome)/settings.tsx, apps/desktop/src/routes/(window-chrome)/settings/feedback.tsx, apps/desktop/src/routes/(window-chrome)/settings/general.tsx, apps/desktop/src/routes/(window-chrome)/settings/integrations/s3-config.tsx, apps/desktop/src/routes/(window-chrome)/settings/license.tsx, apps/desktop/src/routes/(window-chrome)/setup.tsx, apps/desktop/src/routes/(window-chrome)/upgrade.tsx, apps/desktop/src/routes/editor/Editor.tsx, apps/desktop/src/routes/editor/ExportDialog.tsx, apps/desktop/src/routes/editor/Header.tsx, apps/desktop/src/routes/editor/ui.tsx, apps/desktop/src/components/SignInButton.tsx
Replaced deprecated/old variant tokens across many buttons (e.g., secondarygray, lightdarkdark, some primarydark). These are presentation-only changes; event handlers and logic remain the same.
Mode Selection Refactor
apps/desktop/src/components/ModeSelect.tsx, apps/desktop/src/components/Mode.tsx
Switched from image assets to icon-render functions (prop signature now receives { class, style }); removed darkimg/lightimg props; updated container/ring classes (ring-blue-10ring-blue-500) and spacing. Rendering now calls icon(...) with computed class/style.
Share Modal Logic Tweak
apps/desktop/src/routes/editor/ShareButton.tsx
Button variant changed (primarydark) and idle-state check updated from == to === (strict equality).
Theme and Global CSS Shadows
apps/desktop/src/styles/theme.css, apps/web/app/globals.css
Adjusted dark-button-shadow inset color rules (swapped rgba values between selectors), consolidated duplicate .dark-button-shadow declaration in web globals.
Storybook Updates
packages/ui-solid/src/Button.stories.tsx
Story updated to use gray instead of secondary; minor markup/class-order tweaks without functional change.
Minor Import Cleanups
apps/desktop/src/routes/(window-chrome)/upgrade.tsx, apps/desktop/src/routes/editor/Editor.tsx, apps/desktop/src/routes/(window-chrome)/new-main/index.tsx
Removed unused imports (e.g., addEventListener, onMount, others). No behavioral 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
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested labels

codex

Suggested reviewers

  • Brendonovich

Poem

A nibble of gray, a dash of dark—
I hop through styles, leaving a mark.
Icons now shimmer where images stood,
Shadows aligned, the buttons look good.
I twitch my nose and thump with glee—UI hops forward, joyfully. 🐇✨

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 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 298353d and a5595a5.

📒 Files selected for processing (4)
  • apps/desktop/src/components/SignInButton.tsx (1 hunks)
  • apps/desktop/src/routes/(window-chrome)/settings/feedback.tsx (1 hunks)
  • apps/desktop/src/styles/theme.css (1 hunks)
  • packages/ui-solid/src/Button.stories.tsx (5 hunks)
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch update-mode-selection-and-more

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbit in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbit in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbit gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbit read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbit help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbit ignore or @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbit summary or @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbit or @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 changes

This 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 token

Moved 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 types

Replace any in 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.

📥 Commits

Reviewing files that changed from the base of the PR and between c9c2f32 and 298353d.

📒 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.tsx
  • apps/desktop/src/routes/(window-chrome)/settings/feedback.tsx
  • apps/desktop/src/routes/(window-chrome)/settings.tsx
  • apps/desktop/src/routes/(window-chrome)/settings/integrations/s3-config.tsx
  • apps/desktop/src/routes/editor/ui.tsx
  • apps/desktop/src/routes/editor/Header.tsx
  • apps/desktop/src/components/CapErrorBoundary.tsx
  • apps/desktop/src/routes/editor/ExportDialog.tsx
  • apps/desktop/src/routes/editor/Editor.tsx
  • apps/desktop/src/routes/(window-chrome)/settings/general.tsx
  • apps/desktop/src/routes/editor/ShareButton.tsx
  • apps/desktop/src/routes/(window-chrome)/new-main/index.tsx
  • apps/desktop/src/components/ModeSelect.tsx
  • apps/desktop/src/components/Mode.tsx
  • apps/desktop/src/routes/(window-chrome)/settings/license.tsx
  • apps/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.tsx
  • apps/desktop/src/routes/(window-chrome)/settings/feedback.tsx
  • apps/desktop/src/routes/(window-chrome)/settings.tsx
  • apps/desktop/src/routes/(window-chrome)/settings/integrations/s3-config.tsx
  • apps/desktop/src/routes/editor/ui.tsx
  • apps/desktop/src/routes/editor/Header.tsx
  • apps/desktop/src/components/CapErrorBoundary.tsx
  • apps/desktop/src/routes/editor/ExportDialog.tsx
  • apps/desktop/src/routes/editor/Editor.tsx
  • apps/desktop/src/routes/(window-chrome)/settings/general.tsx
  • apps/desktop/src/routes/editor/ShareButton.tsx
  • apps/desktop/src/routes/(window-chrome)/new-main/index.tsx
  • apps/desktop/src/components/ModeSelect.tsx
  • apps/desktop/src/components/Mode.tsx
  • apps/desktop/src/routes/(window-chrome)/settings/license.tsx
  • apps/desktop/src/routes/(window-chrome)/upgrade.tsx
  • packages/ui-solid/src/Button.stories.tsx
  • packages/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.tsx
  • apps/desktop/src/routes/(window-chrome)/settings/feedback.tsx
  • apps/desktop/src/routes/(window-chrome)/settings.tsx
  • apps/desktop/src/routes/(window-chrome)/settings/integrations/s3-config.tsx
  • apps/desktop/src/routes/editor/ui.tsx
  • apps/desktop/src/routes/editor/Header.tsx
  • apps/desktop/src/components/CapErrorBoundary.tsx
  • apps/desktop/src/routes/editor/ExportDialog.tsx
  • apps/desktop/src/routes/editor/Editor.tsx
  • apps/desktop/src/routes/(window-chrome)/settings/general.tsx
  • apps/desktop/src/routes/editor/ShareButton.tsx
  • apps/desktop/src/routes/(window-chrome)/new-main/index.tsx
  • apps/desktop/src/components/ModeSelect.tsx
  • apps/desktop/src/components/Mode.tsx
  • apps/desktop/src/routes/(window-chrome)/settings/license.tsx
  • apps/desktop/src/routes/(window-chrome)/upgrade.tsx
  • packages/ui-solid/src/Button.stories.tsx
  • packages/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 import

Lean 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 adjustments

Updated .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 good

Switch 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 changes

No 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 actions

Looks correct and consistent with the new token set.

apps/desktop/src/routes/editor/ExportDialog.tsx (3)

492-496: Option groups migrated to variant="gray" correctly

Export 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
In apps/desktop/src/components/SignInButton.tsx, change

variant={signIn.isPending ? "secondary" : "primary"}

to

variant={signIn.isPending ? "gray" : "dark"}

No other variant="secondary" or variant="lightdark" occurrences remain in the app code.

Comment on lines +20 to +27
`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,
},
)}
Copy link
Contributor

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.

Suggested change
`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.

Comment on lines 65 to 71
<Button
type="submit"
size="md"
variant="dark"
disabled={!feedback().trim() || feedback().trim().length < 0}
class="mt-2 bg-primary text-primary"
class="mt-2"
>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.

Suggested change
<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}).

@ameer2468 ameer2468 merged commit 0010637 into main Aug 28, 2025
13 of 14 checks passed
@ameer2468 ameer2468 deleted the update-mode-selection-and-more branch August 28, 2025 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants