WEBDEV-9050: Migrate the donation banner thermometer into elements

[jbuckner]

WEBDEV-9050. First of the five in WEBDEV-8602, the donation form migration. Based on main, not stacked. Only the petabox banner uses the thermometer, so it gets its own directory rather than living under the form.

donation-banner-thermometer moves over as ia-donation-thermometer, with a story and the tests on Vitest.

What changed beyond the move:

• shared-resize-observer is gone. The element owns a plain ResizeObserver for the fill and the value label. The old code only observed the label when currentAmountMode changed, and the label re-rendered in a new spot when it swapped sides of the fill, so after the first swap it was never measured again. Both targets are observed once and followed if Lit replaces them, with a test that fails on the old behavior.
• The value label is one node placed by offset from the end of the fill, rather than being re-rendered into a different parent on each side. That's what was raising ResizeObserver loop completed with undelivered notifications on every left-to-right swap (8 errors over four swaps, none now). It lands as an uncaught window error rather than a console call, so petabox's handler would have collected it on every banner resize. Being out of flow, the label also can't stretch the fill past its percentage.
• The progress bar is the bar. role="progressbar" and the aria-value* attributes sit on .thermometer-background instead of a wrapper that also held the goal text, which was keeping $6.5MM GOAL out of the accessibility tree. aria-valuenow and aria-valuemax are clamped to a valid range.
• Out-of-range inputs empty the bar instead of emitting width: NaN%. A zero, negative or non-finite amount or goal reads as no progress, and the fill's width carries a 0% fallback so a missing custom property can't paint a fully funded fundraiser.
• The pill shape doesn't measure the host anymore. A large border radius rounds any height, so the host observer and --bannerThermometerHeight are gone. Only the fill is clipped, so the label isn't cut off when it sits past the end of the track, and box-sizing makes the bar paint at the height it declares (it was 2px taller).
• CSS variables moved to --ia-donation-thermometer-* (height, fill/track colors, border, radius, value colors, goal padding). The goal text color follows --ia-theme-primary-text-color. petabox sets three of the old names and gets updated when it switches over.
• The progress bar has an accessible name (label, default "Donation progress"), and the default strings go through msg().

Known, not fixed here: below about 240px the value label runs into the goal text. main handled that width by pinning the track at 84px and painting a 0px fill on a half-funded bar, so there's no width where main was right and this isn't. The proper fix is a container query that drops the goal text under a threshold, filed separately.

Verified locally: tsc clean, build, full suite 654 passing, eslint 0 errors and prettier clean.

🤖 Generated with Claude Code

https://claude.ai/code/session_01JpyQxVSkZFrWxxGamCcNhY

[github-actions]

PR Preview Action v1.8.1
🚀 View preview at https://internetarchive.github.io/elements/pr/pr-97/
Built to branch ghpages at 2026-09-24 18:20 UTC. Preview will be ready when the GitHub Pages deployment is complete.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 89.55224% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.17%. Comparing base (76db2d8) to head (9f25dfd).

Files with missing lines	Patch %	Lines
...ia-donation-thermometer/ia-donation-thermometer.ts	89.55%	1 Missing and 6 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #97      +/-   ##
==========================================
- Coverage   91.22%   91.17%   -0.05%     
==========================================
  Files          60       61       +1     
  Lines        2518     2585      +67     
  Branches      582      601      +19     
==========================================
+ Hits         2297     2357      +60     
- Misses         77       78       +1     
- Partials      144      150       +6     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[nsharma123]

Code-review pass on the migration. The move itself reads faithfully against the original banner-thermometer, and I verified the ResizeObserver claim holds: patching updated() back to the old "only re-observe when currentAmountMode changes" semantics makes keeps measuring the value label after it changes sides fail, so that regression test does what the description says.

No blockers. Everything below is behaviour carried over from the original component rather than something this PR broke — flagging it here because this PR is the accessibility/ResizeObserver pass, so it's the natural moment.

QA results (all steps pass) are on WEBDEV-9050. Locally: 643 tests pass, eslint 0 errors, prettier clean, new element at ~91% statements.

[nsharma123]

role="progressbar" is on the wrong node (medium, a11y)

The role and the four aria-value* attributes sit on .container, which wraps both the bar and .donate-goal. So the goal text is a descendant of the progressbar — I confirmed this in the DOM on the review app.

Per the ARIA presentational-children rule, progressbar descendants aren't exposed to assistive tech, and aria-label overrides content for the accessible name either way. Net effect: $6.5MM GOAL never reaches a screen reader, and in message mode WE'VE ALMOST REACHED OUR GOAL! is lost entirely.

The ticket's own QA wording — "the bar inside the element's shadow root is role="progressbar"" — describes the intended structure better than the code does.

Fix is small: move role and the aria-* attributes down onto .thermometer-background (line 81), which is the actual bar and is already a sibling of .donate-goal rather than an ancestor.

[jbuckner]

Fixed in fcd9b92. role and the four aria-value* attributes are on .thermometer-background now. The outer .container was only there to hold the role, so it is gone too.

