mirror of
https://github.com/we-promise/sure.git
synced 2026-05-07 12:54:04 +00:00
fix(design-system): replace undefined utility classes and broken /N modifiers (#1660)
* fix(design-system): replace undefined utility classes and broken /N modifiers
Audit of class-name resolution in views surfaced two related silent
failures across ~17 files:
1. Class names that don't exist anywhere in the design system. Tailwind
silently drops them and the element renders with no CSS for that
property.
- bg-primary (and bg-primary/5, /10, /90): never defined as a
custom utility, no --color-primary in @theme. Used as a CTA bg
in 8 places, all rendered transparent.
- text-inverted: typo of text-inverse.
- text-primary-foreground: shadcn/Radix vocabulary, not in our
token system.
- bg-accent / border-accent / text-accent: same shadcn vocabulary;
not defined.
2. Slash modifier (/N) used on custom @utility blocks. Modifiers only
resolve on Tailwind theme colors (anything in tokens.json color.*).
Custom @utility blocks compile to static @apply statements and
silently drop the /N variant. Affected uses:
- border-surface-inset/50 across provider account selectors.
- border-secondary/30, /40 in admin SSO form and simplefin setup.
- bg-surface-inset/30, /40 in settings preferences and simplefin.
Fixes:
| From | To |
|---------------------------------------------------|------------------------------------------------------|
| bg-primary text-white (and similar primary CTAs) | button-bg-primary text-inverse |
| bg-primary text-primary-foreground (badges) | button-bg-primary text-inverse |
| bg-primary text-inverted (typo) | button-bg-primary text-inverse |
| bg-primary text-primary (broken active pill) | bg-inverse text-inverse |
| bg-primary (status dot) | bg-inverse |
| bg-primary/5, bg-primary/10 (subtle accent bg) | bg-gray-tint-5, bg-gray-tint-10 |
| hover:bg-primary/90 | hover:button-bg-primary-hover |
| border-accent bg-accent/10 text-accent (badges) | border-secondary bg-surface-inset text-secondary |
| border-surface-inset/50 | border-secondary |
| border-secondary/30, /40 | border-tertiary |
| bg-surface-inset/30 | bg-surface-inset (full strength) |
| bg-surface-inset/40 | bg-container-inset |
Also documents the alpha-modifier limitation in design/tokens/README.md
under a new "Alpha modifiers in views (/N syntax)" section, with the
opacity-N convention for custom utilities and a note that the
gray-tint-5 / gray-tint-10 family (and similar pre-resolved tints) are
theme colors and accept /N modifiers natively.
The accent-badge mapping uses neutral semantics for now. A dedicated
brand-accent token (text-link-tint-10 etc.) is worth considering as a
follow-up if the "highlighted metadata badge" pattern recurs.
* fix(design-system): replace undefined divide-primary / divide-secondary with alpha tokens
Same class of bug as the rest of this PR: divide-{name} requires the
name to be a theme color (i.e. expose --color-{name}), and our custom
@utility utilities (primary, secondary, etc.) do not. Tailwind silently
drops the unrecognized class and rows render with no separator.
Spotted six instances during the visual audit:
- admin/users/index.html.erb (×2): users table + pending invitations
- admin/sso_providers/index.html.erb (×2): configured + legacy lists
- transactions/categorizes/_transaction_list.html.erb: categorize sidebar
- settings/preferences/show.html.erb: divide-secondary/60 (also broken)
Swapped to the alpha-black/white pattern already used elsewhere in the
codebase (imports/cleans/show, transactions/_summary, etc.):
divide-y divide-primary
-> divide-y divide-alpha-black-200 theme-dark:divide-alpha-white-200
divide-y divide-secondary/60
-> divide-y divide-alpha-black-100 theme-dark:divide-alpha-white-100
The lighter (-100) variant on the preferences list matches the original
intent of /60 (more subtle).
This commit is contained in:
committed by
GitHub
parent
05ef8bd9e7
commit
2bcdf6c554
@@ -1,4 +1,4 @@
|
||||
<details class="group bg-surface rounded-lg border border-surface-inset/50">
|
||||
<details class="group bg-surface rounded-lg border border-secondary">
|
||||
<summary class="flex items-center justify-between gap-2 p-3 cursor-pointer">
|
||||
<div class="flex items-center gap-2">
|
||||
<%= helpers.icon "chevron-right", class: "group-open:transform group-open:rotate-90" %>
|
||||
|
||||
@@ -119,7 +119,7 @@
|
||||
|
||||
<details class="mt-4">
|
||||
<summary class="cursor-pointer text-sm font-medium text-secondary hover:text-primary"><%= t("admin.sso_providers.form.manual_saml_config") %></summary>
|
||||
<div class="mt-3 space-y-3 pl-4 border-l-2 border-secondary/30">
|
||||
<div class="mt-3 space-y-3 pl-4 border-l-2 border-tertiary">
|
||||
<p class="text-xs text-secondary"><%= t("admin.sso_providers.form.manual_saml_help") %></p>
|
||||
|
||||
<div>
|
||||
@@ -202,7 +202,7 @@
|
||||
|
||||
<details class="mt-4">
|
||||
<summary class="cursor-pointer text-sm font-medium text-secondary hover:text-primary"><%= t("admin.sso_providers.form.role_mapping_title") %></summary>
|
||||
<div class="mt-3 space-y-3 pl-4 border-l-2 border-secondary/30">
|
||||
<div class="mt-3 space-y-3 pl-4 border-l-2 border-tertiary">
|
||||
<p class="text-xs text-secondary"><%= t("admin.sso_providers.form.role_mapping_help") %></p>
|
||||
|
||||
<div>
|
||||
@@ -284,7 +284,7 @@
|
||||
<div class="flex gap-3">
|
||||
<%= link_to "Cancel", admin_sso_providers_path, class: "px-4 py-2 text-sm font-medium text-secondary hover:text-primary" %>
|
||||
<%= form.submit sso_provider.persisted? ? "Update Provider" : "Create Provider",
|
||||
class: "px-4 py-2 bg-primary text-inverse rounded-lg text-sm font-medium hover:bg-primary/90" %>
|
||||
class: "px-4 py-2 button-bg-primary text-inverse rounded-lg text-sm font-medium hover:button-bg-primary-hover" %>
|
||||
</div>
|
||||
</div>
|
||||
<% end %>
|
||||
|
||||
@@ -10,7 +10,7 @@
|
||||
|
||||
<%= settings_section title: "Configured Providers" do %>
|
||||
<% if @sso_providers.any? %>
|
||||
<div class="divide-y divide-primary">
|
||||
<div class="divide-y divide-alpha-black-200 theme-dark:divide-alpha-white-200">
|
||||
<% @sso_providers.each do |provider| %>
|
||||
<div class="flex items-center justify-between py-3 first:pt-0 last:pb-0">
|
||||
<div class="flex items-center gap-3">
|
||||
@@ -73,7 +73,7 @@
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<div class="divide-y divide-primary">
|
||||
<div class="divide-y divide-alpha-black-200 theme-dark:divide-alpha-white-200">
|
||||
<% @legacy_providers.each do |provider| %>
|
||||
<div class="flex items-center justify-between py-3 first:pt-0 last:pb-0">
|
||||
<div class="flex items-center gap-3">
|
||||
|
||||
@@ -95,7 +95,7 @@
|
||||
<th class="px-4 py-2 text-right text-xs font-medium text-secondary uppercase"><%= t(".table.role") %></th>
|
||||
</tr>
|
||||
</thead>
|
||||
<tbody class="divide-y divide-primary">
|
||||
<tbody class="divide-y divide-alpha-black-200 theme-dark:divide-alpha-white-200">
|
||||
<% users.each do |user| %>
|
||||
<tr>
|
||||
<td class="px-4 py-3">
|
||||
@@ -137,7 +137,7 @@
|
||||
<% end %>
|
||||
</tbody>
|
||||
<% if pending_invitations.any? %>
|
||||
<tbody class="divide-y divide-primary border-t border-dashed border-primary">
|
||||
<tbody class="divide-y divide-alpha-black-200 theme-dark:divide-alpha-white-200 border-t border-dashed border-primary">
|
||||
<% pending_invitations.each do |invitation| %>
|
||||
<tr class="bg-red-50/30 dark:bg-red-950/20">
|
||||
<td class="px-4 py-3">
|
||||
|
||||
@@ -17,7 +17,7 @@
|
||||
<%= hidden_field_tag :account_id, @account.id %>
|
||||
<div class="space-y-2 max-h-64 overflow-auto">
|
||||
<% @available_binance_accounts.each do |ba| %>
|
||||
<label class="flex items-center gap-3 p-2 rounded border border-surface-inset/50 hover:border-primary cursor-pointer">
|
||||
<label class="flex items-center gap-3 p-2 rounded border border-secondary hover:border-primary cursor-pointer">
|
||||
<%= radio_button_tag :binance_account_id, ba.id, false %>
|
||||
<div class="flex flex-col">
|
||||
<span class="text-sm text-primary font-medium"><%= ba.name.presence || ba.id %></span>
|
||||
|
||||
@@ -16,7 +16,7 @@
|
||||
step[:is_complete] ? "text-green-600" : "text-secondary"
|
||||
end %>
|
||||
<% step_class = if is_current
|
||||
"bg-primary text-primary"
|
||||
"bg-inverse text-inverse"
|
||||
else
|
||||
step[:is_complete] ? "bg-green-600/10 border-alpha-black-25" : "bg-container-inset"
|
||||
end %>
|
||||
|
||||
@@ -17,7 +17,7 @@
|
||||
<%= hidden_field_tag :account_id, @account.id %>
|
||||
<div class="space-y-2 max-h-64 overflow-auto">
|
||||
<% @available_coinbase_accounts.each do |ca| %>
|
||||
<label class="flex items-center gap-3 p-2 rounded border border-surface-inset/50 hover:border-primary cursor-pointer">
|
||||
<label class="flex items-center gap-3 p-2 rounded border border-secondary hover:border-primary cursor-pointer">
|
||||
<%= radio_button_tag :coinbase_account_id, ca.id, false %>
|
||||
<div class="flex flex-col">
|
||||
<span class="text-sm text-primary font-medium"><%= ca.name.presence || ca.account_id %></span>
|
||||
|
||||
@@ -17,7 +17,7 @@
|
||||
<%= hidden_field_tag :account_id, @account.id %>
|
||||
<div class="space-y-2 max-h-64 overflow-auto">
|
||||
<% @available_enable_banking_accounts.each do |eba| %>
|
||||
<label class="flex items-center gap-3 p-2 rounded border border-surface-inset/50 hover:border-primary cursor-pointer">
|
||||
<label class="flex items-center gap-3 p-2 rounded border border-secondary hover:border-primary cursor-pointer">
|
||||
<%= radio_button_tag :enable_banking_account_id, eba.id, false %>
|
||||
<div class="flex flex-col">
|
||||
<span class="text-sm text-primary font-medium"><%= eba.name.presence || eba.account_id %></span>
|
||||
|
||||
@@ -40,7 +40,7 @@
|
||||
</div>
|
||||
<% end %>
|
||||
|
||||
<div class="absolute inset-0 bg-primary/5 pointer-events-none opacity-0 transition-opacity duration-150 ease-in-out z-0" data-mobile-cell-interaction-target="highlight"></div>
|
||||
<div class="absolute inset-0 bg-gray-tint-5 pointer-events-none opacity-0 transition-opacity duration-150 ease-in-out z-0" data-mobile-cell-interaction-target="highlight"></div>
|
||||
</div>
|
||||
<% end %>
|
||||
<% end %>
|
||||
|
||||
@@ -6,7 +6,7 @@
|
||||
<div class="flex items-center gap-2">
|
||||
<%= icon "chevron-right", class: "group-open:transform group-open:rotate-90" %>
|
||||
|
||||
<div class="flex items-center justify-center h-8 w-8 bg-primary/10 rounded-full">
|
||||
<div class="flex items-center justify-center h-8 w-8 bg-gray-tint-10 rounded-full">
|
||||
<div class="flex items-center justify-center">
|
||||
<%= tag.p indexa_capital_item.name.first.upcase, class: "text-primary text-xs font-medium" %>
|
||||
</div>
|
||||
|
||||
@@ -15,7 +15,7 @@
|
||||
We're building a richer onboarding journey to learn about your goals, milestones, and day-to-day needs. For now, head over to the chat sidebar to start a conversation with Sure and let us know where you are in your financial journey.
|
||||
</p>
|
||||
<div>
|
||||
<%= link_to "Start chatting", chats_path, class: "inline-flex items-center gap-2 px-4 py-2 rounded-lg bg-primary text-white font-medium" %>
|
||||
<%= link_to "Start chatting", chats_path, class: "inline-flex items-center gap-2 px-4 py-2 rounded-lg button-bg-primary text-inverse font-medium" %>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
@@ -94,7 +94,7 @@
|
||||
<h4 class="font-medium text-primary mb-3">Permissions</h4>
|
||||
<div class="flex flex-wrap gap-2">
|
||||
<% @current_api_key.scopes.each do |scope| %>
|
||||
<span class="inline-flex items-center gap-1 px-2 py-1 bg-primary text-primary-foreground rounded-full text-xs font-medium">
|
||||
<span class="inline-flex items-center gap-1 px-2 py-1 button-bg-primary text-inverse rounded-full text-xs font-medium">
|
||||
<%= icon("shield-check", class: "w-3 h-3") %>
|
||||
<%= case scope
|
||||
when "read" then "Read Only"
|
||||
|
||||
@@ -48,7 +48,7 @@
|
||||
data-currency-preferences-selected-count-translations-value="<%= selected_count_translations.to_json %>">
|
||||
<% additional_preview_currencies = @user.family.secondary_enabled_currency_objects.first(6) %>
|
||||
<% additional_currency_count = @user.family.secondary_enabled_currency_objects.count %>
|
||||
<div class="rounded-xl border border-secondary p-4 bg-surface-inset/40">
|
||||
<div class="rounded-xl border border-secondary p-4 bg-container-inset">
|
||||
<div class="grid gap-4 md:grid-cols-2">
|
||||
<div>
|
||||
<p class="text-xs font-medium uppercase tracking-wide text-secondary"><%= t(".base_currency_label") %></p>
|
||||
@@ -139,7 +139,7 @@
|
||||
<%= icon("search", class: "text-secondary") %>
|
||||
</div>
|
||||
</div>
|
||||
<div data-list-filter-target="list" class="max-h-80 overflow-auto rounded-xl border border-secondary divide-y divide-secondary/60">
|
||||
<div data-list-filter-target="list" class="max-h-80 overflow-auto rounded-xl border border-secondary divide-y divide-alpha-black-100 theme-dark:divide-alpha-white-100">
|
||||
<p class="hidden px-4 py-3 text-sm text-secondary" data-list-filter-target="emptyMessage">
|
||||
<%= t(".no_matching_currencies") %>
|
||||
</p>
|
||||
@@ -147,8 +147,8 @@
|
||||
<% checked = selected_currency_codes.include?(currency.iso_code) %>
|
||||
<% base_currency = currency.iso_code == primary_currency_code %>
|
||||
<label class="<%= class_names(
|
||||
"filterable-item flex items-start gap-3 px-4 py-3 cursor-pointer transition-colors hover:bg-surface-inset/30",
|
||||
"bg-surface-inset/30": base_currency,
|
||||
"filterable-item flex items-start gap-3 px-4 py-3 cursor-pointer transition-colors hover:bg-surface-inset",
|
||||
"bg-surface-inset": base_currency,
|
||||
"cursor-default": base_currency
|
||||
) %>"
|
||||
data-filter-name="<%= "#{currency.iso_code} #{currency.name}" %>">
|
||||
|
||||
@@ -116,7 +116,7 @@
|
||||
<div class="flex items-center justify-between p-3 rounded-lg bg-container border border-primary">
|
||||
<div class="flex items-center gap-3">
|
||||
<% if item.syncing? %>
|
||||
<div class="w-2 h-2 bg-primary rounded-full animate-pulse"></div>
|
||||
<div class="w-2 h-2 bg-inverse rounded-full animate-pulse"></div>
|
||||
<div>
|
||||
<p class="text-sm font-medium text-primary"><%= item.aspsp_name || t("settings.providers.enable_banking_panel.syncing", default: "Syncing") %></p>
|
||||
<p class="text-xs text-secondary"><%= t("settings.providers.enable_banking_panel.syncing", default: "Syncing") %></p>
|
||||
|
||||
@@ -17,7 +17,7 @@
|
||||
<%= hidden_field_tag :account_id, @account.id %>
|
||||
<div class="space-y-2 max-h-64 overflow-auto">
|
||||
<% @available_simplefin_accounts.each do |sfa| %>
|
||||
<label class="flex items-center gap-3 p-2 rounded border border-surface-inset/50 hover:border-primary cursor-pointer">
|
||||
<label class="flex items-center gap-3 p-2 rounded border border-secondary hover:border-primary cursor-pointer">
|
||||
<%= radio_button_tag :simplefin_account_id, sfa.id, false %>
|
||||
<div class="flex flex-col">
|
||||
<span class="text-sm text-primary font-medium"><%= sfa.name.presence || sfa.account_id %></span>
|
||||
|
||||
@@ -78,7 +78,7 @@
|
||||
<% card_class = if needs_subtype_attention
|
||||
"border-2 border-warning bg-warning/5"
|
||||
elsif likely_closed
|
||||
"border border-secondary/40 bg-surface-inset/30"
|
||||
"border border-tertiary bg-surface-inset"
|
||||
else
|
||||
"border border-primary"
|
||||
end %>
|
||||
|
||||
@@ -36,7 +36,7 @@
|
||||
</div>
|
||||
|
||||
<div class="flex items-center gap-1 flex-shrink-0">
|
||||
<span class="inline-flex items-center gap-1 text-xs font-medium rounded-full px-1.5 py-0.5 border border-accent bg-accent/10 text-accent">
|
||||
<span class="inline-flex items-center gap-1 text-xs font-medium rounded-full px-1.5 py-0.5 border border-secondary bg-surface-inset text-secondary">
|
||||
<%= icon "split", size: "sm", color: "current" %>
|
||||
<%= t("transactions.split_parent_row.split_label") %>
|
||||
</span>
|
||||
|
||||
@@ -112,7 +112,7 @@
|
||||
|
||||
<%# Split indicator %>
|
||||
<% if @split_parent_entry_ids ? @split_parent_entry_ids.include?(entry.id) : entry.split_parent? %>
|
||||
<span class="inline-flex items-center gap-1 text-xs font-medium rounded-full px-1.5 py-0.5 border border-accent bg-accent/10 text-accent" title="<%= t("transactions.transaction.split_tooltip") %>">
|
||||
<span class="inline-flex items-center gap-1 text-xs font-medium rounded-full px-1.5 py-0.5 border border-secondary bg-surface-inset text-secondary" title="<%= t("transactions.transaction.split_tooltip") %>">
|
||||
<%= icon "split", size: "sm", color: "current" %>
|
||||
<%= t("transactions.transaction.split") %>
|
||||
</span>
|
||||
|
||||
@@ -10,7 +10,7 @@
|
||||
</div>
|
||||
<%# Rows %>
|
||||
<div class="shadow-border-xs rounded-lg overflow-hidden">
|
||||
<div class="max-h-[32rem] overflow-y-auto divide-y divide-primary text-sm">
|
||||
<div class="max-h-[32rem] overflow-y-auto divide-y divide-alpha-black-200 theme-dark:divide-alpha-white-200 text-sm">
|
||||
<% entries.each do |entry| %>
|
||||
<%= render partial: "transactions/categorizes/entry_row", locals: { entry: entry, categories: categories } %>
|
||||
<% end %>
|
||||
|
||||
@@ -75,7 +75,7 @@
|
||||
data-categorize-target="filterInput"
|
||||
data-action="keydown.enter->categorize#confirmFilterEdit keydown.escape->categorize#cancelFilterEdit">
|
||||
<button type="button"
|
||||
class="flex items-center justify-center w-6 h-6 rounded-md bg-primary text-inverted hover:opacity-80 transition-opacity shrink-0"
|
||||
class="flex items-center justify-center w-6 h-6 rounded-md button-bg-primary text-inverse hover:opacity-80 transition-opacity shrink-0"
|
||||
data-action="click->categorize#confirmFilterEdit">
|
||||
<%= icon("check", size: "sm") %>
|
||||
</button>
|
||||
|
||||
@@ -84,6 +84,19 @@ Anywhere a `$value` is a string:
|
||||
|
||||
The same syntax appears inside composite values like `shadow.xs.$value`: `"0px 1px 2px 0px {color.black|6%}"`.
|
||||
|
||||
### Alpha modifiers in views (`/N` syntax)
|
||||
|
||||
Tailwind v4's `class/N` modifier (`bg-warning/10`, `text-link/70`, etc.) only resolves on **theme colors** (anything declared under the top-level `color.*` group, which becomes `--color-*` in the generated CSS). It does **not** resolve on classes from this file's `utility.*` group, because those compile to static `@apply` blocks with no modifier-aware definition.
|
||||
|
||||
The mismatch is silent — Tailwind drops the unrecognized class and the element renders with no CSS for that property. Recently caught examples on `text-inverse/70`, `border-secondary/30`, and `bg-surface-inset/40` (all of which produced no class output).
|
||||
|
||||
Until the build script teaches custom utilities to be modifier-aware, the convention is:
|
||||
|
||||
- For alpha on a custom utility: pair the base class with `opacity-N`, e.g. `text-inverse opacity-70` instead of `text-inverse/70`.
|
||||
- For alpha on a theme color: the `/N` modifier works as expected, e.g. `bg-warning/10`, `border-destructive/30`.
|
||||
|
||||
The pre-resolved alpha tints (`color.gray.tint-5`, `color.gray.tint-10`, `color.red.tint-5`, etc.) are theme colors, so `bg-gray-tint-5` and similar work as straight utilities and accept further `/N` modifiers.
|
||||
|
||||
### Adding a new token
|
||||
|
||||
1. Pick the right top-level group.
|
||||
|
||||
Reference in New Issue
Block a user