style(ui): full-product Emil design pass — tokens, focus, motion, a11y#5
Merged
Merged
Conversation
Foundation + product-wide polish following Emil Kowalski's design engineering principles. Visual/interaction only — no behavior changes. Tokens (index.css): elevation scale (shadow-card/popover/overlay), z-index scale, macOS window-control colors, modal/fade keyframes. Shared components: Button gains focus-visible ring, press feedback and a link variant; Modal gains role/aria-modal, focus trap + restore, scroll lock and an entrance animation; Card uses the elevation token (+optional interactive lift); Badge/SearchableSelect/PeriodSelector/ModelCard get focus-visible, ease-out and motion-reduce; dropdowns share the popover shadow + pop-in. Shell & pages: sidebar/onboarding/profiles no longer shift layout on active states (constant font-weight); transition-all replaced with named properties; focus-visible added to nav, list items, tabs and footer buttons; inline accent text-buttons routed through Button link variant. dupe-ignore: register the new "link" variant string (matches existing ghost/secondary entries). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (25)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (9)
🚧 Files skipped from review as they are similar to previous changes (4)
WalkthroughThis PR enhances the UI system with a new link-style button variant, implements full modal accessibility with focus trapping, refactors multiple components to use the shared button system, extends the design theme with new tokens and animations, and systematically improves motion handling across components with motion-reduce support. ChangesUI Enhancement and Accessibility
🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 1
🧹 Nitpick comments (3)
src/shared/components/card.tsx (1)
21-24: ⚡ Quick winVerify Tailwind v4 CSS-variable shadow utility syntax.
On Tailwind v4, these
shadow-[var(--...)]forms may be parsed differently than expected for CSS-variable utilities. If they’re not generated, the card/popover elevation silently disappears.Proposed change if v4 parsing is not generating these classes
- "bg-bg-card rounded-xl border shadow-[var(--shadow-card)]", + "bg-bg-card rounded-xl border shadow-(--shadow-card)", @@ - "transition-shadow duration-200 ease-out motion-reduce:transition-none hover:shadow-[var(--shadow-popover)]", + "transition-shadow duration-200 ease-out motion-reduce:transition-none hover:shadow-(--shadow-popover)",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/shared/components/card.tsx` around lines 21 - 24, The Tailwind arbitrary shadow syntax shadow-[var(--shadow-card)] and hover:shadow-[var(--shadow-popover)] used in the class list inside the Card component (the strings next to featured and interactive in src/shared/components/card.tsx) may not be emitted by Tailwind v4; update by either adding explicit safelist entries or by replacing those arbitrary-variable uses with stable utilities — e.g., create custom utilities/keys like shadow-card and shadow-popover in tailwind.config.js (or add the exact arbitrary class names to the safelist) and then swap the strings "shadow-[var(--shadow-card)]" and "hover:shadow-[var(--shadow-popover)]" to the new utility names so the card/popover elevation is produced reliably.src/shared/components/searchable-select/searchable-select.tsx (1)
136-140: ⚡ Quick winTailwind v4: the current CSS-variable z-index/shadow syntax is valid
z-[var(--z-dropdown)]andshadow-[var(--shadow-popover)]work in Tailwind v4 (they’re equivalent to thez-(--z-dropdown)/shadow-(--shadow-popover)shorthand). Functional change isn’t needed; the shorthand is just a readability improvement.Optional readability refactor
- "animate-pop-in origin-top absolute left-0 right-0 top-full mt-1.5 z-[var(--z-dropdown)]", + "animate-pop-in origin-top absolute left-0 right-0 top-full mt-1.5 z-(--z-dropdown)", - "shadow-[var(--shadow-popover)] p-1", + "shadow-(--shadow-popover) p-1",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/shared/components/searchable-select/searchable-select.tsx` around lines 136 - 140, The Tailwind v4 variable syntax is valid but for readability update the class strings in the SearchableSelect component: replace "z-[var(--z-dropdown)]" with the shorthand "z-(--z-dropdown)" and replace "shadow-[var(--shadow-popover)]" with "shadow-(--shadow-popover)" in the className array used in searchable-select.tsx (the array containing "animate-pop-in origin-top..." and "shadow-[var(--shadow-popover)] p-1") so the semantics remain identical but the classes read cleaner.src/shared/components/modal.tsx (1)
63-82: 💤 Low valueConsider using Tailwind v4 parentheses syntax for CSS variables.
The arbitrary value syntax
z-[var(--z-modal)]andshadow-[var(--shadow-overlay)]works, but Tailwind v4 introduces a cleaner shorthand using parentheses for CSS custom properties.Idiomatic v4 syntax
<div - className="fixed inset-0 z-[var(--z-modal)] flex items-center justify-center" + className="fixed inset-0 z-(--z-modal) flex items-center justify-center" role="dialog" ... <div ref={panelRef} tabIndex={-1} className=" relative bg-bg-primary rounded-2xl border border-border - shadow-[var(--shadow-overlay)] + shadow-(--shadow-overlay) ...🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/shared/components/modal.tsx` around lines 63 - 82, The Tailwind arbitrary-value usage in the modal component should switch to v4 parentheses syntax: replace instances of z-[var(--z-modal)] and shadow-[var(--shadow-overlay)] in the className of the outer role="dialog" div and the inner panel div (the element using ref={panelRef}) with z-(--z-modal) and shadow-(--shadow-overlay) respectively so the CSS custom properties use the idiomatic Tailwind v4 form while leaving the rest of the className values unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/shared/components/modal-focusable-selector.ts`:
- Around line 2-3: The CSS selector modalFocusableSelector currently excludes
disabled buttons but still matches disabled input, textarea, and select elements
causing focus trap calls like focusables?.[0]?.focus() to silently fail; update
modalFocusableSelector to add :not([disabled]) for input, textarea and select
(i.e., ensure each form control in the selector includes :not([disabled])) so
disabled controls are filtered out before focus is applied.
---
Nitpick comments:
In `@src/shared/components/card.tsx`:
- Around line 21-24: The Tailwind arbitrary shadow syntax
shadow-[var(--shadow-card)] and hover:shadow-[var(--shadow-popover)] used in the
class list inside the Card component (the strings next to featured and
interactive in src/shared/components/card.tsx) may not be emitted by Tailwind
v4; update by either adding explicit safelist entries or by replacing those
arbitrary-variable uses with stable utilities — e.g., create custom
utilities/keys like shadow-card and shadow-popover in tailwind.config.js (or add
the exact arbitrary class names to the safelist) and then swap the strings
"shadow-[var(--shadow-card)]" and "hover:shadow-[var(--shadow-popover)]" to the
new utility names so the card/popover elevation is produced reliably.
In `@src/shared/components/modal.tsx`:
- Around line 63-82: The Tailwind arbitrary-value usage in the modal component
should switch to v4 parentheses syntax: replace instances of z-[var(--z-modal)]
and shadow-[var(--shadow-overlay)] in the className of the outer role="dialog"
div and the inner panel div (the element using ref={panelRef}) with
z-(--z-modal) and shadow-(--shadow-overlay) respectively so the CSS custom
properties use the idiomatic Tailwind v4 form while leaving the rest of the
className values unchanged.
In `@src/shared/components/searchable-select/searchable-select.tsx`:
- Around line 136-140: The Tailwind v4 variable syntax is valid but for
readability update the class strings in the SearchableSelect component: replace
"z-[var(--z-dropdown)]" with the shorthand "z-(--z-dropdown)" and replace
"shadow-[var(--shadow-popover)]" with "shadow-(--shadow-popover)" in the
className array used in searchable-select.tsx (the array containing
"animate-pop-in origin-top..." and "shadow-[var(--shadow-popover)] p-1") so the
semantics remain identical but the classes read cleaner.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 207dc473-e2e3-4fd5-b09d-4777d7a6f10c
📒 Files selected for processing (23)
.doctor/dupe-ignoresrc/index.csssrc/layout/sidebar.tsxsrc/onboarding/onboarding-layout.tsxsrc/onboarding/steps/step-microphone.tsxsrc/profiles/list/profile-list-item.tsxsrc/profiles/llm/llm-model-selector.tsxsrc/settings/about/about-section.tsxsrc/settings/general/general-section.tsxsrc/settings/llm/llm-model-download-row.tsxsrc/settings/speech/speech-model-modal.tsxsrc/shared/components/badge/badge.tsxsrc/shared/components/button/button-size-styles.tssrc/shared/components/button/button-variant-styles.tssrc/shared/components/button/button.tsxsrc/shared/components/card.tsxsrc/shared/components/modal-focusable-selector.tssrc/shared/components/modal.tsxsrc/shared/components/model-card/model-card.tsxsrc/shared/components/period-selector.tsxsrc/shared/components/searchable-select/searchable-select.tsxsrc/transcripts/transcript-item.tsxsrc/widget/idle-bar.tsx
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 23 files
Confidence score: 3/5
- There is a concrete user-impacting accessibility risk in
src/shared/components/modal.tsx: initial focus may land on a selector match that is not actually focusable, which can break expected keyboard flow when the modal opens. - Focus restoration in
src/shared/components/modal.tsxappears to run on effect re-renders instead of only on close, so users may experience unexpected focus jumps while the dialog is still open. - Given the medium severity (6/10 and 5/10) and high confidence, this introduces some regression risk in modal interaction behavior before merge.
- Pay close attention to
src/shared/components/modal.tsx- focus targeting and restore timing can cause keyboard navigation instability.
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
Review fixes: - widget.rs: hop NSPanel show/hide to the main thread via run_on_main_thread (calling AppKit from the hotkey background thread crashed on every recording) - toggle: add `ariaLabel` prop so switches with an external label still expose an accessible name; pass it at all three call sites - modal: keep onClose in a ref so focus restoration only runs on close (not on every re-render); make initial focus fall back to the panel; exclude disabled controls from the focus-trap selector Other: - use-fft-bars: gate the rAF loop on `active` so it idles instead of running at 60fps for the app's lifetime (fixes idle CPU/heap churn) - native macOS traffic lights (titleBarStyle Overlay) + premium redesign of Dashboard/Settings (PageHeader, icon stat cards, spacing, type scale) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
1 issue found across 25 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src-tauri/src/window/widget.rs">
<violation number="1" location="src-tauri/src/window/widget.rs:16">
P2: Main-thread dispatch errors are silently ignored, so widget visibility updates can fail without any signal.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| if !panel.is_visible() { | ||
| panel.order_front_regardless(); | ||
| let app = app_handle.clone(); | ||
| let _ = app_handle.run_on_main_thread(move || { |
There was a problem hiding this comment.
P2: Main-thread dispatch errors are silently ignored, so widget visibility updates can fail without any signal.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src-tauri/src/window/widget.rs, line 16:
<comment>Main-thread dispatch errors are silently ignored, so widget visibility updates can fail without any signal.</comment>
<file context>
@@ -7,28 +7,36 @@ use tauri::{AppHandle, LogicalSize, Manager, PhysicalPosition, WebviewWindow};
- if !panel.is_visible() {
- panel.order_front_regardless();
+ let app = app_handle.clone();
+ let _ = app_handle.run_on_main_thread(move || {
+ #[cfg(target_os = "macos")]
+ {
</file context>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A full-product UI polish pass following Emil Kowalski's design-engineering principles, extending the foundational polish from #3. Visual + interaction only — no behavior, data-flow, or copy changes.
What changed
Design tokens (
src/index.css)shadow-card/shadow-popover/shadow-overlay(hairline ring + soft shadow)--z-dropdown/modal/tooltip/toast), macOS window-control colors,fade-in/modal-inkeyframesShared components
focus-visiblering, tactileactive:scalepress, newvariant="link"for inline text actionsrole="dialog"+aria-modal, focus trap + restore, body scroll-lock, entrance animation,--z-modal(focusable selector extracted to its own file per repo convention)interactivehover-liftfocus-visible,ease-out,motion-reduce; dropdowns share the popover shadow +pop-inShell & pages
transition-all→ named properties (transition-[width]for progress bars,transition-colorsfor hovers) everywhere, withmotion-reducefocus-visibleadded to nav links, traffic-light buttons (+aria-label), transcript/profile list items, footer buttonsButton variant="link"tabular-numson numeric badges;…for literal ellipsesVerification
npm run build(tsc + vite) — all new tokens compile (bg-mac-close,shadow-*,z-modalconfirmed in output CSS).doctorsuite green (react + rust); registered the newlinkvariant in.doctor/dupe-ignore🤖 Generated with Claude Code