New test keeps the goal text out of the progressbar asserts the progressbar is the background and that it does not contain .donate-goal (with an exist check on the goal so it cannot pass vacuously).

[nsharma123]

aria-valuenow isn't clamped to aria-valuemax (low, a11y)

Reproduced on the review app at the QA step that sets Current amount to 7000000 against the 6500000 goal: the bar reports aria-valuenow="7000000" with aria-valuemax="6500000".

percentComplete clamps the visual fill to 100%, but the aria value is passed through raw, so an over-goal fundraiser reports out-of-range values. Worth clamping aria-valuenow to goalAmount here (aria-valuetext already carries the true unclamped amount for anyone who wants it).

[jbuckner]

Fixed in fcd9b92, and aria-valuemax with it. Both go through getters now:

private get progressMax(): number {
  return Number.isFinite(this.goalAmount) && this.goalAmount > 0
    ? this.goalAmount
    : 0;
}

private get progressValue(): number {
  if (!Number.isFinite(this.currentAmount)) return 0;
  return Math.min(Math.max(this.currentAmount, 0), this.progressMax);
}

So a negative or NaN goal reports aria-valuemax="0" rather than a range with min above max. aria-valuetext still carries the displayed amount.

Worth flagging that my first attempt named these ariaValueNow / ariaValueMax, which collide with ARIAMixin on HTMLElement (both are public string | null there). A private member of that name breaks the class's structural assignability to Element, which cascaded into 22 tsc errors including a TS1238 on @customElement that pointed nowhere near the cause. pnpm lint and pnpm test were both green through it, because neither typechecks. Separate ticket for a tsc --noEmit CI step, I think.

[nsharma123]

Re-observing on every render raises a ResizeObserver loop error on each left→right label swap (medium)

Reproducible on the review app and locally. Every time the amount label moves from inside the fill to outside it, Chrome raises:

ResizeObserver loop completed with undelivered notifications.

Driving six amount changes on the review app gave exactly three errors — one per left→right transition, with the reverse direction clean. It also fires on the ~600px responsive step, so the ticket's "No console errors either way" QA line isn't quite accurate for this element.

Cause: the label is a different DOM node on each side, and the replacement gets newly observed from inside the observer's own delivery cycle. Moving it out of the fill also changes the fill's flex width, so there's more to deliver than one cycle allows.

This isn't introduced here — the old code hit it too — but calling observeParts() on every update makes it more frequent: across an identical four-swap sequence locally, old observe-once behaviour produced 2 errors and this produces 4.

Worth caring about because it surfaces as an uncaught window error rather than a console API call, so anything hooked to window.onerror in petabox will collect it on every banner resize.

Heads-up on the fix: I tried the obvious cheap one — guarding the fill so it isn't re-observed each render — and it does not help (still 4 errors, though all 23 tests stay green). Getting rid of this properly wants a single stable label node that keeps its DOM position and is placed on either side of the fill with CSS, instead of being re-rendered into a new slot. That would also stop the label perturbing the fill's width, which is a separate latent oddity: min-width: auto on the flex item lets a wide label stretch the fill past its intended percentage.

[jbuckner]

Fixed in fcd9b92, and you were right that the cheap guard is not enough. The label is one node in one place in the DOM now, placed by offset from the end of the fill:

.thermometer-value { left: var(--fill-end--, 0%); width: max-content; }
.value-left .thermometer-value { transform: translateX(-100%); }

