Skip to content

NAS-142557 / 27.0.0-BETA.1 / Bump @truenas/ui-components to 0.7.1 and name unnamed spinners once - #13970

Merged
AlexKarpov98 merged 8 commits into
masterfrom
NAS-142557
Aug 27, 2026
Merged

NAS-142557 / 27.0.0-BETA.1 / Bump @truenas/ui-components to 0.7.1 and name unnamed spinners once#13970
AlexKarpov98 merged 8 commits into
masterfrom
NAS-142557

Conversation

@AlexKarpov98

@AlexKarpov98 AlexKarpov98 commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Bumps @truenas/ui-components from ~0.5.1 to ~0.7.1 and does the work that bump requires.

What the bump broke, and the fixes

TN_DIALOG_LABELS / TnDialogLabels gone renamed TN_DIALOG_CHROME_LABELS / TnDialogChromeLabels — the provider added on this branch no longer compiled
TnTableLabels gained 4 keys sortAscending, sortDescending, expandRow, collapseRow — our bundle was partial, so those four would have shipped untranslated
performance.now missing in specs setup-jest.ts replaces jsdom's performance with a mark/measure-only stub; 0.6+ calls performance.now() for its transition fallbacks, so every spec rendering a side panel or dialog threw
radio option test ids an option now emits radio-button-<x>, not radio-<x> (NAS-142376) — only edit-nfs-ace selects options by test id; radio-group- ids are untouched and the e2e suite references neither
tooltip accessible name TnFormFieldHarness.getTooltip() reads the help button's aria-label, and the library now strips markup out of it — an aria-label is announced verbatim, so <i> and <br> would be read out. The visible tooltip still renders them; the spec was comparing against the wrong one of the two

The four label providers added earlier on this branch had no spec. One now covers all of them and asserts each bundle against the library's own default bundle's key set, so the next release that adds a key fails here instead of shipping an English string to a translated app.

Naming the unnamed

0.7.x also warns, in dev mode, whenever a tn-spinner, tn-progress-bar, tn-dialog-shell or tn-side-panel renders with no accessible name — and jest-fail-on-console makes each warning a failing test. The library's fallbacks ("Loading", "Progress", "Dialog", "Side panel") were English literals with no way in, so the only per-site answer was an ariaLabel on each: about forty, most repeating their neighbours' string.

provideTnFallbackLabels fills the library's TN_FALLBACK_LABELS bundle from translated() once instead — the same shape as the select, autocomplete, dialog-chrome and table bundles this branch already provides. Providing it is also what stands the warning down, per the token's contract: an app-wide fallback is a decision, not a forgotten label. setup-jest registers the same token (via defineGlobalsInjections for Spectator specs, a global TestBed provider for the rest) so specs see the decision the app makes.

20 hand-written [ariaLabel] strings come out of the templates, four of which predate this branch. The 10 that stay read a signal or a field — a job's description, a Docker status, the app-loader's title — and say what no app-wide default can. The three dialogs that render [title]="" plus their own heading keep an explicit ariaLabel, since "Dialog" would be worse than the heading they already show.

The token itself shipped in 0.7.1 via iXsystems/truenas-ui-components#308.

Testing

Everything below is against published 0.7.1:

  • tsc -p src/tsconfig.app.json and yarn buildng (AOT, templates included) clean
  • eslint clean on every changed .ts and .html
  • src/app/modules — 248 suites / 2437 tests
  • src/app/pages by area — 757 suites / 5166 tests
  • everything else under src/app — 110 suites / 782 tests
  • Re-scanned all templates: 0 static [ariaLabel] strings left on a spinner or progress bar

https://claude.ai/code/session_01PP236cd5LPtgSVz1aiL8pm

… of per call site

The library holds no translation files, so its English defaults never reached
TranslateService and every call site had to rebind them. Two new providers feed
the TN_SELECT_LABELS / TN_AUTOCOMPLETE_LABELS tokens from translated(), so the
strings follow a language switch, and 107 duplicated attribute rows come out of
21 templates. Conditional bindings stay — an explicit input still wins.

Needs @truenas/ui-components with the label tokens; package.json bump pending
that release.

Claude-Session: https://claude.ai/code/session_01PP236cd5LPtgSVz1aiL8pm
TnButtonComponent renders [attr.type]="type()" and already defaults type to
'button', so the attribute changed nothing. tn-icon-button has no type input at
all, so its five were inert DOM attributes.

Claude-Session: https://claude.ai/code/session_01PP236cd5LPtgSVz1aiL8pm
Both sets of strings were literals inside the library templates with no input to
bind, so 112 dialogs and 78 tables announced English whatever the language. Two
providers feed the new TN_DIALOG_LABELS / TN_TABLE_LABELS tokens from
translated(), so they follow a language switch like the rest.

