improvements: export button widths, sign out/in button hover colors #924
Conversation
WalkthroughUI adjustments across desktop app and shared Button component: SignInButton now always uses primary variant when idle; ExportDialog updates icon sizes, title text, and layout classes; UI Solid Button variants tweak hover colors and overhaul “white” styling. No API/signature changes or control-flow modifications. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 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. ✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/ui-solid/src/Button.tsx (1)
14-17: Clean up duplicate hover and conflicting disabled classes in primary variantThe string sets both hover:bg-gray-11 and enabled:hover:bg-gray-11, and also sets disabled:bg twice (gray-6 and gray-4). The latter wins due to order, but this is confusing and brittle. Prefer a single enabled:hover and a single disabled:bg for clarity and predictability.
- "bg-gray-12 text-gray-1 hover:bg-gray-11 enabled:hover:bg-gray-11 disabled:bg-gray-6 disabled:text-gray-9 outline-blue-300 disabled:bg-gray-4 disabled:dark:text-gray-9", + "bg-gray-12 text-gray-1 enabled:hover:bg-gray-11 disabled:bg-gray-6 disabled:text-gray-9 outline-blue-300 disabled:dark:text-gray-9",apps/desktop/src/routes/editor/ExportDialog.tsx (2)
171-175: Let icons inherit text color instead of hard-coding text-gray-1These icons always render as text-gray-1. Since they sit inside a Button whose text color already matches the variant, letting them inherit currentColor reduces future coupling to palette changes.
- file: <IconCapFile class="text-gray-1 size-3.5" />, - clipboard: <IconCapCopy class="text-gray-1 size-3.5" />, - link: <IconCapLink class="text-gray-1 size-3.5" />, + file: <IconCapFile class="size-3.5" />, + clipboard: <IconCapCopy class="size-3.5" />, + link: <IconCapLink class="size-3.5" />,
483-485: Make full-width option buttons resilient to overflowtext-nowrap prevents wrapping, but long labels could overflow. Add min-w-0 on the Button and truncate the label for safe ellipsis within flexible layouts.
- class={cx( - "flex flex-1 gap-2 items-center text-nowrap", - settings.exportTo === option.value && selectedStyle, - )} + class={cx( + "flex flex-1 gap-2 items-center text-nowrap min-w-0", + settings.exportTo === option.value && selectedStyle, + )}Additionally (outside these lines), wrap the label:
<Button ...> {option.icon} <span class="truncate">{option.label}</span> </Button>apps/desktop/src/components/SignInButton.tsx (2)
12-17: If variant is intentionally controlled, omit it from the public propsThe component now overrides any caller-provided variant. To avoid a misleading API, remove variant from the accepted props type. This documents the opinionated styling and prevents accidental no-op props.
-export function SignInButton( - props: Omit<ComponentProps<typeof Button>, "onClick">, -) { +export function SignInButton( + props: Omit<ComponentProps<typeof Button>, "onClick" | "variant">, +) {
12-27: Optional: expose pending state to assistive techConsider adding aria-busy to communicate the pending state to screen readers.
- <Button + <Button + aria-busy={signIn.isPending ? "true" : "false"}
📜 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 (3)
apps/desktop/src/components/SignInButton.tsx(1 hunks)apps/desktop/src/routes/editor/ExportDialog.tsx(4 hunks)packages/ui-solid/src/Button.tsx(1 hunks)
⏰ 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). (4)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (rust)
🔇 Additional comments (3)
packages/ui-solid/src/Button.tsx (1)
20-24: Audit: ∼17 usages ofvariant="white"detected — renaming will be a breaking change, proceed with cautionThe “white” variant is now a dark-style button (
bg-gray-12/text-gray-1), which conflicts with its name and may confuse downstream consumers. Our search found approximately 17 call sites across both web and desktop apps, including components likeCommercialGetStarted,SpeedController,TrimmingTool,AboutPage,BlogTemplate,HomePage/Header,MediaFormatConverter,FeaturesPage,recordings-overlay, severalTranscripttabs,ShareHeader,CapCard,CapCardButtons, andInviteAccept.• Renaming this variant (e.g. to “dark” or “neutral”) will require updating every occurrence above.
• To avoid breaking changes, consider introducing a new key (e.g.variant="neutral") and aliasingwhiteto it withinButton.tsx(deprecatingwhiteover time).
• Independently, ensure disabled buttons never receive hover styles by usingenabled:hoverinstead of plainhover.Suggested minimal hover fix in
packages/ui-solid/src/Button.tsx:- "bg-gray-12 hover:bg-gray-11 text-gray-1 dark:disabled:bg-gray-300 dark:disabled:text-gray-8 dark:text-gray-1 disabled:bg-gray-6 disabled:text-gray-9 outline-blue-300", + "bg-gray-12 enabled:hover:bg-gray-11 text-gray-1 dark:disabled:bg-gray-300 dark:disabled:text-gray-8 dark:text-gray-1 disabled:bg-gray-6 disabled:text-gray-9 outline-blue-300",Please verify all
variant="white"usages and decide on a deprecation/aliasing strategy to minimize downstream impact.apps/desktop/src/routes/editor/ExportDialog.tsx (2)
375-375: Dialog title update looks good“Export Cap” clarifies scope and matches the product name.
72-88: size-3.5 utility is already supported—no changes neededI ran the verification script and confirmed:
- Tailwind CSS v3.x is in use across all packages (apps/web → ^3, packages/ui → ^3.3.3, packages/ui-solid → ^3.4.10).
- The
3.5spacing token is already being applied in dozens of existing icon usages (both in web and desktop code).- The desktop and web Tailwind configs extend a shared baseConfig, which includes the
spacing[‘3.5’]value.Since
size-3.5is a widely-used, recognized class and hasn’t been dropped at build time, you can leave it as-is.
This PR:
Summary by CodeRabbit