chore(frontend): shadcn v4 upgrade, Alby theme polish, hub logo#2155
Open
stackingsaunter wants to merge 15 commits intogetAlby:masterfrom
Open
chore(frontend): shadcn v4 upgrade, Alby theme polish, hub logo#2155stackingsaunter wants to merge 15 commits intogetAlby:masterfrom
stackingsaunter wants to merge 15 commits intogetAlby:masterfrom
Conversation
- Refresh vendored UI primitives from shadcn registry (radix-ui, lucide, etc.) - Fix components.json tailwind.config for Tailwind v4 CLI - Preserve Hub splits: buttonVariants, badgeVariants, hybrid tooltip + TouchProvider - Keep custom alert/badge variants; sonner uses document dark class - Remove duplicate use-mobile.ts (use use-mobile.tsx only) - Card keeps shadow-sm to match pre-change product default Made-with: Cursor
Contributor
📝 WalkthroughWalkthroughConsolidates Radix imports to the unified Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/package.json (1)
25-41:⚠️ Potential issue | 🟠 MajorComplete the Radix UI migration or remove the consolidated package.
The PR adds the consolidated
radix-uipackage (line 54) while individual@radix-ui/*packages (lines 25-41) remain. However, the migration is incomplete—two files still import from individual packages:
frontend/src/components/stepper.tsxfrontend/src/components/ui/custom/circle-progress.tsxEither migrate these imports to the consolidated package and remove the individual dependencies, or revert the
radix-uiaddition to maintain consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/package.json` around lines 25 - 41, The package.json shows both the consolidated "radix-ui" package and many individual "@radix-ui/*" packages; resolve this by either (A) migrating the two remaining imports in frontend/src/components/stepper.tsx and frontend/src/components/ui/custom/circle-progress.tsx to import from "radix-ui" and then remove the now-unneeded individual "@radix-ui/*" entries from package.json, or (B) revert/remove the consolidated "radix-ui" dependency and keep the existing individual "@radix-ui/*" entries, ensuring import paths in stepper.tsx and circle-progress.tsx match the chosen approach; update package.json and run install to verify no unresolved imports.
🧹 Nitpick comments (2)
frontend/src/components/ui/command.tsx (1)
53-53: Optional: extract the longCommandclass string for maintainability.The inline class list is quite dense; moving it to a local
const(orcva) would make future edits less error-prone.Refactor sketch
+const commandDialogContentClass = + "**:data-[slot=command-input-wrapper]:h-12 [&_[cmdk-group-heading]]:px-2 [&_[cmdk-group-heading]]:font-medium [&_[cmdk-group-heading]]:text-muted-foreground [&_[cmdk-group]]:px-2 [&_[cmdk-group]:not([hidden])_~[cmdk-group]]:pt-0 [&_[cmdk-input-wrapper]_svg]:h-5 [&_[cmdk-input-wrapper]_svg]:w-5 [&_[cmdk-input]]:h-12 [&_[cmdk-item]]:px-2 [&_[cmdk-item]]:py-3 [&_[cmdk-item]_svg]:h-5 [&_[cmdk-item]_svg]:w-5"; ... - <Command className="**:data-[slot=command-input-wrapper]:h-12 [&_[cmdk-group-heading]]:px-2 [&_[cmdk-group-heading]]:font-medium [&_[cmdk-group-heading]]:text-muted-foreground [&_[cmdk-group]]:px-2 [&_[cmdk-group]:not([hidden])_~[cmdk-group]]:pt-0 [&_[cmdk-input-wrapper]_svg]:h-5 [&_[cmdk-input-wrapper]_svg]:w-5 [&_[cmdk-input]]:h-12 [&_[cmdk-item]]:px-2 [&_[cmdk-item]]:py-3 [&_[cmdk-item]_svg]:h-5 [&_[cmdk-item]_svg]:w-5"> + <Command className={commandDialogContentClass}>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/ui/command.tsx` at line 53, The Command component's long inline className string should be extracted to a named constant (e.g., COMMAND_BASE_CLASSES or commandClasses) or a cva utility to improve maintainability; locate the <Command ...> usage in the file (symbol: Command) and replace the inline className value with the constant, defining that constant above the component so future edits only change the single variable rather than the long attribute string.frontend/src/components/ui/alert.tsx (1)
13-15: Remove duplicate variant utilities to reduce class noise.
[&>svg]:text-currentis already in the base class, andborderis also already present globally. Keeping these duplicates in variant strings isn’t harmful, but removing them will make maintenance clearer.♻️ Suggested cleanup
destructive: - "bg-card text-destructive *:data-[slot=alert-description]:text-destructive/90 [&>svg]:text-current", + "bg-card text-destructive *:data-[slot=alert-description]:text-destructive/90", warning: - "border-warning-foreground border text-warning-foreground *:data-[slot=alert-description]:text-warning-foreground/90 [&>svg]:text-current", + "border-warning-foreground text-warning-foreground *:data-[slot=alert-description]:text-warning-foreground/90",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/ui/alert.tsx` around lines 13 - 15, Remove duplicate utilities from the alert variants: in the Alert component's variant definitions (the base class string and the variant map keys like "warning" and "destructive"), eliminate redundant "[&>svg]:text-current" from the "warning" and "destructive" variant values and remove the redundant "border" from the "warning" variant since those are already present in the shared/base class; update the variant strings accordingly so they only contain unique utilities.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/ui/alert-dialog.tsx`:
- Around line 131-145: The Tailwind selector in AlertDialogMedia's className is
invalid; replace the malformed `*:[svg:not([class*='size-'])]:size-8` with the
correct descendant form `[&_svg:not([class*='size-'])]:size-8` inside the
cn(...) call in the AlertDialogMedia component so child SVGs without a "size-"
class get size-8 applied.
In `@frontend/src/components/ui/field.tsx`:
- Around line 203-209: Replace the loose equality check and redundant optional
chaining for the deduped errors array: in the block that builds uniqueErrors
(the constant uniqueErrors created from new Map(...).values()), change the
conditional if (uniqueErrors?.length == 1) to use strict equality and no
optional chaining — i.e. reference uniqueErrors directly and use ===; keep
returning uniqueErrors[0]?.message (you may keep the inner optional chaining on
the message itself). This updates the check around uniqueErrors to if
(uniqueErrors.length === 1) while leaving the rest of the dedup logic unchanged.
In `@frontend/src/components/ui/popover.tsx`:
- Around line 56-63: PopoverTitle currently declares props as
React.ComponentProps<"h2"> but returns a <div>, causing a type mismatch; fix by
making the rendered element match the prop type: change the returned element to
an <h2> (keeping data-slot="popover-title", className={cn("font-medium",
className)} and {...props}) so h2-specific props are applied correctly, or
alternatively change the prop type to React.ComponentProps<"div"> if you intend
to keep a div—update the signature and ensure className and {...props} stay
intact on the chosen element.
---
Outside diff comments:
In `@frontend/package.json`:
- Around line 25-41: The package.json shows both the consolidated "radix-ui"
package and many individual "@radix-ui/*" packages; resolve this by either (A)
migrating the two remaining imports in frontend/src/components/stepper.tsx and
frontend/src/components/ui/custom/circle-progress.tsx to import from "radix-ui"
and then remove the now-unneeded individual "@radix-ui/*" entries from
package.json, or (B) revert/remove the consolidated "radix-ui" dependency and
keep the existing individual "@radix-ui/*" entries, ensuring import paths in
stepper.tsx and circle-progress.tsx match the chosen approach; update
package.json and run install to verify no unresolved imports.
---
Nitpick comments:
In `@frontend/src/components/ui/alert.tsx`:
- Around line 13-15: Remove duplicate utilities from the alert variants: in the
Alert component's variant definitions (the base class string and the variant map
keys like "warning" and "destructive"), eliminate redundant
"[&>svg]:text-current" from the "warning" and "destructive" variant values and
remove the redundant "border" from the "warning" variant since those are already
present in the shared/base class; update the variant strings accordingly so they
only contain unique utilities.
In `@frontend/src/components/ui/command.tsx`:
- Line 53: The Command component's long inline className string should be
extracted to a named constant (e.g., COMMAND_BASE_CLASSES or commandClasses) or
a cva utility to improve maintainability; locate the <Command ...> usage in the
file (symbol: Command) and replace the inline className value with the constant,
defining that constant above the component so future edits only change the
single variable rather than the long attribute string.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e80baa84-97a7-4183-8282-c11cf91ceae1
⛔ Files ignored due to path filters (1)
frontend/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (38)
frontend/components.jsonfrontend/package.jsonfrontend/src/components/ui/accordion.tsxfrontend/src/components/ui/alert-dialog.tsxfrontend/src/components/ui/alert.tsxfrontend/src/components/ui/avatar.tsxfrontend/src/components/ui/badge.tsxfrontend/src/components/ui/badgeVariants.tsxfrontend/src/components/ui/breadcrumb.tsxfrontend/src/components/ui/button.tsxfrontend/src/components/ui/buttonVariants.tsxfrontend/src/components/ui/calendar.tsxfrontend/src/components/ui/card.tsxfrontend/src/components/ui/carousel.tsxfrontend/src/components/ui/checkbox.tsxfrontend/src/components/ui/command.tsxfrontend/src/components/ui/dialog.tsxfrontend/src/components/ui/dropdown-menu.tsxfrontend/src/components/ui/field.tsxfrontend/src/components/ui/input.tsxfrontend/src/components/ui/label.tsxfrontend/src/components/ui/navigation-menu.tsxfrontend/src/components/ui/pagination.tsxfrontend/src/components/ui/popover.tsxfrontend/src/components/ui/progress.tsxfrontend/src/components/ui/radio-group.tsxfrontend/src/components/ui/select.tsxfrontend/src/components/ui/separator.tsxfrontend/src/components/ui/sheet.tsxfrontend/src/components/ui/sidebar.tsxfrontend/src/components/ui/skeleton.tsxfrontend/src/components/ui/sonner.tsxfrontend/src/components/ui/switch.tsxfrontend/src/components/ui/table.tsxfrontend/src/components/ui/tabs.tsxfrontend/src/components/ui/textarea.tsxfrontend/src/components/ui/tooltip.tsxfrontend/src/hooks/use-mobile.ts
💤 Files with no reviewable changes (1)
- frontend/src/hooks/use-mobile.ts
- Alby: lighter primary gradient, 1px stroke, stacked focus ring - Alby: neutral gray palette; dark primary button label matches light - Hub logo: default/alby light & dark fills; mono for other themes - Figtree for alby; map tailwind font-sans via --app-font-sans - Link buttons: data-slot/data-variant for themed primary styles Made-with: Cursor
stackingsaunter
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/ExternalLink.tsx`:
- Around line 5-12: The Props type currently exposes all AnchorHTMLAttributes
allowing callers to override component-owned attributes and to pass anchor-only
props into the non-HTTP <span> branch; change Props to explicitly omit "target",
"rel", and "onClick" (and keep omitting "href"|"className"|"children") from the
forwarded attributes so those are owned by the component (e.g. Props =
React.PropsWithChildren<{to: string; className?: string; onClick?:
React.MouseEventHandler}>&
Omit<React.AnchorHTMLAttributes<HTMLAnchorElement>,'href'|'className'|'children'|'target'|'rel'|'onClick'>),
and when rendering the non-HTTP branch use appropriate HTMLElement attributes
(React.HTMLAttributes<HTMLSpanElement>) rather than anchor-only attributes;
ensure the component applies its internal target/rel/onClick after the spread
or, better, relies on the typing omission so callers cannot override
target/rel/onClick for ExternalLink.
In `@frontend/src/components/icons/AlbyHubLogo.tsx`:
- Around line 61-70: The logic in logoPalette incorrectly uses invert ||
isDarkMode so invert forces the dark palette regardless of current mode; update
the darkAppearance calculation in logoPalette to flip relative to the current
mode (use an XOR-style check: darkAppearance = isDarkMode !== invert or
equivalently darkAppearance = invert ? !isDarkMode : isDarkMode) so that when
invert is true the palette is the opposite of isDarkMode and you still return
"default-alby-dark" or "default-alby-light" accordingly; leave the
isDefaultOrAlbyTheme early return unchanged.
In `@frontend/src/themes/default.css`:
- Around line 8-10: The root CSS variable --app-font-sans currently falls back
to serif; update its fallback to a proper sans-serif stack so Tailwind's
font-sans (which now references --app-font-sans) doesn't revert to serif when
"Inter var" is unavailable—replace the trailing serif fallback with a sensible
sans stack (e.g. system UI and common sans fonts) in the --app-font-sans
declaration.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4ca21131-240d-42f2-b870-b00baa974ffa
⛔ Files ignored due to path filters (1)
frontend/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (10)
frontend/package.jsonfrontend/src/components/AppSidebar.tsxfrontend/src/components/ExternalLink.tsxfrontend/src/components/icons/AlbyHubLogo.tsxfrontend/src/components/layouts/TwoColumnFullScreenLayout.tsxfrontend/src/components/ui/custom/external-link-button.tsxfrontend/src/components/ui/custom/link-button.tsxfrontend/src/themes/alby.cssfrontend/src/themes/default.cssfrontend/src/themes/index.css
✅ Files skipped from review due to trivial changes (1)
- frontend/src/themes/index.css
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/package.json
Select Xcode.app for CGO on macOS so Wails WebKit compiles. Skip Apple signing/notarize/DMG when the PR head is a fork (no secrets); upload an unsigned zip for Wails fork PRs. Set fail-fast false on the HTTP build matrix so Linux jobs finish if macOS fails. Made-with: Cursor
For workflow_call runs, require inputs.build-release for macOS signing. Fork PR logic unchanged. Made-with: Cursor
Revert workflow_call gating and fork unsigned logic. Remove Xcode selection step added for CGO builds. Made-with: Cursor
Contributor
|
Please don't mix 10 different things into one PR, this is impossible to review. (especially any updates to libraries should be separate from any features) |
reneaaron
approved these changes
Mar 25, 2026
Contributor
reneaaron
left a comment
reneaaron
left a comment
There was a problem hiding this comment.
Took a close look and fixed / improved a lot of things that were just very hard to maintain or not consistent overall.
Overall some nice changes, the Alby theme with the updated fonts / buttons for sure looks nice! 💯
tACK
| "prepare": "cd .. && husky frontend/.husky" | ||
| }, | ||
| "dependencies": { | ||
| "@fontsource-variable/figtree": "^5.2.10", |
Contributor
There was a problem hiding this comment.
Wow, that's cool :) Never heard about that before...
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/themes/alby.css`:
- Around line 92-123: The three complex selectors using chained :not() (for
example :is(button, a,
span)[data-slot="button"][data-variant="default"]:not(:disabled):not([aria-disabled="true"])
and their :hover and :focus-visible variants) violate the stylelint
"selector-not-notation: complex" rule; replace chained :not() calls with a
single :not() that contains a comma-separated list like :not(:disabled,
[aria-disabled="true"]) in each selector (update the base selector, its :hover
and its :focus-visible variants accordingly) so each selector uses the complex
:not(:disabled, [aria-disabled="true"]) form.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4f70fc1f-364a-4ac3-afa0-e84cc8e31609
⛔ Files ignored due to path filters (3)
frontend/public/fonts/Inter-italic.var.woff2is excluded by!**/*.woff2frontend/public/fonts/Inter-roman.var.woff2is excluded by!**/*.woff2frontend/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (10)
frontend/package.jsonfrontend/src/components/AppSidebar.tsxfrontend/src/components/icons/AlbyHubLogo.tsxfrontend/src/components/layouts/TwoColumnFullScreenLayout.tsxfrontend/src/components/ui/sonner.tsxfrontend/src/fonts.cssfrontend/src/main.tsxfrontend/src/themes/alby.cssfrontend/src/themes/default.cssfrontend/src/themes/index.css
💤 Files with no reviewable changes (2)
- frontend/src/main.tsx
- frontend/src/fonts.css
✅ Files skipped from review due to trivial changes (1)
- frontend/src/components/AppSidebar.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/src/themes/index.css
- frontend/src/components/ui/sonner.tsx
- frontend/package.json
Comment on lines
+92
to
+123
| .theme-alby :is(button, a, span)[data-slot="button"][data-variant="default"]:not(:disabled):not( | ||
| [aria-disabled="true"] | ||
| ) { | ||
| background: | ||
| linear-gradient( | ||
| 180deg, | ||
| rgba(255, 255, 255, 0.58) 0%, | ||
| rgba(255, 236, 175, 0.22) 100% | ||
| ), | ||
| #fff0b8; | ||
| box-shadow: 0 0 0 1px #ffdf6f; | ||
| } | ||
|
|
||
| --destructive: hsl(0 84% 60%); | ||
| .theme-alby | ||
| :is(button, a, span)[data-slot="button"][data-variant="default"]:hover:not(:disabled):not( | ||
| [aria-disabled="true"] | ||
| ) { | ||
| background: | ||
| linear-gradient( | ||
| 180deg, | ||
| rgba(255, 255, 255, 0.45) 0%, | ||
| rgba(255, 224, 140, 0.28) 100% | ||
| ), | ||
| #ffe9a3; | ||
| box-shadow: 0 0 0 1px #ffdf6f; | ||
| } | ||
|
|
||
| --border: hsl(0 0% 15%); | ||
| --input: hsl(0 0% 15%); | ||
| --ring: hsl(47 100% 40%); | ||
| /* Keep keyboard focus ring (Button uses ring via box-shadow); stack with brand stroke. */ | ||
| .theme-alby | ||
| :is(button, a, span)[data-slot="button"][data-variant="default"]:focus-visible:not(:disabled):not( | ||
| [aria-disabled="true"] | ||
| ) { |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and examine the relevant lines
fd "alby.css" --type fRepository: getAlby/hub
Length of output: 83
🌐 Web query:
Stylelint selector-not-notation rule CSS :not() pseudo-class behavior
💡 Result:
The Stylelint rule "selector-not-notation" controls the notation used for the CSS :not pseudo-class selectors, enforcing either "simple" or "complex" notation based on Selectors Level 3 vs. Level 4 specifications. "simple" (Selectors Level 3, backwards compatible): Allows only a single simple selector inside :not, e.g. :not(a), :not(.foo). Disallows complex selectors like :not(a.foo) or lists like :not(a, div). Multiple conditions must use chained :nots, e.g. :not(a):not(div), which affects specificity (e.g. 0,2,1 vs. 0,1,1 for complex). "complex" (Selectors Level 4, modern): Allows complex selectors like :not(a.foo) and selector lists like :not(a, div), :not(a).foo:not(:empty). The rule is autofixable for most cases, converting between notations (with caveats for specificity changes). Configuration examples: { "selector-not-notation": "simple" } { "selector-not-notation": "complex" }
Citations:
- 1: https://stylelint.io/user-guide/rules/selector-not-notation/
- 2: https://www.stylelint.cn/user-guide/rules/selector-not-notation
- 3: Add selector-not-notation stylelint/stylelint#5974
🏁 Script executed:
# Examine lines 92-123 of alby.css
sed -n '92,123p' frontend/src/themes/alby.cssRepository: getAlby/hub
Length of output: 975
🏁 Script executed:
# Find stylelint configuration files
fd "stylelint" -type f
fd ".stylelintrc" -type f
find . -maxdepth 2 -name "*stylelint*" -type fRepository: getAlby/hub
Length of output: 433
🏁 Script executed:
# Read the stylelint configuration
cat ./.stylelintrc.jsonRepository: getAlby/hub
Length of output: 1061
🌐 Web query:
stylelint-config-standard-scss selector-not-notation default value
💡 Result:
The default value for the "selector-not-notation" rule in stylelint-config-standard-scss is "complex".
Citations:
- 1: https://github.com/stylelint/stylelint-config-standard/blob/HEAD/index.js
- 2: https://github.com/stylelint-scss/stylelint-config-standard-scss/blob/main/index.js
- 3: https://github.com/stylelint-scss/stylelint-config-standard-scss
Use the complex :not() form so Stylelint passes.
These three selectors violate the selector-not-notation: complex rule (the default in stylelint-config-standard-scss). The complex notation requires combining multiple selectors in a single :not() with a comma-separated list, not chained :not() pseudo-classes.
💡 Suggested selector rewrite
- .theme-alby :is(button, a, span)[data-slot="button"][data-variant="default"]:not(:disabled):not(
- [aria-disabled="true"]
- ) {
+ .theme-alby :is(button, a, span)[data-slot="button"][data-variant="default"]:not(
+ :disabled,
+ [aria-disabled="true"]
+ ) {
@@
- :is(button, a, span)[data-slot="button"][data-variant="default"]:hover:not(:disabled):not(
- [aria-disabled="true"]
- ) {
+ :is(button, a, span)[data-slot="button"][data-variant="default"]:hover:not(
+ :disabled,
+ [aria-disabled="true"]
+ ) {
@@
- :is(button, a, span)[data-slot="button"][data-variant="default"]:focus-visible:not(:disabled):not(
- [aria-disabled="true"]
- ) {
+ :is(button, a, span)[data-slot="button"][data-variant="default"]:focus-visible:not(
+ :disabled,
+ [aria-disabled="true"]
+ ) {📝 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
| .theme-alby :is(button, a, span)[data-slot="button"][data-variant="default"]:not(:disabled):not( | |
| [aria-disabled="true"] | |
| ) { | |
| background: | |
| linear-gradient( | |
| 180deg, | |
| rgba(255, 255, 255, 0.58) 0%, | |
| rgba(255, 236, 175, 0.22) 100% | |
| ), | |
| #fff0b8; | |
| box-shadow: 0 0 0 1px #ffdf6f; | |
| } | |
| --destructive: hsl(0 84% 60%); | |
| .theme-alby | |
| :is(button, a, span)[data-slot="button"][data-variant="default"]:hover:not(:disabled):not( | |
| [aria-disabled="true"] | |
| ) { | |
| background: | |
| linear-gradient( | |
| 180deg, | |
| rgba(255, 255, 255, 0.45) 0%, | |
| rgba(255, 224, 140, 0.28) 100% | |
| ), | |
| #ffe9a3; | |
| box-shadow: 0 0 0 1px #ffdf6f; | |
| } | |
| --border: hsl(0 0% 15%); | |
| --input: hsl(0 0% 15%); | |
| --ring: hsl(47 100% 40%); | |
| /* Keep keyboard focus ring (Button uses ring via box-shadow); stack with brand stroke. */ | |
| .theme-alby | |
| :is(button, a, span)[data-slot="button"][data-variant="default"]:focus-visible:not(:disabled):not( | |
| [aria-disabled="true"] | |
| ) { | |
| .theme-alby :is(button, a, span)[data-slot="button"][data-variant="default"]:not( | |
| :disabled, | |
| [aria-disabled="true"] | |
| ) { | |
| background: | |
| linear-gradient( | |
| 180deg, | |
| rgba(255, 255, 255, 0.58) 0%, | |
| rgba(255, 236, 175, 0.22) 100% | |
| ), | |
| `#fff0b8`; | |
| box-shadow: 0 0 0 1px `#ffdf6f`; | |
| } | |
| .theme-alby | |
| :is(button, a, span)[data-slot="button"][data-variant="default"]:hover:not( | |
| :disabled, | |
| [aria-disabled="true"] | |
| ) { | |
| background: | |
| linear-gradient( | |
| 180deg, | |
| rgba(255, 255, 255, 0.45) 0%, | |
| rgba(255, 224, 140, 0.28) 100% | |
| ), | |
| `#ffe9a3`; | |
| box-shadow: 0 0 0 1px `#ffdf6f`; | |
| } | |
| /* Keep keyboard focus ring (Button uses ring via box-shadow); stack with brand stroke. */ | |
| .theme-alby | |
| :is(button, a, span)[data-slot="button"][data-variant="default"]:focus-visible:not( | |
| :disabled, | |
| [aria-disabled="true"] | |
| ) { |
🧰 Tools
🪛 Stylelint (17.4.0)
[error] 92-94: Expected complex :not() pseudo-class notation (selector-not-notation)
(selector-not-notation)
[error] 106-108: Expected complex :not() pseudo-class notation (selector-not-notation)
(selector-not-notation)
[error] 121-123: Expected complex :not() pseudo-class notation (selector-not-notation)
(selector-not-notation)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/themes/alby.css` around lines 92 - 123, The three complex
selectors using chained :not() (for example :is(button, a,
span)[data-slot="button"][data-variant="default"]:not(:disabled):not([aria-disabled="true"])
and their :hover and :focus-visible variants) violate the stylelint
"selector-not-notation: complex" rule; replace chained :not() calls with a
single :not() that contains a comma-separated list like :not(:disabled,
[aria-disabled="true"]) in each selector (update the base selector, its :hover
and its :focus-visible variants accordingly) so each selector uses the complex
:not(:disabled, [aria-disabled="true"]) form.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/src/components/stepper.tsx (1)
235-237: Widen thePanelref node type for polymorphicasChildrendering.The
Panelcomponent supports theasChildprop, which allows rendering viaSlot.Root(a polymorphic component). WhenasChild=true, non-divelements can be rendered, makingHTMLDivElement | nulloverly specific. Widen both the ref callback andscrollIntoStepperPanelto acceptHTMLElement | nullfor type accuracy and forward-compatibility, sincescrollIntoView()works on any element type.Proposed change
- ref={(node: HTMLDivElement | null) => + ref={(node: HTMLElement | null) => scrollIntoStepperPanel(node, tracking) }-function scrollIntoStepperPanel( - node: HTMLDivElement | null, +function scrollIntoStepperPanel( + node: HTMLElement | null, tracking?: boolean ) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/stepper.tsx` around lines 235 - 237, The ref callback currently types the node as HTMLDivElement | null which is too specific for polymorphic asChild rendering; update the ref signature and the scrollIntoStepperPanel function to accept HTMLElement | null instead of HTMLDivElement | null so non-div elements (e.g., Slot.Root) are supported and scrollIntoView can be called; specifically change the ref={(node: HTMLDivElement | null) => scrollIntoStepperPanel(node, tracking)} typing and the scrollIntoStepperPanel parameter type to HTMLElement | null (preserving null handling).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@frontend/src/components/stepper.tsx`:
- Around line 235-237: The ref callback currently types the node as
HTMLDivElement | null which is too specific for polymorphic asChild rendering;
update the ref signature and the scrollIntoStepperPanel function to accept
HTMLElement | null instead of HTMLDivElement | null so non-div elements (e.g.,
Slot.Root) are supported and scrollIntoView can be called; specifically change
the ref={(node: HTMLDivElement | null) => scrollIntoStepperPanel(node,
tracking)} typing and the scrollIntoStepperPanel parameter type to HTMLElement |
null (preserving null handling).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8f5a2f47-cf90-4d10-b25b-eb7745bed61b
⛔ Files ignored due to path filters (1)
frontend/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (3)
frontend/package.jsonfrontend/src/components/stepper.tsxfrontend/src/components/ui/custom/circle-progress.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/package.json
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
2 participants
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
shadcn / Tailwind v4
components.json, Tailwind v4–compatible setup).radix-ui,lucide-react, etc.).buttonVariants/badgeVariants, hybrid tooltip +TouchProvider, custom variants,sonnertied to documentdarkclass.shadow-smto match currentmasterdefault.Alby theme & branding
#ffdf6fring, stackedfocus-visiblering with theme--ring.@fontsource-variable/figtree; Tailwindfont-sansfollows per-theme--app-font-sans(default.css/index.css).Hub wordmark (
AlbyHubLogo)#202020/#ffc800light; white /#ffe480dark); other themes use monochromecurrentColor.invertforces dark-surface variant on default/alby (e.g. intro column).Link-as-button styling
ExternalLink/LinkButton/ExternalLinkButtonforwarddata-slot/data-variantso themed primary button CSS applies.Test plan
cd frontend && yarn lintSummary by CodeRabbit
New Features
Improvements
Bug Fixes