* fix(views): DS drift sankey tooltip and imports icon token
Replace raw palette classes on the cashflow Sankey tooltip with functional tokens (aligned with time_series_chart). Use bg-surface for the YNAB import option icon background.
Refs #1971, #1951
* fix(views): add privacy-sensitive sankey tooltip class
Align the sankey tooltip with privacy mode masking by appending privacy-sensitive while keeping the DS tokenized tooltip styling.
---------
Signed-off-by: glorydavid03023 <glorydavid03023@gmail.com>
* fix(jobs): delegate recurring-transaction sync gate to Sync.for_family
`IdentifyRecurringTransactionsJob#family_has_incomplete_syncs?` hand-rolled
the list of provider `*_items` associations it polled — plaid, simplefin,
lunchflow, enable_banking, sophtron — missing nine other `Syncable`
provider concerns on `Family`: coinbase, binance, kraken, coinstats,
snaptrade, mercury, brex, indexa_capital, ibkr. When a sync on any of those
nine was in flight, the debounce gate fell through and
`RecurringTransaction::Identifier` ran against a partial dataset; the
follow-up re-enqueue then hit the `find_or_initialize_by` upsert path and
inherited the stale `occurrence_count`. Same drift pattern that bolted
sophtron on as the 5th entry (#591) was already an iteration of.
The maintainers' own `Sync.for_family` (sync.rb:61) already enumerates every
`*_items` association via `Family.reflect_on_all_associations(:has_many)`
filtered by inclusion of `Syncable` — exactly the helper the gate should
delegate to so the list cannot drift again.
- Add `Sync.any_incomplete_for?(family)` class method that wraps
`for_family(family).incomplete.exists?`.
- Rewrite `family_has_incomplete_syncs?` to delegate. 14 lines → 1.
- New test file `test/jobs/identify_recurring_transactions_job_test.rb`
covers in-flight Coinbase + Mercury (gate fires), idle (identifier runs),
missing family, and superseded-by-newer-schedule.
- `test/models/sync_test.rb` gets 2 new tests pinning
`any_incomplete_for?` against a provider `_items` sync and a
family-itself sync.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(jobs): stub Rails.cache.read for supersession test (NullStore in test env)
`Rails.cache` is `ActiveSupport::Cache::NullStore` in the Rails test env, so
the previous test's `Rails.cache.write(cache_key, @scheduled_at + 10, ...)`
was a no-op and `Rails.cache.read(cache_key)` returned `nil`. The
supersession short-circuit `return if latest_scheduled && latest_scheduled
> scheduled_at` then fell through, the job proceeded to invoke
`RecurringTransaction::Identifier`, and the Mocha
`.expects(:identify_recurring_patterns).never` failed in CI.
Switch to `Rails.cache.stubs(:read).with(cache_key).returns(...)` — the
same idiom `test/models/provider/twelve_data_test.rb:186-197` already uses
for the cache layer. Add an `assert_nil` on the bare `perform` return so
Minitest's assertion counter sees an explicit assertion (silences the
"missing assertions" warning).
No production-code change. Behavior under test is unchanged; only the test
mechanism for simulating "newer scheduled run already in cache" is fixed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(binance): add full account sync and transaction processing
- Fixed a bug that hindered Account setup
- Wire up Binance accounts, sync statistics, and unlinked account tracking in the accounts dashboard.
- Support setting a sync_start_date during Binance account setup.
- Set Binance accounts' opening balance to zero to ensure the ledger builds cleanly from the actual trade history.
- Expand the Binance importer and processor to handle Spot, Margin, Earn, P2P, and Futures trades and assets.
- Implement TransactionBuilder to parse raw Binance trades, accurately calculating fees, base/quote asset amounts, and market values for proper ledger integration.
- Update Binance API timeout (`recvWindow`) to 60,000ms to prevent connection drops.
These changes provide comprehensive support for tracking Binance portfolios, ensuring accurate historical ledgers and proper visibility of sync statuses in the frontend dashboard.
* refactor(binance): enforce strong params, double-entry safety, and native fiat currency support
- Implement strong parameters in BinanceItemsController#complete_account_setup to satisfy Rails security guidelines.
- Add robust date parsing with a grace fallback to prevent controller crashes on malformed sync start dates.
- Wrap P2P transaction creations inside a database transaction block to guarantee ledger integrity and prevent orphan records.
- Optimize P2P deduplication queries by batching checks for both transaction and funding external IDs.
- Shift P2P entry persistence from forced USD tracking to native fiat values extracted directly from the Binance API payload.
- Update BinanceAccount::ProcessorTest assertions and fixtures to validate native fiat and fee calculation logic.
* fix(binance): process sync trades before caching transaction payload
- Reorder Binance processor execution to insert trade records into the database prior to updating the `raw_transactions_payload` cache. This guarantees that if a database insertion fails, the cache won't prematurely mark the sync as successful, ensuring the data is retried on the next run.
- Move `set_opening_anchor_balance(balance: 0)` out of the generic crypto exchange account builder and apply it specifically during Binance account creation.
- Refactor date parsing in BinanceItemsController to explicitly catch `ArgumentError` via a block instead of using a blanket inline `rescue`.
- Clean up the `setup_accounts` view template by removing hardcoded default translation strings.
* fix(binance): enhance trade sync logic and error propagation
- Pass `startTime` (from `sync_start_date`) to spot and futures trade endpoints on initial sync to optimize data fetching.
- Include previously synced futures pairs alongside spot pairs when resolving relevant symbols to properly recover sold-out assets.
- Re-raise exceptions in processor rescue blocks to prevent silent failures and ensure errors are correctly propagated to background jobs.
- Decrease Binance API `recvWindow` from 60000ms to 5000ms to align with recommended default timeout values.
* fix(enable_banking): clear stuck pending flag when ASPSP reuses same transaction_id for booked version
* fix: scope pending→booked bypass to user_modified entries only
* refactor: extract clear_pending_flags_from_extra helper to deduplicate pending-flag removal logic
* refactor: use clear_pending_flags_from_extra in user_modified bypass path
* fix(provider_import_adapter): add type check in clear_pending_flags_from_extra
Add a check to ensure that the value associated with a provider key in
the `extra` hash is a Hash before attempting to call `delete` on it.
This prevents a `NoMethodError` when encountering malformed data where
the provider key exists but does not map to a Hash.
* fix(provider_import_adapter): fix indentation and ensure proper return in clear_pending_flags_from_extra
* fix(provider_import_adapter): make clear_pending_flags_from_extra private
* fix: guard clear_pending_flags_from_extra against non-Hash extra values
* fix(holdings): carry provider cost_basis forward to calculated rows
Providers like IBKR Flex emit holdings on report_date and only
include trades within the query window. The reverse calculator + gapfill therefore produces rows past report_date with nil cost_basis, even though the provider supplied a basis on the snapshot. That nil basis silently blanks `Trend`, the Reports "Total Return" card, the Top Holdings return column, and Gains by Tax Treatment, because every one of them gates on `holding.avg_cost`.
When a calculated row would otherwise have no usable cost_basis, backfill it with the most recent provider-supplied cost_basis for the same (security, currency) on or before the holding date. Existing calculated/manual values are preserved (they outrank a provider carry-forward), and existing provider carry-forwards are refreshed when a newer snapshot supersedes them.
* - Fix currency mismatch: provider snapshots were keyed by (security_id,
currency) but calculated rows use account currency while IBKR provider
rows use the security's native currency (e.g., USD vs EUR). Now keyed
by security_id only; carry_forward_provider_cost_basis converts via
Money#exchange_to at the snapshot date (same convention as
ReverseCalculator for trade prices), with a ConversionError fallback.
- Trim long inline comment to three lines
- Fix safe-nav inconsistency: existing.cost_basis.positive? ->
existing&.cost_basis&.positive?
- Add test: refreshes stale carry-forward when a newer provider snapshot
arrives
- Add test: carry-forward is a no-op for forward-strategy accounts with
no provider holdings
* fix(holdings): prevent overwriting zero-valued manual cost basis
Ensure that manual cost basis entries with a value of zero (e.g., for free
shares) are not overwritten by provider carry-forward values during
materialization.
Additionally, updated the logic to allow zero-valued manual or
calculated cost bases to be preserved, and added tests to verify
currency conversion and error handling during cost basis carry-forward.
* refactor(holdings): allow zero-valued cost basis in provider snapshots
Remove the filter that restricted provider cost basis snapshots to values
greater than zero. This ensures that manual cost basis entries with a
value of zero (e.g., for free shares) are correctly captured and
available for carry-forward logic.
* perf(holdings): optimize provider cost basis snapshot lookup
Filter provider cost basis snapshots by the security IDs present in the
current holdings set to reduce the amount of data loaded into memory.
* refactor(holdings): move PortfolioCache FX fix to dedicated branch
Remove date-accurate exchange rate fix from this branch — it has been
split into fix/portfolio-cache-historical-fx-rate to keep concerns
separate.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* revert(portfolio_cache): restore date-accurate FX in get_price
36676784 removed date: date from exchange_to intending to move it to
fix/portfolio-cache-historical-fx-rate, but that branch was a duplicate
of db1051d2 which was already in main. The revert therefore regressed
portfolio_cache.rb below main's state. Restore the historical exchange
rate lookup so this branch no longer removes a fix already present in main.
* fix(portfolio_cache): restore date-accurate FX and its test
36676784 removed date: date from exchange_to and deleted the historical
FX test, intending to carry them in fix/portfolio-cache-historical-fx-rate.
That branch was a duplicate of db1051d2 already in main, so the removal
regressed portfolio_cache.rb below main's state. Restore both.
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(family-sharing): prevent silent data loss when rehoming or removing users
Fixes#1689.
Two destructive paths could strand a pre-existing user's family and accounts:
1. Invitation#accept_for unconditionally overwrote user.family_id, orphaning
the prior family + its accounts with no user able to reach them.
2. Settings::ProfilesController#destroy then called @user.destroy when an admin
removed the rehomed member, destroying the only login path back to the
now-orphaned data.
Add hard-block guards on both paths. accept_for refuses when the invitee
already belongs to a family with accounts; ProfilesController#destroy refuses
when the member owns accounts in another family (legacy state from the old
flow). InvitationsController#create surfaces a specific, actionable flash so
the admin understands why the auto-accept was refused.
No automatic recovery of already-orphaned data — that needs a separate
one-shot script per dosubot's analysis on the issue.
* fix(family-sharing): scope invite orphan-guard to invitee-owned accounts (#1896 review)
Codex flagged (P1) and the maintainer review independently raised that
would_orphan_existing_family? keyed off user.family.accounts.exists? —
any account in the invitee's current family — which wrongly blocked a
non-owner member from leaving a multi-user household.
Rename to would_orphan_owned_accounts? and key off
user.owned_accounts.where.not(family_id: family_id), making the invite
guard symmetric with the destroy-path guard in
Settings::ProfilesController. A member who owns no accounts now orphans
nothing by moving and is free to accept the invitation; an owner is
still blocked.
Add a regression test for the non-owner case and update the existing
tests to give the invitee explicit account ownership.
* Remove extra comments per project conventions
---------
Co-authored-by: Juan José Mata <jjmata@jjmata.com>
* feat(ibkr): compute net_market_flows from IBKR equity delta and trade flows
Replace the hardcoded net_market_flows: 0 in HistoricalBalancesSync with an
exact derivation from IBKR's own equity summary data, eliminating any
dependency on third-party security price providers for Period Return.
Formula: nmf = Δnon_cash - net_buy_sell
- non_cash = IBKR equity total - materializer cash (exact per IBKR)
- net_buy_sell = sum of trade amounts converted to base currency using
the stored fx_rate_to_base (IBKR's own FX rate, already on Trade#exchange_rate)
Sets non_cash_adjustments = net_buy_sell so the virtual column identity
(end_non_cash_balance = start + nmf + adjustments) resolves to IBKR's
exact equity figure.
* test(ibkr): add sell-trade and no-trade nmf tests; fix memoization guard
- Add test: sell trades (negative amount) correctly isolate market loss in nmf
- Add test: no-trade scenario produces nmf = full Δnon_cash
- Fix: `return {} unless account` inside ||= exited the method without memoizing;
restructure to `if account ... else {} end` so the result is always cached
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(ibkr): exclude dividend/interest trades from net_buy_sell; use historical FX date
Addresses two issues flagged in code review:
- P1: trades with qty=0 (Dividend, Interest) were included in net_buy_sell,
inflating/deflating nmf on dates with income events. Filter to qty != 0 at
the SQL level so only buy/sell trades affect the market-flow calculation.
- P2: Money#exchange_to defaulted to Date.current when no custom_rate was
stored, causing historical nmf to drift as FX rates change over time.
Pass date: entry.date so the fallback lookup uses the trade's own date.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* test(ibkr): cover Money::ConversionError fallback in trade_flows_by_date
Adds a test that stubs Money#exchange_to to raise ConversionError for a
cross-currency trade with no stored exchange_rate, verifying that the
rescue clause falls back to entry.amount and that nmf and
end_non_cash_balance still resolve correctly.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(ibkr): log warning when FX conversion falls back to unconverted amount
When Money::ConversionError is raised for a cross-currency trade with no
stored exchange_rate, warn with entry currency, account currency, date,
amount, and entry/account IDs so the silent fallback is visible in logs.
Same-currency ConversionErrors (unexpected but possible) stay silent.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(ibkr): skip unconvertible FX trades, redact log, tighten join
- On Money::ConversionError, skip the entry from net_buy_sell rather
than falling back to the raw amount (which treated e.g. EUR as CHF);
nmf now absorbs the full Δnon_cash for that date instead of silently
misstating period return
- Remove entry amount, entry ID, and account ID from the FX warning log
to avoid exposing financial data in log output
- Consolidate entryable_type guard into the JOIN condition rather than a
separate WHERE clause
- Add inline comment on the first-day zero case to distinguish intent
from a bug
- Update ConversionError test to assert skip behavior (nmf=200, not 50)
* fix(ibkr): exclude dates with unconvertible FX trades from balance upsert
* fix(ibkr): skip upsert_all when all balance rows are filtered by failed FX dates
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix: Avoid overlay in provider section on mobile
* feat: Reduce gap between divs
* fix: keep all the elements inside a dedicated container to avoid accessibility issues with the summary node
* fix(settings): preserve OpenAI form input on validation failure
Fixes#1824.
The OpenAI settings form auto-submits on blur, so typing the URI base
before the model triggers cross-field validation. The rescue re-renders
the page with values read from Setting.openai_*, which is still blank
because the failed save was rejected — so the user's input disappears
and they see 'OpenAI model is required' with no value to fix.
Stash the submitted uri_base and model on rescue and prefer them over
the saved Setting when rendering, so the user can finish typing the
missing field and re-submit.
* test(settings): cover openai_model preservation on validation fail (#1862)
jjmata asked for symmetric coverage of the model field. Add a test where
the user changes the URI base and clears the model in the same submit:
the cross-field validation fails and the re-rendered model input must
reflect the submitted (cleared) value rather than reverting to the saved
model. Complements the existing uri_base preservation test.
* fix(views): clear Rule 2 + Rule 5 findings from weekly DS drift (#1951)
Token swaps + i18n cleanup across the three files flagged in the
weekly merged-commit drift scan.
**`app/views/admin/users/index.html.erb`**
- `bg-green-100 text-green-800` → `bg-success/10 text-success` (2 callsites — active-subscription badge + super_admin role legend)
- `bg-surface-default` → `bg-surface` (`--color-surface-default` isn't defined; canonical token is `--color-surface`)
- `bg-red-50/30 dark:bg-red-950/20` → `bg-destructive/5` (pending-invitation row highlight; functional token resolves correctly in both themes via `--color-destructive`)
- Hand-rolled destructive button classes (`text-red-600`, `border-red-300`, `hover:bg-red-50`) → functional tokens (`text-destructive`, `border-destructive`, `hover:bg-destructive/10`)
- Drop redundant `default:` args from `t(".roles.member", default: "Member")` and `t(".role_descriptions.member", default: "Basic user access…")` — the locale keys exist in `config/locales/views/admin/users/en.yml`
**`app/views/imports/new.html.erb`**
- `icon_bg_class: "bg-gray-tint-5"` → `"bg-surface-inset"` (`gray-tint-5` isn't a defined utility; `bg-surface-inset` carries the same muted-background intent and theme-swaps correctly)
**`app/views/settings/profiles/show.html.erb`**
- Drop redundant `default:` args from `t(".group_title", default: "Group")`, `t(".group_form_label", default: "Group name")`, and `t(".group_form_input_placeholder", default: "Enter group name")` — all three keys exist in `config/locales/views/settings/en.yml`
**Deferred** to a separate PR (Rule 1 findings on admin/users):
- `<details>` block (lines 54–180) → `DS::Disclosure(:card)` — bigger refactor with custom summary content + Stimulus controller attributes; warrants its own diff.
- Destructive button shell → `DS::Button(:destructive)` — same reason; the class-token swap in this PR clears the immediate violation without changing the form-with structure or visual.
Refs #1951.
* fix(profiles): restore i18n default: args for group_* keys
@jjmata + @codex correctly flagged: `settings.profiles.show.group_title`,
`group_form_label`, and `group_form_input_placeholder` are defined in
en.yml + 4 other locales (de, es, pl, pt-BR), but missing from 8
locales (ca, fr, nb, nl, ro, tr, zh-CN, zh-TW).
With `config.i18n.fallbacks = true` those locales currently fall
back to en values, so end-users see English copy rather than a
translation-missing marker. The `default:` arg makes the fallback
explicit at the call site without depending on the Rails fallback
chain being configured a particular way — restores the original
defensive behavior from before #1955.
Admin/users role keys keep their `default:` removal — verified that
`roles.member` and `role_descriptions.member` exist in all 8
admin/users locales (`grep -c "^\s*member:"` returns 2 for every
locale file).
* fix(trades): prevent MissingTemplate for Turbo Stream requests on update/create failure
* Linter noise
---------
Co-authored-by: sentry[bot] <39604003+sentry[bot]@users.noreply.github.com>
Co-authored-by: Juan José Mata <jjmata@jjmata.com>
* feat(docker): add jemalloc to reduce memory fragmentation
Install libjemalloc2 in the base image and preload it via LD_PRELOAD in
docker-entrypoint when available. Reduces RSS growth from glibc's default
allocator fragmentation under Rails workloads.
* feat(docker): add DISABLE_JEMALLOC env var + preserve existing LD_PRELOAD
* feat(docker): add jemalloc status logging to entrypoint
* refactor(docker): simplify jemalloc logging to warn-only when disabled/missing
* chore(helm): bump pipelock to 2.5.0 and surface 2.5 config
Bumps pipelock.image.tag from 2.2.0 to 2.5.0 and exposes the most
relevant 2.5 features as structured Helm values:
- pipelock.requestBodyScanning: scan outbound bodies and sensitive
headers for prompt-injection and DLP payloads. Disabled by default;
roll out with action=warn before flipping to block.
- pipelock.healthWatchdog: structured config for the wedge-detection
watchdog with an exposeSubsystems toggle for /health detail.
- pipelock.mcpToolPolicy.rules: structured values for rendering
mcp_tool_policy.rules including redirect-profile references.
Also fixes a latent config-validation regression: pipelock 2.x rejects
an enabled mcp_tool_policy with no rules, but the chart previously
defaulted to enabled=true with an empty rules list, which hard-fails
'pipelock check'. The default is now enabled=false; operators must
explicitly enable and provide at least one rule.
Refreshes README, CHANGELOG, docs/hosting/pipelock.md, docs/hosting/ai.md,
compose example pin comment, and pipelock.example.yaml to call out 2.5
highlights (Audit Packet v0 verifiers, SPIFFE-strict envelopes, scanner
attribution on MCP block receipts, pipelock doctor). Also fixes a stale
docs/hosting/mcp.md reference to the removed compose.example.pipelock.yml.
* chore(helm): fail helm template when mcp_tool_policy enabled with no rules
Adds a guard in asserts.tpl so an operator who sets
pipelock.mcpToolPolicy.enabled=true without populating
pipelock.mcpToolPolicy.rules gets a clear render-time error instead
of a container crash-loop with the pipelock validation message.
Per CodeRabbit feedback on #1913.
* Versions
---------
Co-authored-by: Juan José Mata <jjmata@jjmata.com>
* fix(enable_banking): match bank list search against BIC, not just name
Bank-search filter on the Enable Banking bank-selection modal only indexed
`aspsp[:name]`, so users searching by BIC code (e.g. `INGDDEFF`) got no
results even when the bank was rendered in the list. Switch the per-item
data attribute to a `name + BIC` haystack and read from it in the Stimulus
controller, so either token matches.
Refs #1814
* style(bank_search): apply Biome formatting to forEach callback (#1874 review)
* fix: cascade destroy transfers and reset transaction kind on account destruction.
* Add rescue no method to transfer transaction reset
---------
Co-authored-by: arumaio <aruma.pro+git@protonmail.com>
Follow-up to #1917 — the responsive label-swap pair in
`_transfer_match.html.erb` was deferred because DS::Pill has no
caller-controlled `class:` arg yet. Wrapping each `DS::Pill` in a
`<span>` with the responsive visibility classes (`hidden lg:inline` /
`inline lg:hidden`) gets the same effect without expanding the
component API — the parent span's `display` controls visibility, the
child pill keeps its own `inline-flex` chrome when visible.
Closes the last open callsite from #1917's deferred-list. Same tone
(`:neutral`) and shape (`marker: false` rounded-full) as the other
neutral status badges migrated in PR B.
* Use date comparisons for interval thresholds
Replace hard-coded day counts in Period#interval with direct date comparisons (end_date > start_date + 5.years and + 1.year) for clearer intent and to avoid magic numbers; updated inline comments. No behavioral change intended aside from improved readability.
* Use advance(years:) for year-based comparisons
Replace start_date + N.years with start_date.advance(years: N) to apply calendar-year semantics (respecting leap years/month boundaries). Update comments to clarify 'calendar years' and the resulting interval choices (monthly for >5 years, weekly for >1 year). Intent is to make the period interval calculation more correct for calendar-aware date comparisons.
The admin users page wraps four top-level sibling sections inside a
single `bg-container rounded-xl shadow-border-xs p-4` card:
1. description paragraph
2. filter form
3. trials-expiring summary grid
4. families/groups list
5. role descriptions (`settings_section` collapsible → DS::Disclosure :card)
The first three carried their own `mb-6`; the families list and the
role descriptions section had no margin at all, so the families card
sat flush against the role-descriptions card with zero gap — clearly
broken next to the well-spaced upper sections.
Apply spacing at the **layout** level: hoist `space-y-6` onto the
outer container and drop the per-child `mb-6`. All five siblings now
get a consistent 24px gap.
No other admin or settings pages match this exact pattern (single
outer card + multiple sibling sections without parent space-y) — the
settings layout already wraps `<%= yield %>` in `space-y-4`, and other
pages with outer cards (`api_keys/show`, `llm_usages/show`, etc.)
either rely on that layout or carry their own internal `space-y-N`.
* refactor(views): migrate 6 residual inline alerts to DS::Alert
PR #1731 extended DS::Alert and migrated 9 inline alert blocks. Six
hand-rolled alert blocks slipped through that sweep and stayed on raw
palette tokens with no `theme-dark:` variants:
- `app/views/settings/llm_usages/show.html.erb` — "About Cost Estimates"
blue info block. Most visible offender: `bg-blue-50 border border-blue-200`
+ `text-blue-900 / text-blue-700 / text-blue-600` rendered as a bright
white-blue island in dark mode (the bug spotted on the LLM usage page).
- `app/views/accounts/confirm_unlink.html.erb` — yellow warning with
bullet list.
- `app/views/oidc_accounts/new_user.html.erb` — blue info heading.
- `app/views/oidc_accounts/link.html.erb` — two blocks (yellow verify
warning + blue create info). Also flips the file's pre-existing
`text-gray-600` hint paragraph to `text-secondary` (caught by the
`DeprecatedClasses` erb_lint rule on save).
- `app/views/rules/confirm.html.erb` — AI cost notice.
- `app/views/rules/confirm_all.html.erb` — AI cost notice.
All six migrate to `DS::Alert.new(title:, variant:)` (with a block content
slot for the rich/conditional bodies). DS::Alert resolves `bg-info/10`,
`border-info/20`, etc. from the `@theme` semantic tokens, so dark mode
now renders a subtle blue/yellow tint over the page surface instead of
a hardcoded light-mode pill.
Out of scope (left as-is, not alert-shaped):
- `app/views/assistant_messages/_tool_calls.html.erb` — a tool-call
display panel (not an alert; needs its own token sweep).
- `app/views/import/rows/_form.html.erb` — inline cell-error tooltip
(`bg-red-50 border border-red-200`) — also not alert-shaped; a future
PR can swap it to `bg-destructive/10 border-destructive-subtle` once
#1932 lands.
Surfaced while scanning DS drift for the LLM usage page bug. Tracking
issue: #1715 (closed but conceptually relevant) / #1911 (active drift
patrol).
* fix(oidc): keep alert description in <p>, retarget tests for DS::Alert title
CI on #1933 caught three test failures introduced by migrating the
two OIDC link alerts and the verify-redirect copy from hand-rolled
`<h3>` / `<p>` markup to `DS::Alert`:
1. `OidcAccountsControllerTest#test_should_show_create_account_option_for_new_user`
2. `OidcAccountsControllerTest#test_does_not_show_create_account_button_when_JIT_link-only_mode`
3. `SessionsControllerTest#test_redirects_to_account_linking_when_no_OIDC_identity_exists`
DS::Alert renders its `title:` slot as a `<p>` (semantically the alert
heading lives on the container's `aria-labelledby`, not on a heading
tag) and renders block / message content directly inside a `<div>`,
not a `<p>`. The pre-migration markup used `<h3>` for the heading and
`<p class="...text-blue-700">` for the description, so the tests
above asserted those specific tags.
Two fixes:
- `app/views/oidc_accounts/link.html.erb` — wrap the html_safe
description bodies in explicit `<p>` tags inside the DS::Alert
block. Restores the `<p>` element the session-redirect test asserts
on, and keeps the description as a semantic paragraph rather than
a bare text node inside the alert container.
- `test/controllers/oidc_accounts_controller_test.rb` — flip the two
`assert_select "h3", text: "Create New Account"` calls to match the
DS::Alert title `<p>`. The test was asserting an implementation
detail of the pre-migration markup; switching to the new tag keeps
the assertion meaningful (the heading text still has to render)
without re-introducing an `<h3>` outside of DS::Alert.
* fix(test): match Create New Account title with regex (sr-only "Info:" prefix)
DS::Alert prepends `<span class="sr-only">Info:</span>` inside the
title `<p>`, so the full text content is "Info: Create New Account",
not "Create New Account". `assert_select "p", text: "Create New Account"`
requires an exact text match and rejected the prefixed string. Switch
to a regex match — keeps the heading-text assertion meaningful without
coupling to the screen-reader prefix.
Two regressions from the recent token sweep, both producing low-contrast
results in dark mode.
## DS::Toggle off-track
PR #1843 (DS::Toggle a11y + token swaps) replaced the raw
`bg-gray-100 theme-dark:bg-gray-700` off-track with `bg-surface-inset`
for semantic alignment. `bg-surface-inset` resolves to gray-800 in
dark mode, but the toggle typically sits inside `bg-container`
(gray-900). The contrast ratio dropped from ~2.45:1 (gray-700 vs
gray-900) to ~1.5:1 (gray-800 vs gray-900) — visibly worse than the
pre-#1843 baseline and below WCAG 1.4.11 (3:1 for UI components).
Most visible inside the transaction-edit modal SETTINGS section
(`Exclude`, `One-time Expense`) where the off-state switches nearly
vanished into the modal chrome.
Introduce `--color-toggle-track` (light: gray-100, dark: gray-700) and
swap `bg-surface-inset` → `bg-toggle-track` in DS::Toggle. Restores the
pre-#1843 off-track contrast while keeping a semantic token (instead
of the raw palette references the migration was trying to remove).
## border-destructive subtle borders
PR #1849 (single-color tokens to @theme) flagged that
`border-destructive/N` rendered the wrong shade (the `@utility
border-destructive` block defined red-500 light, while
`--color-destructive` in `@theme` is red-600 — `/N` resolves from
@theme), and swapped a couple of callsites to solid `border-destructive`.
Solid renders red-500/red-400 at full saturation in both modes, which
reads as a loud error border on contexts that were meant to be subtle
(left-rule on the provider-sync "view error details" pane, error-message
box in SimpleFIN settings, alert-component border, provider connection
error rows).
Two callsites (`DS::Alert`, settings/providers/_connection_row) still
carried the broken `border-destructive/20` / `/25` modifier — same
off-shade footgun #1849 was meant to retire.
Introduce `--color-destructive-subtle` (light: red-200, dark: red-800)
and swap the four subtle-by-intent callsites to `border-destructive-subtle`:
- app/components/DS/alert.rb (destructive variant)
- app/views/settings/providers/_connection_row.html.erb (err status)
- app/components/provider_sync_summary.html.erb (error-details left rule)
- app/views/simplefin_items/edit.html.erb (error-message box)
The handful of intentionally-loud `border-destructive` callsites
(split-transaction over-allocation, blank-name account labels, etc.)
keep the solid token.
Regenerated `_generated.css` via `npm run tokens:build`.
PR #1840 bumped DS::Button icon-only `:md` size from `w-9 h-9` (36×36)
to `w-11 h-11` (44×44) for WCAG 2.5.5 enhanced touch target. DS::Menu's
`:icon` variant uses DS::Button at the default `:md` size, so every
row-level "..." action-list trigger grew from 36×36 to 44×44.
For dense lists where each row has a trigger — most visibly the
transaction category dropdown (`category/dropdowns/_row.html.erb`) —
the per-row height bump (+8px) compounds: a 5-category panel that
used to fit in ~220px now wants ~260px, the badges look smaller
relative to the row chrome, and the overall density that made the
dropdown scannable regresses visibly.
Add an `:icon_sm` variant that renders the trigger as DS::Button at
`size: :sm` (32×32). Meets WCAG 2.5.8 AA (24×24) — appropriate for
compact in-row triggers where 44×44 isn't required. Standalone
toolbar / row-action `...` triggers should keep `:icon` for AAA.
Migrate `category/dropdowns/_row.html.erb` to `:icon_sm` to restore
the pre-#1840 dropdown density.
PRs #1855, #1857, #1858 (DS::Disclosure :card/:card_inset/:inline
variants) introduced a `<div class="w-full">` wrapper around
`summary_content`. The wrapper is required for non-default variants —
their `<summary>` is `display: list-item` (no flex), so a caller's
inner flex+justify-between div would shrink-wrap to content width.
But for the `:default` variant, `<summary>` is already
`flex items-center justify-between`. Wrapping caller siblings in a
single `w-full` block collapses them into one flex child, killing the
justify-between distribution. This regressed the only default-variant
summary_content caller — `accounts/_accountable_group.html.erb` (the
homepage account sidebar) — where the group name and total/sparkline
divs no longer aligned across the row.
Render `summary_content` bare for `:default` (summary is the flex
container) and keep the `w-full` wrapper for `:card`, `:card_inset`,
`:inline`.
* refactor(transactions): migrate 5 transaction badges to DS::Pill (#1751 PR B)
Migrates the hand-rolled "Pending" / "Review recommended" / "Potential
duplicate" / "Split" badges across the transaction views to the
extended DS::Pill primitive from #1902.
**Visual contract for badge mode**
In #1902 the badge mode (`marker: false`) used `rounded-md` (chip shape)
because the marker mode does. But every existing pill / status badge
in the codebase uses `rounded-full` — see
`settings/providers/_status_pill.html.erb`,
`settings/providers/_maturity_badge.html.erb`, and the inline
transaction badges this PR is migrating. To keep the visual contract
consistent, this PR shifts `DS::Pill`'s badge mode to `rounded-full`
(marker mode stays `rounded-md`, unchanged from #1829). The shape
distinction now reads: markers are tags, badges are pills.
**Callsites migrated** (5):
- `app/views/transactions/_transaction.html.erb` — Pending,
Review-recommended, Possible-duplicate, Split badges
- `app/views/transactions/_header.html.erb` — Pending badge
- `app/views/transactions/_split_parent_row.html.erb` — Split badge
**Tone mapping**
| Badge | Tone | Notes |
|---|---|---|
| Pending | `:neutral` | unchanged copy/icon, gains subtle DS-controlled bg |
| Review recommended | `:neutral` | matches existing `bg-surface-inset` look |
| Possible duplicate | `:warning` | DS semantic alias for the existing `text-warning` |
| Split | `:neutral` | matches existing `bg-surface-inset` look |
**Deferred to follow-up PRs**
- `app/views/transactions/_transfer_match.html.erb` — uses two
responsive-visibility variants (`hidden lg:inline-flex` for long
copy, `inline-flex lg:hidden` for short). DS::Pill currently has no
`class:` arg for caller-controlled wrapper classes; deferring until
that lands.
- `app/views/transactions/searches/filters/_badge.html.erb` — has a
close button alongside the label (`button_to clear_filter_*`) and
uses `rounded-3xl p-1.5` instead of a true pill. Closer to a
removable filter chip — better fit for a separate `DS::FilterChip`
primitive than for `DS::Pill`.
Refs #1751.
* refactor(misc): migrate misc badges to DS::Pill (#1751 PR D)
Replaces five misc badge callsites with `DS::Pill` (badge mode:
`marker: false`, `show_dot: false`) so the long-tail badges share the
same shape, padding, and dark-mode tokens as the rest of the design
system. No raw palette classes remain in the migrated files.
Migrated:
- app/views/shared/_badge.html.erb — converted to a thin shim that
renders `DS::Pill`; preserves the block-content API and the
`pulse: true` option (wraps the pill in `animate-pulse`). Maps
`success`/`error`/`warning`/default → `:success`/`:error`/`:warning`/`:neutral`.
- app/views/accounts/_tax_treatment_badge.html.erb — maps tax
treatments to DS tones: `:tax_exempt → :green`,
`:tax_deferred → :indigo` (was raw blue-500/10),
`:tax_advantaged → :violet` (was raw purple-500/10), default → `:neutral`.
- app/views/reports/_investment_performance.html.erb (line ~121,
inline twin of the tax-treatment badge) — uses the same mapping via
a new `tax_treatment_pill_tone` helper.
- app/helpers/reports_helper.rb — replaces `tax_treatment_badge_classes`
with `tax_treatment_pill_tone` (the old helper had no other callers).
- app/views/import/qif_category_selections/show.html.erb (~line 86) —
inline split badge → `tone: :warning`.
- app/views/investment_activity/_badge.html.erb — fixed activity
enum mapped to DS tones: Buy/Reinvestment → :indigo,
Sell → :red, Dividend/Interest → :green, Contribution → :violet,
Withdrawal → :amber, others → :gray.
Skipped (true mismatches, not extendable without changing DS::Pill):
- app/views/shared/_color_badge.html.erb — takes an arbitrary
user-supplied color via `color-mix(in oklab, #{color} ...)`. DS::Pill
only supports the fixed tone enum, so this would lose information.
- app/views/categories/_badge.html.erb — same reason; renders
`category.color` (arbitrary hex per record).
- app/views/investment_activity/_quick_edit_badge.html.erb — interactive
button with a Stimulus controller, click action, hover state, and
dropdown anchor. DS::Pill renders a `<span>`; converting would
destroy the interactive surface.
Stack: based on `feat/ds-pill-transactions-1751-b` (PR #1917), which
ships the `marker: false` → `rounded-full` badge shape this PR depends on.
Refs #1751.
DS::Toggle already renders a paired hidden field for the off-state
value. Adding an external `hidden_field_tag` with the same `name` in a
caller view causes ID/label collisions (the auto-generated id matches
the checkbox id, so `<label for=...>` targets the hidden field) and
sends duplicate params.
Inline ERB comment so the warning surfaces wherever the component is
read or copied.
* refactor(transactions): migrate 5 transaction badges to DS::Pill (#1751 PR B)
Migrates the hand-rolled "Pending" / "Review recommended" / "Potential
duplicate" / "Split" badges across the transaction views to the
extended DS::Pill primitive from #1902.
**Visual contract for badge mode**
In #1902 the badge mode (`marker: false`) used `rounded-md` (chip shape)
because the marker mode does. But every existing pill / status badge
in the codebase uses `rounded-full` — see
`settings/providers/_status_pill.html.erb`,
`settings/providers/_maturity_badge.html.erb`, and the inline
transaction badges this PR is migrating. To keep the visual contract
consistent, this PR shifts `DS::Pill`'s badge mode to `rounded-full`
(marker mode stays `rounded-md`, unchanged from #1829). The shape
distinction now reads: markers are tags, badges are pills.
**Callsites migrated** (5):
- `app/views/transactions/_transaction.html.erb` — Pending,
Review-recommended, Possible-duplicate, Split badges
- `app/views/transactions/_header.html.erb` — Pending badge
- `app/views/transactions/_split_parent_row.html.erb` — Split badge
**Tone mapping**
| Badge | Tone | Notes |
|---|---|---|
| Pending | `:neutral` | unchanged copy/icon, gains subtle DS-controlled bg |
| Review recommended | `:neutral` | matches existing `bg-surface-inset` look |
| Possible duplicate | `:warning` | DS semantic alias for the existing `text-warning` |
| Split | `:neutral` | matches existing `bg-surface-inset` look |
**Deferred to follow-up PRs**
- `app/views/transactions/_transfer_match.html.erb` — uses two
responsive-visibility variants (`hidden lg:inline-flex` for long
copy, `inline-flex lg:hidden` for short). DS::Pill currently has no
`class:` arg for caller-controlled wrapper classes; deferring until
that lands.
- `app/views/transactions/searches/filters/_badge.html.erb` — has a
close button alongside the label (`button_to clear_filter_*`) and
uses `rounded-3xl p-1.5` instead of a true pill. Closer to a
removable filter chip — better fit for a separate `DS::FilterChip`
primitive than for `DS::Pill`.
Refs #1751.
* refactor(providers): migrate provider badges to DS::Pill (#1751 PR C)
Migrates the provider-bucket pill/badge callsites to the extended
DS::Pill primitive (badge mode, rounded-full) from #1917.
Callsites migrated (3):
- app/views/settings/providers/_status_pill.html.erb — provider
connection status pill. Status → tone mapping:
:ok → :success, :warn → :warning, :err → :error,
else → :neutral.
- app/views/settings/providers/_maturity_badge.html.erb — alpha/beta
label. Tone :neutral, no dot.
- app/views/sophtron_items/_sophtron_item.html.erb (line 27) —
"manual sync" warning. Tone :warning, no dot.
The settings/providers/_status_pill partial wraps DS::Pill rather
than being deleted, since _connection_row still calls it via
`render "settings/providers/status_pill", status: status` — keeping
the partial preserves the seam without a wider refactor.
Dead code removed: SettingsHelper#status_pill_classes (no remaining
callers after the migration).
Skipped:
- app/views/simplefin_items/_activity_badge.html.erb — not actually
a pill/badge. It renders <p> text with `text-warning` plus an
inline icon below the heading; no rounded-full shape and no chip
semantics. Migrating it would change the layout, not consolidate
a pill pattern.
Refs #1751. Stacks on #1917.
Migrates the hand-rolled "Pending" / "Review recommended" / "Potential
duplicate" / "Split" badges across the transaction views to the
extended DS::Pill primitive from #1902.
**Visual contract for badge mode**
In #1902 the badge mode (`marker: false`) used `rounded-md` (chip shape)
because the marker mode does. But every existing pill / status badge
in the codebase uses `rounded-full` — see
`settings/providers/_status_pill.html.erb`,
`settings/providers/_maturity_badge.html.erb`, and the inline
transaction badges this PR is migrating. To keep the visual contract
consistent, this PR shifts `DS::Pill`'s badge mode to `rounded-full`
(marker mode stays `rounded-md`, unchanged from #1829). The shape
distinction now reads: markers are tags, badges are pills.
**Callsites migrated** (5):
- `app/views/transactions/_transaction.html.erb` — Pending,
Review-recommended, Possible-duplicate, Split badges
- `app/views/transactions/_header.html.erb` — Pending badge
- `app/views/transactions/_split_parent_row.html.erb` — Split badge
**Tone mapping**
| Badge | Tone | Notes |
|---|---|---|
| Pending | `:neutral` | unchanged copy/icon, gains subtle DS-controlled bg |
| Review recommended | `:neutral` | matches existing `bg-surface-inset` look |
| Possible duplicate | `:warning` | DS semantic alias for the existing `text-warning` |
| Split | `:neutral` | matches existing `bg-surface-inset` look |
**Deferred to follow-up PRs**
- `app/views/transactions/_transfer_match.html.erb` — uses two
responsive-visibility variants (`hidden lg:inline-flex` for long
copy, `inline-flex lg:hidden` for short). DS::Pill currently has no
`class:` arg for caller-controlled wrapper classes; deferring until
that lands.
- `app/views/transactions/searches/filters/_badge.html.erb` — has a
close button alongside the label (`button_to clear_filter_*`) and
uses `rounded-3xl p-1.5` instead of a true pill. Closer to a
removable filter chip — better fit for a separate `DS::FilterChip`
primitive than for `DS::Pill`.
Refs #1751.
#1858's :inline variant landed (commit 8de14ed2), unblocking the third
sure-design drift finding on this file (#1895 / #1898).
The :inline variant is the right shape for an in-table-cell metadata
expander — no surface, no padding, no shadow; the summary reads as plain
text-link copy. The bot recommended this exact variant when filing the
issues; previous PR (#1903) covered the two token findings but deferred
the <details> migration until the variant was available.
Closes#1895. Closes#1898.
`app/views/settings/debugs/show.html.erb` had two non-functional Tailwind
classes flagged by sure-design's weekly merged-commit scan (#1895, #1898):
- `bg-surface-default` → `bg-surface`. `bg-surface-default` doesn't map
to any DS color variable (`--color-surface-default` isn't defined);
`--color-surface` is the canonical token, auto-generates `bg-surface`.
- `divide-gray-100` → `divide-alpha-black-200 theme-dark:divide-alpha-white-200`.
Matches the existing pattern used by `admin/sso_providers/index.html.erb`,
`admin/users/index.html.erb`, and `settings/preferences/show.html.erb`
for tbody dividers. No `divide-primary` utility exists yet, so the
bot's suggestion gets the same effect via the alpha tokens.
The third drift finding on this file — the in-cell `<details>`
metadata expander — is deferred until #1858's `DS::Disclosure :inline`
variant lands on `main`. The `:default` variant renders a
`bg-surface px-3 py-2 rounded-xl` card chrome that's wrong for an
in-table-cell trigger; the `:inline` variant in #1858 is the right
shape and will get a follow-up PR once that lands.
Closes#1895 partially. Closes#1898 partially. Both bot issues stay
open until the `<details>` migration also lands.
* feat(design-system): extend DS::Pill with badge mode + semantic tones (#1751)
Adds two extensions to the existing `DS::Pill` (originally landed as a
stage marker primitive in #1829) so it can also serve as the shared
status / category badge across the app — the use case tracked by #1751.
**Badge mode (`marker: false`)**
The original `DS::Pill` was intentionally sub-12px (text-[10px] /
text-[11px]) + uppercase + tracking-wide so it reads as a marker
(`Beta`, `Canary`, `NEW`), not a label. That shape is wrong for
status badges where the surrounding context is regular UI copy and the
pill needs to feel like a chip (`Pending`, `Active`, `Past due`,
`Failed`).
The new `marker: false` flag drops the uppercase + arbitrary
sub-12px text and snaps the chrome to the DS text scale:
- `marker: false, size: :sm` → `text-xs` (12px), normal case
- `marker: false, size: :md` → `text-sm` (14px), normal case
- `marker: true` (default) → existing #1829 behavior, unchanged
**Semantic tone aliases**
Status badges read more naturally with semantic tone names than with
the underlying palette colors:
| Alias | Resolves to |
|---|---|
| `:success` | `:green` |
| `:warning` | `:amber` |
| `:error` / `:destructive` | `:red` (new tone, added here) |
| `:info` | `:indigo` |
| `:neutral` | `:gray` |
Visual-name tones (`:violet`, `:indigo`, `:fuchsia`, `:amber`,
`:green`, `:gray`, `:red`) still work as before — semantic aliases
resolve through `SEMANTIC_TONE_ALIASES` at component init time, so
the callsite can pick whichever name reads better. Unknown tones
still fall back to `:violet` (existing behavior).
**Red palette**
Adds the `:red` tone (palette already present in
`design/tokens/sure.tokens.json` — `red-50/100/200/500/700/tint-10`).
Needed for `:error` / `:destructive` status badges.
**Icon slot**
Adds an `icon:` option (already documented in the component's
doc-comment as planned). When set, the Lucide glyph replaces the
colored dot inside the pill — useful for status badges that read
better with a glyph (`circle-check`, `triangle-alert`,
`loader`, etc.) than the generic dot.
**Scope**
API + tests + Lookbook preview only. No callsite migrations in this
PR — that's the next slice of #1751, done as separate per-bucket PRs
(transaction badges, provider badges, misc) to keep diffs small.
DS::Pill currently has no in-app callsites (#1829 shipped the
primitive ahead of consumers), so this is a pure-additive change.
Existing API is fully backwards-compatible — `marker:` defaults to
`true`, so without that flag the pill renders exactly as it does
today.
* fix(test): use assert_no_selector for dot-suppression assertion
`refute_selector ..., count: 1` only fails when there are exactly 1
matches — it would silently pass for 0 OR 2+. The intent is "no dots
should render when an icon is set"; `assert_no_selector` strictly
asserts zero matches.
Flagged by coderabbit on #1902.
* feat(design-system): add :inline variant + migrate indexa_capital + snaptrade panels
Adds an `:inline` variant to `DS::Disclosure` for plain text-link-style
toggles that have no surface, no padding, no shadow — the disclosure
reads as a clickable summary text + revealed content, nothing more.
Use case: "Alternative auth" form section toggle in the Indexa Capital
provider panel; "Manage connections" lazy-loaded toggle in the
Snaptrade provider panel. Both were the last raw-`<details>` callsites
in `app/views/settings/providers/`.
Migrations:
- `_indexa_capital_panel.html.erb` — single inline `<details>` revealing
username / document / password form fields under an "Alternative auth"
summary text.
- `_snaptrade_panel.html.erb` — lazy-load `<details>` with
`data-controller="lazy-load"` etc. The new `tag.details ... **opts`
forwarding from #1857 lets the Stimulus controller attrs flow
through cleanly via DS::Disclosure's `data:` keyword.
Chevron rotation on snaptrade gets the standard
`motion-safe:transition-transform motion-safe:duration-150` treatment
(was `transition-transform` without the motion-safe gate).
Variant summary now:
| Variant | Details surface | Use case |
|---|---|---|
| `:default` | none / bg-surface summary | inline expander inside parent card |
| `:card` | `bg-container shadow-border-xs rounded-xl p-4` | provider rows, settings sections |
| `:card_inset` | `bg-surface-inset rounded-xl p-4` | inset sub-panels |
| `:inline` | no surface | text-link-style toggles |
* fix(review): guard variant.to_sym against nil in DS::Disclosure
CodeRabbit on #1858 flagged that `variant: nil` crashed with
`NoMethodError` at `variant.to_sym` before the explicit `VARIANTS`
check could run. Use safe navigation (`variant&.to_sym`) so nil
falls through to the validation, and inspect `@variant` in the
error message so nil / non-symbol inputs render readably.
Verified manually via runner: `DS::Disclosure.new(variant: nil)` now
raises `ArgumentError: Invalid variant: nil. Must be one of
[:default, :card, :card_inset, :inline]`.
* feat(design-system): add :card_inset variant + migrate ibkr_panel and settings/_section
Wraps up the disclosure migration cluster from #1715 §6:
1. **New `:card_inset` variant** on `DS::Disclosure`. Same contract
as `:card` but uses `bg-surface-inset rounded-xl p-4` (no shadow)
for inset sub-panels embedded inside a parent card surface.
2. **Migrate `_ibkr_panel.html.erb`** — the "flex query details"
disclosure (`<details class="group bg-surface-inset rounded-xl p-4">`)
was the one panel skipped from #1856 because it used the inset
surface. Now uses `DS::Disclosure(variant: :card_inset)`. Chevron
gets the `motion-safe:transition-transform motion-safe:duration-150`
treatment along the way.
3. **Migrate `settings/_section.html.erb`** — the global "collapsible
settings card" primitive backing 19 callsites via the
`settings_section(...)` helper. The collapsible branch's
`<details class="group bg-container shadow-border-xs rounded-xl p-4">`
becomes `DS::Disclosure(variant: :card, open: open, data: ...)`.
While here:
- Update `disclosure.html.erb` to spread `**opts` onto the `<details>`
element via `tag.details`. Previously opts were captured but never
applied; the `settings/_section` migration needs `data-controller`
+ `data-auto-open-param-value` to flow through to the rendered
`<details>`.
- Non-collapsible branch in `settings/_section.html.erb` stays as
raw `<section>` — different semantics (not expandable), DS::Disclosure
can't replace because it always renders `<details>`.
API:
DS::Disclosure.new(
variant: :card | :card_inset | :default,
open: bool,
data: { controller: "...", ... } # forwarded to <details>
)
* fix(review): merge caller class in DS::Disclosure + i18n plaid deletion
- DS::Disclosure: extract caller class: from opts and merge via class_names
before forwarding to tag.details. Prevents the latent duplicate keyword
arg error when callers pass class: alongside the variant-derived classes.
- plaid_items/_plaid_item: localize "(deletion in progress...)" via
t('.deletion_in_progress') + add en locale key, matching lunchflow /
mercury / sophtron / coinstats convention.
* fix(panels): replace text-white and bg-gray-tint-10 with semantic tokens
`text-white` → `text-inverse` on the EnableBanking reauthorize button
(`bg-warning` background); `bg-gray-tint-10` → `bg-container-inset` on
the IndexaCapital item avatar wrapper. Both flagged by sure-design as
non-functional palette tokens.
Pre-existing on main; surfaced by the re-indentation that this PR
applied during the disclosure migration.
* feat(design-system): DS::Disclosure :card variant + migrate 14 provider items
Resolves part of #1715 §6. The provider-item view templates
(binance, brex, coinbase, coinstats, enable_banking, ibkr,
indexa_capital, kraken, lunchflow, mercury, plaid, simplefin,
snaptrade, sophtron — 14 in total) all hand-rolled the same
`<details open class="group bg-container p-4 shadow-border-xs
rounded-xl">` shell with a custom summary inside and content below.
Extend `DS::Disclosure` with a `:card` variant that bakes the card
chrome onto the `<details>` element itself; the summary becomes
slot-driven via the existing `summary_content` slot. Provider items
keep their custom summary content (logos, brand colors, status copy)
unchanged — they just hand it to the slot instead of writing it
between `<summary>` tags.
API:
DS::Disclosure.new(variant: :card, open: true) do |d|
d.with_summary_content do
<div class="flex items-center gap-2">
chevron + custom summary markup
</div>
end
body content
end
While here:
- Drop the no-op `group-open:transform` from the default chevron
(Tailwind v4 applies `rotate-90` directly).
- Add `motion-safe:transition-transform motion-safe:duration-150`
to chevron rotation for reduced-motion respect (matches the
pattern landing in #1841).
- Extract `summary_classes` / `details_classes` helpers so the
default and card surfaces stay readable side-by-side.
Note: this PR touches `DS::Disclosure` and will textually conflict
with #1841 (focus-ring + reduced-motion polish). Both changes are
compatible — when #1841 merges first, the resolution is just
preserving both edits (the focus-ring classes are already merged
into `summary_classes` here).
* feat(design-system): migrate 3 provider panels to DS::Disclosure :card variant
Resolves the panel slice of #1715 §6. Continuation of the
DS::Disclosure :card variant work — same migration pattern, applied
to the 3 provider-PANEL templates that share the card shape with the
provider-item templates landing on the parent branch.
Migrated `<details class="group bg-container p-4 shadow-border-xs
rounded-xl">` → `DS::Disclosure.new(variant: :card)` in:
- `app/views/settings/providers/_kraken_panel.html.erb` — 1 details
in the items-each loop.
- `app/views/settings/providers/_mercury_panel.html.erb` — 1 details
in the items-each loop.
- `app/views/settings/providers/_brex_panel.html.erb` — 2 details:
one in the items-each loop, one standalone "add connection" panel
that opened by default when no active items existed. The
conditional `<%= "open" unless active_items.any? %>` becomes
`open: active_items.none?` on the `:card` disclosure.
Panels do NOT show a chevron in their summary (different UX from
the per-item rows in #1855), so the migration preserves that — no
chevron inserted.
NOT migrated (intentionally — different shapes):
- `_ibkr_panel.html.erb` — `<details class="group bg-surface-inset
rounded-xl p-4">`. Uses bg-surface-inset, not bg-container — needs
a `:card-inset` variant we haven't built. Deferred.
- `_indexa_capital_panel.html.erb` — `<details class="group">` with
no card chrome. Inline expander; doesn't fit either disclosure
variant.
- `_snaptrade_panel.html.erb` — same inline pattern as indexa_capital.
* fix(review): use ring-alpha-black-300 focus token in DS::Disclosure
CodeRabbit P2: switch the focus-visible outline from raw
gray-900/white palette values to the alpha-black-300 ring token,
matching the established focus pattern on settings/provider_card.html.erb.
This keeps theme behavior centralized in the design system tokens
instead of branching on theme-dark: in the component.
Applies to both :default and :card summary variants.
* fix(review): stretch DS::Disclosure summary_content to full width
Codex P2 follow-up on the disclosure-migration stack: \`<summary>\` is
\`display: list-item\`, so a flex inner div inside the slot
shrink-wraps to content width — any \`justify-between\` the caller
adds has nothing to distribute, and the right-side admin actions
collapse toward the title across every provider-item partial migrated
to \`DS::Disclosure variant: :card\` in #1855 (and the panels in
#1856 / #1857 / #1858 that inherit this component).
Wrap the slot in \`<div class=\"w-full\">\` so caller-supplied flex
rows stretch across the card. \`:default\` variant is unchanged
(it never uses \`summary_content\`).
* fix(review): stretch :card summary flex row to full width
Codex P2 follow-up on #1856: the migrated kraken / mercury / brex
panel summary rows wrap their content in
\`<div class=\"flex items-center justify-between gap-X\">\`, but a
flex container inside \`<summary>\` (\`display: list-item\`)
shrink-wraps to content size, so \`justify-between\` had nothing to
distribute and the right-side admin actions collapsed toward the
title.
Add \`w-full\` so the flex row stretches across the card. The deeper
component-level fix lands in #1855 (wraps \`summary_content\` in a
\`w-full\` block); this commit makes #1856 self-contained against the
merge order.
* feat(design-system): DS::Disclosure :card variant + migrate 14 provider items
Resolves part of #1715 §6. The provider-item view templates
(binance, brex, coinbase, coinstats, enable_banking, ibkr,
indexa_capital, kraken, lunchflow, mercury, plaid, simplefin,
snaptrade, sophtron — 14 in total) all hand-rolled the same
`<details open class="group bg-container p-4 shadow-border-xs
rounded-xl">` shell with a custom summary inside and content below.
Extend `DS::Disclosure` with a `:card` variant that bakes the card
chrome onto the `<details>` element itself; the summary becomes
slot-driven via the existing `summary_content` slot. Provider items
keep their custom summary content (logos, brand colors, status copy)
unchanged — they just hand it to the slot instead of writing it
between `<summary>` tags.
API:
DS::Disclosure.new(variant: :card, open: true) do |d|
d.with_summary_content do
<div class="flex items-center gap-2">
chevron + custom summary markup
</div>
end
body content
end
While here:
- Drop the no-op `group-open:transform` from the default chevron
(Tailwind v4 applies `rotate-90` directly).
- Add `motion-safe:transition-transform motion-safe:duration-150`
to chevron rotation for reduced-motion respect (matches the
pattern landing in #1841).
- Extract `summary_classes` / `details_classes` helpers so the
default and card surfaces stay readable side-by-side.
Note: this PR touches `DS::Disclosure` and will textually conflict
with #1841 (focus-ring + reduced-motion polish). Both changes are
compatible — when #1841 merges first, the resolution is just
preserving both edits (the focus-ring classes are already merged
into `summary_classes` here).
* fix(review): use ring-alpha-black-300 focus token in DS::Disclosure
CodeRabbit P2: switch the focus-visible outline from raw
gray-900/white palette values to the alpha-black-300 ring token,
matching the established focus pattern on settings/provider_card.html.erb.
This keeps theme behavior centralized in the design system tokens
instead of branching on theme-dark: in the component.
Applies to both :default and :card summary variants.
* fix(review): stretch DS::Disclosure summary_content to full width
Codex P2 follow-up on the disclosure-migration stack: \`<summary>\` is
\`display: list-item\`, so a flex inner div inside the slot
shrink-wraps to content width — any \`justify-between\` the caller
adds has nothing to distribute, and the right-side admin actions
collapse toward the title across every provider-item partial migrated
to \`DS::Disclosure variant: :card\` in #1855 (and the panels in
#1856 / #1857 / #1858 that inherit this component).
Wrap the slot in \`<div class=\"w-full\">\` so caller-supplied flex
rows stretch across the card. \`:default\` variant is unchanged
(it never uses \`summary_content\`).
* feat(design-system): add DS::SearchInput + migrate 2 broken-focus callsites
Resolves#1715 §3.
Two standalone search-field callsites — `/settings/preferences`
currency filter and `/settings/providers` filter row — had a hand-
rolled markup that ended in `focus:ring-gray-500`. That utility has
no backing token in the design system (`ring-gray-500` isn't in
Tailwind's default + Sure doesn't register a gray ring color), so
the input rendered with zero focus indicator on a bordered
bg-container surface. Keyboard users couldn't tell when the field
was focused.
Introduce `DS::SearchInput` — icon-on-left, bordered, token-backed
focus ring matching the DS::Button pattern landing in #1840
(`outline-2 outline-offset-2 outline-gray-900` with the dark-mode
override). API:
DS::SearchInput.new(
name: "...",
placeholder: "...",
value: ...,
aria_label: "...", # defaults to placeholder
class: "...", # passed to the wrapper
**opts # spread onto the <input>, e.g. data-*
)
Migrate the two broken callsites. Three other "search" patterns
stay as-is (out of scope for this PR):
- `form.search_field :search` inside `styled_form_with` blocks
(accounts/show/_activity.html.erb, UI::Account::ActivityFeed) —
already routes through StyledFormBuilder's form-field CSS.
- Embedded-dropdown search input inside DS::Select, DS::Menu, and
the splits/category-select panels — uses a different shape
(no border, no ring) because the parent panel provides the chrome.
- Category dropdown's combobox search input
(app/views/category/dropdowns/show.html.erb) — has a custom
`role=combobox` flow and stays intentionally distinct.
* feat(design-system): add embedded variant to DS::SearchInput, migrate 2 more callsites
Adds `variant: :embedded` to `DS::SearchInput` for search inputs that
live *inside* another DS panel (DS::Select dropdown, splits category
filter, future DS::Popover-hosted filters). No own border / no own
focus ring — the parent panel provides the chrome, so adding ring
+ outline competes with its `focus-within` state.
API:
DS::SearchInput.new(variant: :embedded, placeholder: "...", data: {...})
The `:standalone` default (from the previous commit) stays unchanged
and remains the right choice for top-of-list filter inputs.
Migrated:
- `app/components/DS/select.html.erb` — the in-dropdown search input
for `DS::Select.new(searchable: true)`. Was the only remaining
internal raw <input type="search"> markup in the component.
- `app/views/splits/_category_select.html.erb` — split-transaction
category picker filter. Same shape as DS::Select's search but
hand-rolled because the picker isn't a vanilla DS::Select.
Three other search patterns stay out of scope (intentionally, per
the previous commit):
- `form.search_field :search` inside `styled_form_with` — uses
form-field CSS, different visual contract.
- `app/views/category/dropdowns/show.html.erb` — bespoke
`role="combobox"` flow with `aria-expanded` / `aria-autocomplete`
semantics that don't belong in this primitive.
* fix(review): mobile font + embedded variant focus-within ring
- DS::SearchInput: switch text-sm -> text-base sm:text-sm on both
variants so the input keeps its 16px base size on mobile. iOS
Safari zooms the viewport when a focused input is below 16px,
which the unconditional text-sm was triggering on the Settings
Preferences currency search and Settings Bank Sync provider
search.
- DS::Select (searchable variant) + splits/_category_select:
add focus-within:ring-4 focus-within:ring-alpha-black-200
(with theme-dark variant) on the wrapper around the embedded
search input. The embedded variant intentionally has no own
focus ring so it inherits chrome from its parent panel — but
the two current parent panels were not providing one, so
keyboard focus on the dropdown search box rendered with no
visible indicator. Ring matches the .form-field token used
across the design system.
* fix(merge): repair DS::Select search input merge resolution
The previous merge of main left invalid Ruby inside the DS::SearchInput
`data:` hash:
aria-label="<%= t("helpers.select.search_placeholder") %>"
This is an ERB string assignment masquerading as a hash entry — it does
not parse and would have raised SyntaxError at render. Two follow-ups:
- Drop the `aria-label` entry entirely. `DS::SearchInput` already
defaults `aria_label` to `placeholder`, and `placeholder` is set
on the call, so the resulting <input> already carries
`aria-label="<%= t(...) %>"`.
- Restore the `input->select#syncTabindex` action that main #1848
added on the embedded search input. It keeps the roving tabindex
on the listbox in sync as filtered results change. Original PR
branch had only `list-filter#filter`; reintegrate both with
explicit `input->` event prefixes for parity with main.
---------
Signed-off-by: Juan José Mata <juanjo.mata@gmail.com>
Co-authored-by: Juan José Mata <juanjo.mata@gmail.com>