Nothing to remove at the call sites — the provider is the only way to reach
these. [emptyMessage] / [loadingMessage] stay per-table inputs.

Claude-Session: https://claude.ai/code/session_01PP236cd5LPtgSVz1aiL8pm
…oviders

Three things in 0.7.0 the ~0.5.1 code did not meet:

`TN_DIALOG_LABELS` / `TnDialogLabels` are gone — renamed `TN_DIALOG_CHROME_LABELS`
/ `TnDialogChromeLabels`, a letter apart from `TN_DIALOG_SHELL_DEFAULT_LABEL`,
which is a different thing. `TnTableLabels` gained four keys — `sortAscending`,
`sortDescending`, `expandRow`, `collapseRow` — so the bundle we provided was
partial and those four strings would have shipped untranslated. Neither compiled,
which is how both were found.

`setup-jest.ts` replaces jsdom's `performance` with a stub carrying only `mark`
and `measure`, so anything the stub omits is missing entirely. 0.6+ reads
`performance.now()` to time its transition fallbacks, and without it every spec
rendering a side panel or a dialog threw.

The four label providers added on this branch had no spec; one covering all of
them asserts each bundle against the library's OWN default bundle's key set, so
the next release that adds a key fails here rather than shipping an English
string to a translated app.

Claude-Session: https://claude.ai/code/session_01PP236cd5LPtgSVz1aiL8pm
0.7.0 warns, in dev mode, whenever a `tn-spinner`, `tn-progress-bar`,
`tn-dialog-shell` or `tn-side-panel` renders without an accessible name — and
`jest-fail-on-console` turns each warning into a failing test. The library's own
fallbacks ("Loading", "Progress", "Dialog", "Side panel") are English literals
with no way in, so the only per-call-site answer was an `ariaLabel` on each of
them: about forty of them, most carrying the same string as its neighbours.

`provideTnFallbackLabels` fills the library's new `TN_FALLBACK_LABELS` bundle
from `translated()` instead, once, the same way this branch already provides the
select, autocomplete, dialog-chrome and table bundles. Providing it is also what
stands the warning down, per the token's contract: an app-wide fallback is a
decision, not a forgotten label. `setup-jest` registers the same token — via
`defineGlobalsInjections` for Spectator specs and a global `TestBed` provider for
the rest — so a spec sees the same decision the app makes.

Twenty hand-written `[ariaLabel]` strings come out of the templates, four of them
predating this branch. What stays is the ten that read a signal or a field —
a job's description, a Docker status, the app-loader's title — because those say
something no app-wide default can.

The three dialogs that render `[title]=""` and their own heading keep an explicit
`ariaLabel`: "Dialog" would be a worse name than the heading they already show.

