Skip to content

fix(frontend): one card and dialog surface, kpi-tile on Opportunities - #1380

Merged
IAmJSD merged 8 commits into
LeanerCloud:mainfrom
IAmJSD:ui/issue-743
Oct 24, 2026
Merged

IAmJSD merged 8 commits into
LeanerCloud:mainfrom
IAmJSD:ui/issue-743

Conversation

@IAmJSD

@IAmJSD IAmJSD commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator

The Opportunities summary cards were hand-built .card innerHTML while Home used the .kpi-tile component, so the same kind of KPI looked different on the two pages. Card styles disagreed on border and shadow, and each of the four dialog systems (.modal, .modal-overlay, confirmDialog, the Archera offer) had its own scrim, radius, shadow and z-index literal. This PR reuses the kpi-tile on Opportunities and puts every card and dialog panel on one shared surface through tokens.

Refs #743
Partial: the shared .dialog component with a header row and close button, and merging the four JS modal systems into one implementation, are not done (they need edits to about 20 dialogs and their close wiring).

What changed

  • 9b7ca8e4: new frontend/src/lib/kpi-tile.ts (buildKpiTile/renderKpiTiles, textContent only). dashboard.ts renderDashboardSummary uses it, replacing a 23-line hand-built loop with identical markup (sparkline slot included). recommendations.ts renderRecommendationsSummary uses it instead of innerHTML (data-kpi count / savings / upfront / payback). The card/value/savings classes are kept, so existing selectors and tests still work.
  • 03fe2ef6 (CSS):
    • styles/base.css adds --cudly-scrim, --cudly-shadow-dialog and a named z-index ladder (--cudly-z-modal 1000, -modal-raised 1050, -page-overlay 1100, -confirm 2000, -toast 2100). The values are unchanged, so the stacking order is the same and a confirm still opens above any modal.
    • .card is now 1px --cudly-border + --cudly-shadow-sm + --cudly-r-md + --cudly-surface. .kpi-tile inherits that instead of redefining it, and .plan-card, .group-card and .upcoming-card match it (.upcoming-card keeps its 4px blue left accent). .stat-card stays a borderless muted inset tile.
    • .modal, .modal-overlay, .modal-confirm-backdrop and .archera-offer-backdrop use the scrim; .modal-content, .modal-confirm and .archera-offer-panel use --cudly-r-lg (12px, matching the existing mobile bottom-sheet corners) and the dialog shadow. All dialog and toast z-indexes use the tokens.
  • e1fa861e: tests-e2e/surfaces.spec.ts. Plus new jest files kpi-tile.test.ts, recommendations-kpi-tiles.test.ts and surface-css.test.ts.

Why this approach

This is the subset that fixes what users see without rewriting dialog markup: one tile component, one card surface and one dialog surface, all through tokens. The z-index values keep their order on a named ladder rather than being collapsed, since a confirm must still open above a modal. The dialog shadow is a new --cudly-shadow-dialog rather than the suggested --cudly-shadow-lg, because shadow-lg (0.08 alpha) disappears over a 50% scrim. The scrim is a slate tint (15, 23, 42) matching the existing shadow tokens instead of pure black. .stat-card keeps its borderless fill because it sits inside a card, and a border there would draw a box inside a box. The tile builder lives in lib/ so recommendations.ts (5,700 lines) and dashboard.ts share one implementation.

Screenshots

Before is main (a14dc8f2), after is this branch. Both are headless Chromium at 1440x900 against the repo's mocked e2e API.

Before After
summary-tiles-home-vs-opps before summary-tiles-home-vs-opps after
new-plan-modal before new-plan-modal after
delete-confirm-dialog before delete-confirm-dialog after
  • Home tiles (top) vs Opportunities cards (bottom): different title weight, value size, padding and border on main. After, Opportunities uses the same kpi-tile and the two rows match.
  • New Plan modal: 8px corners, flat panel, black 0.5 scrim on main. After, 12px corners, dialog shadow and slate scrim, the same as the confirm dialog (subtle; look at the corners).
  • Delete-plan confirm: 8px corners, its own heavier shadow, black 0.45 scrim, and the plan card behind has no border. After, the same radius, shadow and scrim as the New Plan modal, and the plan cards behind have the shared hairline border.

