fix: class-merge regressions across buttons, forms, overlays, tabs#498
Merged
kelsos merged 26 commits intorotki:mainfrom Apr 20, 2026
Merged
fix: class-merge regressions across buttons, forms, overlays, tabs#498kelsos merged 26 commits intorotki:mainfrom
kelsos merged 26 commits intorotki:mainfrom
Conversation
… dark mode The grey color base applies `dark:bg-rui-grey-300`, but the compound variant for text/outlined/list only overrode it with `bg-transparent` (no dark: prefix). Since both utilities have equal specificity in the flat tv() output, the dark: rule won in dark mode, leaving text and outlined grey buttons with a light-grey fill. The pre-migration SCSS avoided this via nested-selector specificity; the flat tv() output needs an explicit `dark:bg-transparent`.
tailwind-merge was classifying both `text-rui-*` and custom typography utilities (`text-body-*`, `text-caption`, `text-overline`, `text-h*`, `text-subtitle-*`) as font-size, treating them as conflicts and silently dropping the color. This stripped `text-rui-text` from rendered class strings wherever a tv() slot combined it with a typography utility — most visibly in data table body cells, which ended up rendering with inherited black text in dark mode. Register `text-rui-*` in the text-color group and the typography utilities in the font-size group so tw-merge keeps them as non-conflicting classes.
The regex used to extract the valid icon list only matched `const RuiIcons = [...];`, but the bundled `dist/icons/index.js` emits `var RuiIcons = [...]`. Parsing silently failed, leaving validIcons empty — every consumer-supplied `include` entry was rejected as "not a valid RuiIcon" and no icons ended up registered, producing runtime "Icon not found" warnings. Accept `const`/`var`/`let` and make the trailing semicolon optional.
The text-field and text-input labels used Tailwind's named `text-base` and `text-xs` utilities, which emit both `font-size` and `line-height` in one rule. The library's own `leading-[...]` variant classes relied on appearing later in the cascade than `text-*` to override the bundled line-height. That worked inside the library's shipped `style.css`. In consumer apps, Tailwind's JIT scans only the consumer's own source and regenerates `.text-base` / `.text-xs` into a later-loaded bundle. Since the consumer never emits the library's arbitrary `leading-[2.5]` etc., the consumer's `.text-base` wins the cascade and pins the label's line-height to `1.5rem`, throwing off vertical alignment. The fieldset legend was hit by the same bug, causing its top border to visibly drop on focus. Switch the label/legend to arbitrary `text-[1rem]` / `text-[0.75rem]` so only font-size is emitted. `leading-*` stays authoritative in both states regardless of consumer cascade order. Also align the autocomplete float label with the text-field active label by giving it an explicit `leading-4` (16px) and removing the extra `px-1` so its text starts at the same x-offset.
…pairs Audit of the library found four more places pairing a named Tailwind text-* utility with a conflicting leading-* override. Named text-* emits both font-size and line-height, so the consumer's later-loaded `.text-*` rule wins the cascade and overrides the library's intended line-height — the same class of bug that affected the text-field label and the fieldset legend. Swap the named utility to its arbitrary equivalent (`text-[Xrem]`) so only font-size is emitted and the library's `leading-*` stays authoritative: - buttons: `size: 'lg'` uses `text-base leading-5` (wanted 1.25rem) - tables: `RuiTableHead.columnText` uses `text-sm leading-6` (wanted 1.5rem) - text-area: base label `text-base` overridden by variant `leading-[3.2]` / `leading-[2.5]` - text-area: active label `text-xs leading-tight` (wanted 1.25)
The `selection` slot signature declared `chipAttrs` as optional, but one of the two slot call sites already passed it and the value is always computed via `chipAttrs(item, i)`. Consumers binding the prop to a child component hit `Record<string, unknown> | undefined` and had to narrow. Pass `chipAttrs` at the other call site (inside <RuiChip>) too and drop the optional marker so the slot contract matches reality.
RuiMenu's activator slot exposes `attrs` via `v-bind="{ attrs: menuAttrs, ... }"`.
Vue-tsc infers the slot prop type from the v-bind value, not from the
`defineSlots` annotation. The two branches of `menuAttrs` (and
`baseMenuAttrs`) returned structurally different shapes (`{}` vs
`{ onMouseover, onMouseleave, onClick }`), so the inferred slot type
collapsed to a discriminated union where `attrs` is either "all handlers
defined" or "all handlers undefined".
Consumers typing their own slot with individually-optional handlers
(`{ onMouseover?: () => void; ... }`) don't match either arm and hit
an assignability error.
Return the same keys from every branch (with `undefined` when disabled)
so the inferred type is a single object shape with each handler
independently nullable, matching the `defineSlots` contract.
`defineModel<number | string>()` without a default infers the generated `update:modelValue` payload as `value?: number | string`, forcing consumers who type their `@update:model-value` handler to accept `undefined` or narrow at the call site. The component already used `0` as an internal fallback (see the `getCurrentIndex` computation), so declaring `default: 0` aligns the runtime behavior with the public type and narrows the emit payload to `value: number | string` without changing any observable behavior for existing consumers — they already got `0` when no value was provided.
useAutoCompleteKeyboardNavigation returned focusedValueIndex wrapped in readonly(), but useAutoCompleteFocus writes to it via `set(deps.focusedValueIndex, -1)` in onInputFocused. The write silently failed in production and spammed "Set operation on key 'value' failed: target is readonly" warnings in dev. Return the raw ref (with an eslint-disable for @rotki/composable-return-readonly, matching the precedent in search.ts) and widen the return type from Readonly<Ref<number>> to Ref<number> so the contract matches the actual shared-mutable-state usage between the two composables.
The autocomplete/menu-select/date-time-picker root wrapper shipped `w-full inline-flex flex-col`. Consumer classes like `w-[20rem]` land on the same element via `$attrs`, but without tw-merge both utilities reach the stylesheet and cascade order decides the winner. Post-tv() refactor the library no longer bundles these utilities in its own CSS — they're regenerated by the consumer's Tailwind, where `w-full` happens to come last and silently pins the element to parent width. Symptom: rotki's EVM-accounts filter bar had `w-[20rem]`/`w-[25rem]` on TagFilter and TableFilter, but both rendered at the parent's 192px and wrapped to two lines. Switch to block-level `flex flex-col`: the element still fills its parent by default, but a consumer-provided width utility is no longer competing with a library one on the same element. Text-field/text-area aren't affected — they apply `ui.wrapper()` to an inner div, so consumer classes on the root never collide.
The button root carried `relative` unconditionally as a positioning context for the absolute-positioned loading spinner. Consumers that pin a button with `fixed` (e.g. a FAB anchored to `bottom-4 right-4`) passed the position utility via class; the library's `relative` and the consumer's `fixed` both landed on the same element, and cascade order — `relative` emitted later — demoted the consumer's positioning to relative offsets. The rotki scroll-to-top FAB ended up 16px left of its flow position instead of 16px from the viewport right. The spinner is only rendered `v-if="loading"`, so move `relative` to the `loading: true` variant. The positioning context exists exactly when the spinner needs it; the rest of the time the button respects whatever `position` utility the consumer applied.
Color variants set `dark:text-rui-text` (white) for dark mode so filled buttons render a light label over their colored background. The outlined/text/list compound then paints `text-rui-<color>` to match the outline or underline — but only in light mode, because `dark:text-rui-text` has the more specific prefix and wins under the `.dark` selector. Outlined primary/secondary/etc. dropped to a white label in dark mode while the outline kept its themed color, visually disconnecting text from border. Add a `dark:text-rui-<color>` sibling in each outlined/text/list compound so the label matches the outline in both themes.
Consumer classes passed to RuiButton (e.g. `class="w-full justify-start text-left"`
on a dropdown suggestion row) landed on the same element as the tv-generated
`ui.root()` output via Vue's separate `:class` merging. No tw-merge meant
both `justify-center` (library base) and `justify-start` (consumer) shipped
to the stylesheet, and cascade order picked `justify-center`. The rotki
TableFilter suggestion rows rendered centered instead of left-aligned.
Pass `$attrs.class` through `ui.root({ class })` so tailwind-variants'
twMerge deduplicates conflicting utilities and the later (consumer) value
wins. `$attrs.class` is still rebound via `v-bind` with the class key
stripped so Vue doesn't re-concatenate it onto the element.
Also widen `cn()`'s signature to accept Vue's runtime `ClassValue` so
`$attrs.class` can be passed directly without casting — Vue's type allows
`false` for conditional bindings and we were rejecting it.
The disabled bg/text overrides and the focus-visible ring used the `!disabled:` / `!focus-visible:` modifier-important ordering, which Tailwind does not recognize — none of those utilities compiled to CSS. Grep of the bundled stylesheet returned zero rules matching `\!disabled\:`, so disabled buttons kept their variant color/bg instead of fading, and the focus ring was never applied at all. Swap to `disabled:!bg-…` / `focus-visible:!ring-…` (modifier first, important second) so the utilities actually land in CSS. Also add `disabled:cursor-not-allowed` so disabled buttons show the expected pointer instead of the browser default. Outlined/text/list need an explicit `dark:disabled:!bg-transparent` sibling to the light-mode `disabled:!bg-transparent`: the base root's `dark:disabled:!bg-white/[.12]` carries the `.dark` ancestor and out- specifies a plain `disabled:!bg-transparent` on the variant. Pairing the two matches specificity and keeps non-filled disabled buttons transparent in both themes.
RuiButton sets `disabled = disabled || loading`, so the disabled bg/
text/outline utilities fired for loading buttons too. Loading primary
buttons lost their primary bg, loading outlined buttons lost their
themed outline color, and everything rendered as the grey Material
disabled palette — with the spinner sitting on top. In dark mode this
showed up as a white outline flash because `rui-text-disabled` is 50%
white under `.dark`.
Move the disabled fade into `{ loading: false, ... }` compounds, so
they only fire when the button is disabled for real. The variant
colors stay intact during loading, the spinner reads against the
same background the button had before, and pure disabled buttons
still get the greyed-out appearance.
Also switch the loading cursor to `cursor-progress` with `!important`
so it beats the base `disabled:cursor-not-allowed` — communicates
"busy" rather than "blocked" while the spinner is up.
Commit 16e87a2 bumped button padding to MD3 spec (px-6 py-2.5 at md, px-8 py-3 at lg, px-4 py-1.5 at sm), which widened every button by 8px per side and disrupted visual density that existing rotki layouts were tuned around. At 24h/10v the medium button reads as oversized next to adjacent form controls. Restore the v2.12.2 scale: md px-4 py-1.5 (16/6) sm px-2.5 py-1 (10/4) lg px-6 py-2 (24/8) text px-2 text sm px-1.5 text lg px-2.5 Keep leading-5 and text size tokens from 16e87a2 — only padding reverts. MD3 alignment is not load-bearing here; matching the existing rotki look is more important.
Vertical tabs were locked to `!h-[3rem]` (48px). The rotki exchange switcher renders each tab as a logo (40px square) plus a label underneath — content ~92px tall — so everything above the 48px ceiling got clipped by the tab bar's `overflow-auto`, slicing the top third off the exchange logo. Swap `!h` for `!min-h` so simple text tabs still land on the 48px floor (matching horizontal layout) while content-heavy tabs grow to fit instead of overflowing into the scroll container's clip.
Earlier commit e9c8287 dropped `w-full inline-flex` from the activator wrapper to stop the library from stomping on consumer width utilities. That fixed the rotki EVM filter case but broke the fill-parent default: inside a flex-row parent (rotki's staking ETH location select, card bodies rendered as `display:flex`), the wrapper collapsed to content width instead of filling the available space — the "Select location" dropdown shrank from full row width to ~118px of label text. Put `w-full inline-flex flex-col` back on the wrapper so it fills its container across every parent layout, and route `$attrs.class` through `ui.wrapper({ class: cn($attrs.class) })` in RuiAutoComplete, RuiMenuSelect, and RuiDateTimePicker. tailwind-variants' twMerge now deduplicates a consumer `w-[20rem]` against the library's `w-full`, so the consumer wins cleanly instead of both landing on the element and letting cascade order decide. Drops `class` from the `v-bind` fallthrough (`getRootAttrs($attrs, [])`) so Vue doesn't re-concatenate the raw consumer class after tv has merged it.
`px-1` (4px) hugged the digit/text too tight, especially on 3-digit counts where the characters touched the rounded edge. Bump to `px-1.5` (6px) on the content slot and `px-2.5` (10px) when the badge holds both an icon and text, so the content sits a bit off the border.
The activator root had `class="relative inline-flex"` hardcoded and merged with `$attrs.class` via Vue's default attr inheritance, so a consumer override like `class="block flex-1 max-w-full"` (rotki's TableFilter wrapper reserving a 400px flex slot for the Combined filters autocomplete) landed on the same element. Cascade order picked `inline-flex` and the tooltip collapsed to content width, shrinking the autocomplete inside it to ~48px. Disable attr inheritance, declare a tv() `rootStyle` for the base `relative inline-flex`, and pass `$attrs.class` through it so the twMerge step deduplicates conflicting `display` utilities.
The notification root had `class="top-2 right-2 fixed ..."` hardcoded plus `v-bind="$attrs"` so a consumer override like `class="top-[3.5rem] z-[10000]"` (rotki's NotificationPopup, pushing the popup below the 56px app header) landed on the same element. Cascade order picked `top-2` and the popup floated up under the header instead of below it. Route the base classes plus `$attrs.class` through a tv() `rootStyle` and strip class from the `v-bind` fallthrough so tailwind-variants' twMerge deduplicates the conflicting `top-*` utilities.
`:class="ui.root(...)"` and `v-bind="$attrs"` both landed on the root
div, so a consumer `class="h-auto"` collided with the base
`h-full w-full` and cascade order picked the library value. Cards
ignored consumer sizing overrides on the outer wrapper.
Strip `class` from the `v-bind` fallthrough and merge `$attrs.class`
into `ui.root({ class })` (falling back through `classNames?.root`)
so twMerge deduplicates conflicting size/layout utilities and the
consumer wins.
The skeleton root had variant-driven sizing like `w-full h-4`/`w-6 h-6`
from `skeletonType({ type })`, and Vue concatenated `$attrs.class` on
top via `v-bind="$attrs"`. A consumer `class="w-48"` or `class="h-8"`
collided with the base width/height and cascade order picked the
variant's value, so consumer sizing silently no-op'd.
Pass `$attrs.class` through tv's `class` parameter
(`skeletonType({ type, class: cn(attrs.class) })`) so twMerge
deduplicates width/height utilities and the consumer wins. Strip
class from the `v-bind` fallthrough so Vue doesn't re-concatenate.
Root had `class="flex flex-col items-start"` hardcoded with `v-bind="$attrs"` on top; a consumer `class="flex-row"` or `class="items-center"` hit the same element and the base won via cascade order. Route the base and `$attrs.class` (falling back to `classNames?.root`) through a tv() `rootStyle` so twMerge deduplicates conflicting `flex-*` / `items-*` utilities. Drop class from the `v-bind` spread so Vue doesn't re-concatenate.
RuiColorPicker, RuiColorBoard, and RuiColorHue each had hardcoded layout/sizing utilities on their root div (`relative`, `w-full h-40`, `w-full h-3.5`, etc.) combined with `v-bind="$attrs"`. A consumer sizing override like `class="h-24"` hit the same element and lost to cascade order. Declare a `rootStyle` tv() for each, route `$attrs.class` through its `class` param, and strip class from the `v-bind` fallthrough so twMerge deduplicates conflicting width/height/display utilities.
`hasExpandedItemSlot = computed(() => !!slots['expanded-item'])` captured its first-render snapshot and never re-evaluated. Vue's `slots` object is not reactive to property access inside a `computed`, so a consumer that conditionally registers the slot — e.g. rotki's `<template v-if="anyExpansion" #expanded-item>` on AccountBalancesTable, where `anyExpansion` starts false while rows load — got stuck with `expandable` permanently false. Clicking the chevron updated `expanded` and the button's `aria-expanded` flipped, but the expanded row never rendered because the `<tr v-if="expandable && ...">` guard saw a stale `false`. - Move the slot-presence check inline to the template (`v-if="expandable && !!$slots['expanded-item'] && isExpanded(...)"`) so every render reads fresh slot state. - Drop `hasExpandedItemSlot` from `useTableExpansion`'s deps and simplify `expandable` to `!!get(expanded)` — its only job now is to reflect whether the consumer wired a two-way `expanded` model. Existing unit tests were updated to match; the replaced "no slot → not expandable" case no longer applies because that gating moved into the template. E2e coverage stays in place because the example app always renders `#expanded-item` unconditionally — the regression only showed up under conditional-slot consumers.
kelsos
force-pushed
the
fix/dark-mode-table-and-button
branch
from
13ca52e to
1686f65
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #498 +/- ##
==========================================
- Coverage 85.05% 84.59% -0.46%
==========================================
Files 139 139
Lines 5051 5057 +6
Branches 1500 1502 +2
==========================================
- Hits 4296 4278 -18
- Misses 755 779 +24 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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
Tightens up a family of regressions that appeared after the tv() / inline-Tailwind refactor and the MD3 spacing alignment. They share one root cause: library classes and consumer classes landed on the same element without
tw-merge, so CSS cascade order (not theclassNamestring) decided which utility won — and the consumer usually lost. This PR fixes each instance where that bit us, plus a couple of orthogonal dark-mode / padding issues noticed along the way.Class-merge fixes (tv's
twMergenow dedupes)RuiButton— consumerjustify-startlost to basejustify-center;fixed bottom-4 right-4FAB lost to baserelativeRuiAutoComplete/RuiMenuSelect/RuiDateTimePicker— consumerw-[20rem]lost to wrapperw-full; wrapper also stayed fill-parent by defaultRuiTooltip— consumerblock flex-1 max-w-fulllost to baseinline-flex, collapsing rotki's TableFilterRuiNotification— consumertop-[3.5rem]lost to basetop-2, floating the popup under the app headerRuiCard— consumerh-autolost to baseh-full w-fullRuiSkeletonLoader— consumerw-48lost to variantw-fullRuiAccordion— consumerflex-*/items-*lost to baseflex flex-col items-startRuiColorPicker/RuiColorBoard/RuiColorHue— consumer sizing lost to hardcodedw-full h-*Button state fixes
!disabled:*modifier-important syntax never generated CSS; swapped todisabled:!*dark:text-rui-textfrom color variant was overriding the outlinedtext-rui-<color>; addeddark:text-rui-<color>per compound{ loading: false, ... }compounds so loading buttons no longer greyscale the spinner backgroundcursor-progressduring loading,cursor-not-allowedwhen truly disabledOther
RuiTabvertical layout:!h-[3rem]→!min-h-[3rem]so tabs with logo+label (rotki's Kraken exchange tab) stop clipping the logopx-6 py-2.5/px-8 py-3/px-4 py-1.5to v2.12.2's tighterpx-4 py-1.5/px-6 py-2/px-2.5 py-1— MD3 alignment broke existing rotki layoutsRuiBadgecontent padding:px-1→px-1.5(andpx-2→px-2.5with icon+text)cn()signature widened to Vue's runtimeClassValueso callers can pass$attrs.classwithout castingTest plan
w-[20rem]/w-[25rem]