Measured with an in-page window.addEventListener("error") (Playwright's pageerror never sees these, which is the same reason a console check misses them), over four left-to-right swaps plus a container-resize pass: 8 errors on c116430, 0 now. Side decisions are unchanged, L R L R L R L R on both.

Your point about the fill's width was the half I would have missed. The label is out of flow, so min-width: auto on the flex item cannot stretch the fill any more, and the fill lands on its exact percentage at every width I tried. I also dropped display: flex from .thermometer-background so that shrink path is gone rather than dormant.

[nsharma123]

Doc comment says "once", but the fill is re-observed on every render (nit)

Line 150 runs on each updated(), and per spec observe() on an already-observed target removes and re-adds it — which resets its last-reported size and costs an extra notification every render. The value label just below is guarded against exactly this (line 153); the fill isn't.

Cheap to guard the same way, which would also make the "Observes the fill once" comment true. To be clear though, I tested this in isolation and it does not fix the loop error above — see the comment on updated().

[jbuckner]

Fixed in fcd9b92. Both targets go through one helper that leaves an unchanged target alone:

private observeInPlaceOf(
  observer: ResizeObserver,
  observed: Element | null,
  current: Element | null,
): Element | null {
  if (current === observed) return observed;
  if (observed) observer.unobserve(observed);
  if (current) observer.observe(current);
  return current;
}

The label still needs following, but only because currentAmountMode: off discards it, not because it changes sides. Comment says that now.

[nsharma123]

goalAmount = 0 renders style="width: NaN%" (low, robustness)

Both of these are reachable straight from the demo's number inputs:

• goalAmount = 0, currentAmount = 0 → 0/0 → NaN → the element emits the literal attribute style="width: NaN%" (verified in the DOM).
• A negative currentAmount → a negative percentage.

In both cases the browser drops the invalid width, so the fill falls back to content-based flex sizing — I measured ~42px and ~91px respectively — instead of collapsing to zero. So it renders a visible, wrong-looking fill rather than an empty bar.

Clamping to [0, 100] and guarding the zero goal covers both, e.g.:

private get percentComplete(): number {
  if (!(this.goalAmount > 0)) return 0;
  const pct = (this.currentAmount / this.goalAmount) * 100;
  return Math.min(Math.max(pct, 0), 100);
}

[jbuckner]

Fixed in fcd9b92, close to your version:

private get percentComplete(): number {
  const goal = this.progressMax;
  if (goal === 0 || !Number.isFinite(this.currentAmount)) return 0;
  const percent = (this.currentAmount / goal) * 100;
  return Math.min(Math.max(percent, 0), 100);
}

Guarding the amount as well as the goal, since Math.min(Math.max(NaN, 0), 100) is still NaN. formatNumber returns $0 for a non-finite input too, so the label cannot read $NaNMM.

The fill's width comes from a --fill-end-- custom property now, and it is written var(--fill-end--, 0%). Without the fallback a missing property makes the declaration invalid, width falls back to auto, and the bar paints as a fully funded fundraiser. Tests for a zero goal, a negative amount, a NaN amount and a NaN goal.

[nsharma123]

Bar paints 2px taller than the declared height (low, CSS)

This rule is content-box with a 1px border on line 283 and height: 100% on line 281, so the bar renders height + 2px and overflows the host box. Measured on the review app: 22px against the 20px default, and 42px at the QA step's 40px.

Invisible in petabox (which sets the border to 0) and easy to miss behind the pill radius, but it shows up with the Square preset or any non-zero border — and it matters a little more now that :host declares an explicit height, since the painted bar no longer matches it.

box-sizing: border-box on this rule fixes it.

[jbuckner]

Fixed in fcd9b92. box-sizing: border-box on the rule. Measured 22px to 20px at the default and 42px to 40px at the 40px step.

New test paints the bar at its declared height pins it at 40px, which fails at 42px if the rule goes away.

[nsharma123]

Missing HTMLElementTagNameMap declaration (nit)

The README's "Adding a Component → Naming" section asks for this explicitly ("Declare each element in HTMLElementTagNameMap so querySelector is typed and a mistyped tag in a template is caught at build time"):

declare global {
  interface HTMLElementTagNameMap {
    'ia-donation-thermometer': IADonationThermometer;
  }
}

Fair warning that it's a weakly-adopted rule — only ia-item-navigator and ia-status-indicator currently comply — so entirely reasonable to skip. But it's free, it'd drop the cast in ia-donation-thermometer.test.ts, and ia-status-indicator puts it at the end of the file like this if you want a template.

[jbuckner]

Skipping this one. Nothing here needs it: no createElement("ia-donation-thermometer"), and no untyped querySelector being assigned to the element type. Where a call site genuinely wants the type I would rather say so there, with querySelector<IADonationThermometer>(...), than declare it globally.

Fair point that it would drop the cast in the test, but one cast is cheaper than the boilerplate on every element.

Your 2-of-17 count is the real signal though. Either the README line should go or all 17 elements should comply, and picking that off in a migration PR just widens the split. Happy to file it as its own ticket if you think it should be the former.

[jbuckner]

Thanks Neeraj, all of it was worth picking up. Six fixed in fcd9b92, one skipped, details in the inline replies.

The resize loop went 8 errors to 0 over four label swaps. Your read on the cause was right and the cheap guard really does not help: the label is one node placed by offset now, and out of flow so it cannot stretch the fill.

Two things beyond your list:

Narrow tracks still have a problem, and I am leaving it. Clipping now covers only the fill, so the label is never cut off. But below about 240px the label runs into the goal text instead (31px of overlap at 200px, 71px at 120px). Nothing to do with the banner at real widths, and there is no width where main was right and this is not:

container	main fill	new fill	true fill
120	0	0.3	0.3
200	0	40.3	40.3
240	38.5	60.3	60.3
280	78.5	80.3	80.3

Below ~200px main pinned the track at 84px and refused to shrink, so the component overflowed its container, and the fill read 0px on a half-funded bar. That is the component getting its one job wrong, so I would rather take the overlap and fix it properly with a container query that drops the goal text under a threshold. Filed as WEBDEV-9062.

CI does not typecheck. My first cut named the new getters ariaValueNow / ariaValueMax, which collide with ARIAMixin. 22 tsc errors, pnpm lint and pnpm test both green, breaks at pnpm build. Worth a tsc --noEmit step, filed as WEBDEV-9063.

Still open: the clip layer takes the full border radius rather than the border's inner curve, so at an intermediate radius like 4px the fill can paint about 1px over the corner. Invisible at both shipped presets (9999px and 0). Left it.

654 tests pass, tsc clean, build clean, eslint 0 errors.