Verification

Run on this branch's head e1fa861e on top of main a14dc8f2:

  • Lint: 0 errors (142 warnings, all in existing files). Typecheck and build pass.
  • Jest: 112 suites, 3519 passed, 1 skipped. New tests pass with --randomize (33). New: kpi-tile.test.ts (5: full tile, optional parts, empty detail, text-not-markup, render order), recommendations-kpi-tiles.test.ts (2: the Opportunities summary is four .kpi-tile cards with the expected titles and data-kpi; only savings is green), surface-css.test.ts (26: tokens defined, z-ladder order, every scrim/dialog panel/z-index/card surface uses the shared tokens).
  • Playwright: 159 passed, including the new surfaces.spec.ts (2: computed styles of the first Home and Opportunities tiles match on border, radius, shadow, padding, background and title/value type; the New Plan modal and the Delete-plan confirm share scrim, 12px radius and shadow).
  • git-secrets scan clean (planted key flagged); scripts/test-git-secrets-allowlist.sh: 137 pass.

Notes for review

  • Biggest visible change: every .card in the app now has a 1px hairline border and a lighter shadow (the old one was 0 2px 4px at 0.1 black). Worth looking over the Admin, Plans and Purchases cards.
  • Opportunities tiles now use Home's muted 14px medium titles instead of bold dark titles, and have no detail line, so they are slightly shorter than Home's.
  • Dialog corners go from 8px to 12px, and the scrim is a slate tint at 0.5 (confirm and Archera used 0.45 black).
  • --cudly-shadow-dialog is a new token; the issue suggested reusing shadow-lg.
  • Left out: (a) one .dialog component with a shared header row and close button. Only confirmDialog has an X today; adding one to every .modal means editing about 20 dialogs in index.html and TS, plus close wiring. (b) Merging the four JS modal systems (modal.ts openModal, confirmDialog.ts, the Archera offer, the auth/plans .modal-overlay variants) into one implementation.

Overlaps

Those PRs will be opened alongside this one and merged in sequence, with conflicts resolved on the later branch.

Note for the UI owner

This changes UI code that @cristim owns. IAmJSD reviewed the before/after screenshots and asked for it to be merged; any design point you disagree with can be a follow-up.

🤖 Generated with Claude Code

IAmJSD and others added 3 commits October 24, 2026 01:34
Home built its four summary cards as .card.kpi-tile with DOM calls, while
Opportunities wrote plain .card innerHTML, so the same kind of KPI had a
different title weight, value size, padding and border on the two pages.

New lib/kpi-tile.ts (buildKpiTile / renderKpiTiles) builds the tile markup
once with textContent. Home's renderDashboardSummary now uses it (same
markup as before, sparkline slot included) and so does the Opportunities
summary (data-kpi count / savings / upfront / payback, no detail line or
sparkline). The card/value/savings classes stay, so existing selectors and
tests keep working.

Refs LeanerCloud#743

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Cards: .card was a borderless 0 2px 4px shadow on white, .kpi-tile and
.group-card used a hairline border, .plan-card and .upcoming-card a token
shadow with no border. .card is now the single surface (1px
--cudly-border, --cudly-shadow-sm, --cudly-r-md, --cudly-surface). .kpi-tile
inherits it instead of redefining it, and .plan-card, .group-card and
.upcoming-card match it. .stat-card stays a borderless muted inset tile
because it sits inside a card.

Dialogs: .modal, .modal-overlay, confirmDialog and the Archera offer each
had their own scrim (0.5 vs 0.45 black), radius (8px) and shadow, plus
loose z-index literals. New base.css tokens: --cudly-scrim,
--cudly-shadow-dialog and a named z-index ladder (modal 1000, raised 1050,
page overlay 1100, confirm 2000, toast 2100). The values are unchanged, so
the stacking order stays the same and a confirm still opens above any
modal. Every dialog panel now uses --cudly-r-lg (12px, matching the
existing mobile bottom-sheet corners) and the dialog shadow.

Refs LeanerCloud#743

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…face

Compares computed styles in Chromium. The first Home and Opportunities
summary tiles must match on border, radius, shadow, padding, background and
title/value type. The New Plan .modal and the Delete-plan confirmDialog must
match on scrim color, radius (12px) and shadow.

Refs LeanerCloud#743

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@IAmJSD IAmJSD added priority/p2 Backlog-worthy severity/medium Moderate harm type/chore Maintenance / non-user-visible triaged Item has been triaged urgency/this-quarter Within the quarter impact/many Affects most users effort/m Days labels Oct 24, 2026
@coderabbitai

coderabbitai Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Caution

Review failed

The pull request is closed.

Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

IAmJSD and others added 3 commits October 24, 2026 16:50
Conflicts:
- frontend/src/dashboard.ts: import block. Kept this branch's
  buildKpiTile import and main's formatTrendTooltipTitle (LeanerCloud#1323) on the
  combined chart-utils import.
- frontend/src/recommendations.ts: import block. Kept this branch's
  renderKpiTiles import and main's captureFocus/restoreFocus import.

The shared-surface CSS (base, components, modals, plans, settings)
merged without conflicts; main's CSS changes in those files (motion
tokens, contrast, spacing, code-example block) touch other properties
and keep the branch's scrim, dialog shadow and z-index tokens intact.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
No textual conflicts. Checked main's changes next to this branch's:
LeanerCloud#1344 moved the YTD sparkline into loadYtdSparkline(), which looks up
.kpi-tile-spark[data-spark-key="ytd"]; lib/kpi-tile.ts still renders
that slot for the YTD tile. CSS added by LeanerCloud#1336/LeanerCloud#1332/LeanerCloud#1388 (sticky
section nav z-index 5, table scroll shadows z-index 9, sort buttons)
sits below the dialog z-index ladder and does not touch card or dialog
surfaces.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Conflicts:
- frontend/src/dashboard.ts (imports): kept this branch's lib/kpi-tile
  import and took main's widened chart-utils import (formatSparseTrendMessage,
  MIN_TREND_POINTS from LeanerCloud#1334).
- frontend/src/dashboard.ts (renderDashboardSummary): this branch replaced
  the hand-built tile loop with buildKpiTile; main (LeanerCloud#1376, LeanerCloud#739) added a
  guard to that loop so an absent NO_VALUE figure is not styled as a
  saving. Kept buildKpiTile and moved main's guard into it
  (lib/kpi-tile.ts: valueSavings && value !== NO_VALUE), so Home keeps
  main's behaviour. Because Opportunities uses the same builder, its
  savings tile showing NO_VALUE is now neutral too, matching LeanerCloud#739's intent.
  Added a kpi-tile.test.ts case for it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
IAmJSD and others added 2 commits October 24, 2026 22:16
Conflicts:
- frontend/src/dashboard.ts: import block only. Kept this branch's
  buildKpiTile import (lib/kpi-tile) and main's upcoming-scope import
  (LeanerCloud#1365) side by side. No body conflicts; main's onboarding, upcoming
  scope and chart a11y changes merged cleanly around the kpi-tile loop.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@IAmJSD

IAmJSD commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator Author

Independent review (IAmJSD's agent, fresh context, given only the diff, the issue and the PR text): APPROVE 66f68acc2af6ed165c002eda4ed285270e05fb7a

Blocking:

  • none
    Non-blocking:
  • frontend/src/dashboard.ts:318-324: renderDashboardSummary still declares its own inline tile type ({kpi,title,value,valueSavings?,detail}), which duplicates KpiTileSpec from lib/kpi-tile.ts. Typing tiles as readonly KpiTileSpec[] would leave one definition.
  • frontend/src/styles/components.css:72-79: Opportunities tiles now also get the .kpi-tile hover lift and shadow-md, although they are not interactive. Home already had this. It makes static figures look clickable on both pages, so it could be a follow-up.
  • Behaviour change brought in by the b34340f resolution: the guard value !== NO_VALUE now lives in buildKpiTile, so the Opportunities savings tile showing NO_VALUE is neutral, where main colours it green. This is correct (fix(frontend): three money formatters, mixed placeholders, left-aligned money columns #739's intent) and covered by kpi-tile.test.ts. recommendations-kpi-tiles.test.ts does not exercise the Opportunities NO_VALUE path, though, and the PR body does not mention the change.
  • Remaining literal z-indexes (components.css:732 column-filter popover 1000, :950 column-visibility popover 1200, :1365 tooltip 1000) and the drawer scrim (responsive.css:104 rgba(0,0,0,0.4)) predate this PR and sit outside the dialog ladder. A popover at 1200 renders above .modal (1000) and the Archera page (1100). This is pre-existing and not a regression.
    Attacked: Read the full origin/main...HEAD diff (main 049c617 == merge base). Remerge-diffed all five merges: 3ded379, b34340f and 83e9ba9 had import-only conflicts plus b34340f's renderDashboardSummary body. In that body, main's fix(frontend): three money formatters, mixed placeholders, left-aligned money columns #739/fix(frontend): one money formatter and placeholder, right-align money #1376 NO_VALUE guard was moved into buildKpiTile intact, so nothing from main was dropped. fc0882a and 66f68ac were clean. Checked that every Opportunities formatter (formatScaledRange, formatSavingsRange, formatPaybackRange, NO_VALUE) returns plain text, so the move from innerHTML to textContent shows no raw entities. Checked .card .value/.card h3 specificity against .kpi-tile-value, the #recommendations-summary grid rules, .upcoming-card border-left ordering after the new border shorthand, every z-index and scrim literal left in CSS, and that loadYtdSparkline still finds the YTD spark slot. Ran kpi-tile, recommendations-kpi-tiles, surface-css and dashboard jest suites in a scratch worktree: 159 passed, 1 skipped.

@IAmJSD

IAmJSD commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator Author

Local verification at 66f68acc merged onto current main (049c617f; merge-tree c94e793d; Linux, Node, headless Chromium):

  • This branch's own checks passed at this head: lint, typecheck, build, the full jest suite, the full Playwright e2e suite, and the git-secrets checks (see Verification in the PR body).
  • The combined tree of every UI PR in this merge wave (tree 22a093f5, main plus 5 PRs) passes npm ci, lint, typecheck, build, jest (1 skipped, 4374 passed, 4375 total), and the full Playwright suite (259 passed).
  • CI is green on 66f68acc (CI Success, Run pre-commit hooks).
  • The independent review above approves this exact head.
  • Nothing was applied to any cloud.

@IAmJSD
IAmJSD marked this pull request as ready for review October 24, 2026 22:57
@IAmJSD
IAmJSD merged commit d82f0dc into LeanerCloud:main Oct 24, 2026
29 checks passed
@IAmJSD

IAmJSD commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator Author

Marked ready and merged by IAmJSD's agent, at IAmJSD's request (IAmJSD reviewed the before/after screenshots; cristim is traveling), after the independent review and local verification above, all at 66f68acc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/m Days impact/many Affects most users priority/p2 Backlog-worthy severity/medium Moderate harm triaged Item has been triaged type/chore Maintenance / non-user-visible urgency/this-quarter Within the quarter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant