feat(ui): give the surfaces a pill and an alert to share - #96
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two shared components, extracted from code that already existed rather than invented for the screen that comes next.
VatRateshad lost the weight of the others, and each carried its own margin. That is the trajectory frontend ADR 0003 § 3 predicts from the second copy on — there were four.TableCell, which now composes it instead of drawing it.Six Material Symbols icons come along:
check_circle,error,warning,hourglass,edit,visibility_off— taken from the set this project already uses, never redrawn.The icon is passed on a pill, derived on an alert
Not a detail, and it came out of review: a product's draft and retired share the neutral tone and mean opposite things — one is still being written, the other was taken out of sight. Deriving the icon from the tone would make them indistinguishable, which is the exact fault an icon exists to prevent. So a pill takes its icon from the caller: the tone expresses a contract, the icon says which one.
An alert is the reverse. A tone names the kind of news, and news of one kind always looks the same, so the caller has nothing to choose and nothing to get wrong.
role="alert"is carried by the component. The four hand-written ones each had to remember it.Nothing animates on a state
A state is read, not watched. The one animation left in the gallery is a
busybutton's spinner, which is an action in progress rather than a state — said here so it is a decision rather than an oversight.A defect I introduced, and how it was caught
Alertcarries no margin, where the class it replaced had one. On the sign-in screen — the only one that stacks in normal flow rather than with agap— the next field's notch, which rises onto its own border, landed on top of the alert and madeEmail addressunreadable.200 tests, lint, and three guards were all green. A screenshot was not.
The fix is not a margin inside the component, which would impose a layout on all four callers: the surface gives the space, as frontend ADR 0003 § 5 assigns it. Verified afterwards by measuring the rendered rectangles, not by looking again.
What changes on existing screens
VatRates's alert regains the weight of the other three. A correction.edit,check_circle,visibility_off.Tests
Two existing tests in
VatRates.spec.tsqueried.in-use, a selector the shared component does not produce. Rewritten on behaviour, with the user's standing approval, after checking the behaviour was intact — and stricter than before: they now also assert the sentence is announced (role="alert"), which is what makes a refusal reach someone who is not looking at the screen. The old selector guaranteed nothing of the sort.Pill.spec.tsadds 10: what each renders, that a pill without an icon renders none, that every tone is carried, that a pill never animates, that an alert announces itself, and that the three alert tones draw three different icons.Test plan
just frontend-check— typecheck, lint, theme tokens, type scalejust frontend-test— 200 tests in 25 filesjust frontend-build— gallery absent from the built assetsjust e2e— desktop-light 4, desktop-dark 4, mobile-light 5Backend untouched, so
backend-check,backend-testandapi-checkdo not apply.Part of #81's aftermath, and the groundwork for #75 — the image screen needs both, and neither was invented for it.