Needs a @truenas/ui-components release carrying TN_FALLBACK_LABELS
(iXsystems/truenas-ui-components#308); the package.json bump is pending it.

Claude-Session: https://claude.ai/code/session_01PP236cd5LPtgSVz1aiL8pm
@AlexKarpov98
AlexKarpov98 requested a review from a team as a code owner August 26, 2026 15:48
@AlexKarpov98
AlexKarpov98 requested review from william-gr and removed request for a team August 26, 2026 15:48
@bugclerk bugclerk changed the title NAS-142557: Bump @truenas/ui-components to 0.7.0 and name unnamed spinners once NAS-142557 / 27.0.0-BETA.1 / Bump @truenas/ui-components to 0.7.0 and name unnamed spinners once Aug 26, 2026
@bugclerk

Copy link
Copy Markdown
Contributor

Three conflicts, all from the version bump this branch is about:

- package.json / yarn.lock: master moved @truenas/ui-components to ~0.5.3,
  this branch to ~0.7.0. Kept 0.7.0 and regenerated the lockfile from it.
- setup-jest.ts: two imports landing on the same line. Both are wanted —
  master's `Component`, this branch's `TestBed` for the global
  TN_FALLBACK_LABELS provider.

Claude-Session: https://claude.ai/code/session_01PP236cd5LPtgSVz1aiL8pm
Comment thread src/app/pages/signin/signin.component.html
@codecov

codecov Bot commented Aug 26, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.00000% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.50%. Comparing base (1ba1031) to head (1468d1d).
⚠️ Report is 3 commits behind head on master.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
.../app/core/providers/tn-fallback-labels.provider.ts 0.00% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #13970      +/-   ##
==========================================
- Coverage   88.51%   88.50%   -0.01%     
==========================================
  Files        1889     1894       +5     
  Lines       71294    71327      +33     
  Branches     9204     9212       +8     
==========================================
+ Hits        63103    63129      +26     
- Misses       8191     8198       +7     

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

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread src/app/core/providers/tn-labels.providers.spec.ts
Comment thread package.json Outdated
@github-actions

github-actions Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Five findings, all LOW — nothing here blocks the merge. 🎉

Moving five bundles of library chrome copy onto DI tokens is the right shape: tn-dialog-shell's close/fullscreen buttons and tn-table's sort controls had no input to bind at all, so this is the only way they ever get translated, and the Record<keyof T, string> on each labelKeys makes a bundle that grows a key a compile error rather than a string that quietly ships in English. Deleting tn-select-labels.constant.ts and the ~200 template rows that re-bound it is a real subtraction — every tn-select in the app now gets the same wording without two attributes apiece. 👏 The performance.now() stub and its comment are a nice catch too.

The findings are all in the seams around that work, not the work itself:

  • Only one of the five new tokens is mirrored into setup-jest.ts, so specs and production disagree on tn-select/tn-autocomplete/dialog/table copy — visible in the two specs this PR changed to assert a bare 'Select an option'.
  • provideTnFallbackLabels is the one provider the new spec's bundle table skips, though TN_DEFAULT_FALLBACK_LABELS is exported and used a few lines away in setup-jest.ts.
  • TN_FALLBACK_LABELS ends up registered twice in setup-jest.ts; the root beforeEach already covers Spectator specs, so the defineGlobalsInjections entry is dead.
  • The five provider factories re-enumerate their keys after labelKeys already did, which the type system does not need.
  • auth-section.component.spec.ts now hardcodes helptext fragments instead of deriving them from helptextUsers.

Details and suggested diffs are inline.

@AlexKarpov98
AlexKarpov98 marked this pull request as draft August 26, 2026 19:28
…er changes

0.7.1 carries TN_FALLBACK_LABELS (iXsystems/truenas-ui-components#308), so the
provider added on this branch now runs against a released version rather than a
pending one.

The same release carries two changes of its own that specs here were asserting
against the old behaviour of:

- A radio OPTION emits `radio-button-<x>` where it used to emit `radio-<x>`
  (NAS-142376). Only `edit-nfs-ace` selects options by test id; `radio-group-`
  ids are untouched, and the e2e suite references neither.
- `TnFormFieldHarness.getTooltip()` reads the help button's `aria-label`, and the
  library now strips markup out of that — an `aria-label` is announced verbatim,
  so `<i>` and `<br>` in it would be read out. The visible tooltip still renders
  them, so the spec was comparing against the wrong one of the two.

Claude-Session: https://claude.ai/code/session_01PP236cd5LPtgSVz1aiL8pm
@AlexKarpov98 AlexKarpov98 changed the title NAS-142557 / 27.0.0-BETA.1 / Bump @truenas/ui-components to 0.7.0 and name unnamed spinners once NAS-142557 / 27.0.0-BETA.1 / Bump @truenas/ui-components to 0.7.1 and name unnamed spinners once Aug 26, 2026
Comment thread src/app/modules/jobs/components/job-item/job-item.component.html Outdated
CodeQL flagged the spec helper that stripped tags out of the expected helptext as
an incomplete sanitizer. It was never sanitizing anything — it derived the plain
text the tooltip's `aria-label` now holds — but the test does not need the whole
string to make its point. It asserts on a markup-free fragment of each helptext
instead, which reads the same either way and still says which of the two tooltips
landed on which option. No stripper, no alert.

Two review notes:

`unlock-sed-disks` carried a comment explaining why `type="button"` was spelled
out on each non-submit button. The attributes came off earlier on this branch, so
it described code that no longer exists — and read as an instruction to put them
back. TnButtonComponent defaults `type` to 'button', which is what makes their
removal safe here as everywhere else.

The three job progress bars now name themselves the same way: `job-name` already
wrapped the identifier in a translated sentence, and `job-item` and the two bars
in `job-progress-dialog` were naming themselves with a bare API method — a screen
reader announcing "pool.dataset.delete", dotted identifier and all, never having
been through `translate`.

Claude-Session: https://claude.ai/code/session_01PP236cd5LPtgSVz1aiL8pm
Comment thread src/setup-jest.ts
Comment thread src/setup-jest.ts
Comment thread src/app/core/providers/tn-labels.providers.spec.ts
Comment thread src/app/core/providers/tn-table-labels.provider.ts
@AlexKarpov98
AlexKarpov98 marked this pull request as ready for review August 26, 2026 21:04
@AlexKarpov98
AlexKarpov98 requested a review from aervin August 26, 2026 21:04
@AlexKarpov98
AlexKarpov98 merged commit 5e5366c into master Aug 27, 2026
16 checks passed
@AlexKarpov98
AlexKarpov98 deleted the NAS-142557 branch August 27, 2026 10:32
@bugclerk

Copy link
Copy Markdown
Contributor

This PR has been merged and conversations have been locked.
If you would like to discuss more about this issue please use our forums or raise a Jira ticket.

@truenas truenas locked as resolved and limited conversation to collaborators Aug 27, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants