Skip to content

feat: submission celebration, extension guardrails, and Unit Hub rework - #234

Closed
Clupai8o0 wants to merge 89 commits into
ui/notification-linksfrom
ui/inbox-extension-card
Closed

Clupai8o0 wants to merge 89 commits into
ui/notification-linksfrom
ui/inbox-extension-card

Conversation

@Clupai8o0

Copy link
Copy Markdown

Summary

This slice lands the confirm-in-place submission flow, with a shared celebration system across tasks, units and portfolios. It closes the task-extension date bypass, reworks the Unit Hub page (read tracking, session timing, markdown bodies), and carries the accessibility, contrast and performance fixes that came out of reviewing the above.

Stacked PR

This PR targets ui/notification-links. The diff shown (ui/notification-links..ui/inbox-extension-card) is this slice only: 184 files, +17,434/-2,660 across 89 commits, one of which is the merge of ui/notification-links at 78902d4. It must not be merged before ui/notification-links merges.

The stack is a graph, not a straight line. theme/status-and-globals (PR #192) is an ancestor of everything below it. cc4a834e6 sits on this branch's own line, and ui/panel-layout and ui/a11y-audit are two sibling forks off it. Neither is an ancestor of the other. Both were merged into ui/inbox-extension-card directly, at 7972ea6 and 3c19fb3.

ui/notification-links was cut later, from a552a1a7c on this branch, which is already downstream of both of those merges, and it was merged back in at 78902d4. So ui/panel-layout and ui/a11y-audit are ancestors of ui/notification-links and need no separate merge step ahead of this PR. Only ui/notification-links does.

git merge-base ui/panel-layout ui/a11y-audit                        # cc4a834e6
git merge-base --is-ancestor ui/panel-layout ui/a11y-audit          # false
git merge-base --is-ancestor ui/a11y-audit ui/panel-layout          # false
git merge-base --is-ancestor ui/panel-layout ui/notification-links  # true
git merge-base --is-ancestor ui/a11y-audit ui/notification-links    # true

What changed and why

Submission and celebration. The file uploader is now one panel that collapses from uploading into uploaded instead of swapping components. The upload dialog confirms the submission inline, with a step indicator, a "what to upload" callout and per-file status rows, rather than sending the student on to a separate success state. Task.processTaskStatusChange now remembers the status a submission started from (statusBeforeSubmission), so a move out of Fix and Resubmit reads as a resubmission rather than a fresh one. It takes a new claimCelebration argument and returns SubmissionCelebration | null, so a caller with a surface of its own renders the celebration inline instead of an overlay firing over the page, and the snackbar stays suppressed either way. The same milestone-celebration machinery drives a portfolio-submitted moment and a "tasks signed off since you last opened this project" dialog on the project dashboard, comparing current statuses against the last-visit snapshot. A first visit records silently, and staff and the portfolio view never check.

Two races surfaced while building this. canClose is wired as the dialog's closePredicate, and Material runs a closePredicate for programmatic closes too, so a successful upload could still prompt "discard changes?". The predicate now answers on the submission having landed first. Separately the uploader reports completion on an 800ms timer that was never cancelled and had no ngOnDestroy to cancel it in, so closing the dialog inside that window called back into a destroyed component. The timer is cancelled on destroy, and the dialog records the submission itself as it goes, which leaves the celebration unclaimed for the dashboard.

Extension requests. The past-deadline date was enforced by a disabled field plus a validity getter. hasDateRange went false once the deadline was behind the student, and isDateInRange answered that by returning true outright, which switched the check off rather than relaxing it. The field has no form control, so Material's own min and max validators never ran, and an empty calendar only stops the calendar. Commit 6059112 records proving this against the component, which sent a 173 week extension request. addEvent now refuses the write outright once there is no date range left, so extensionDuration can only ever be derived from a date that was allowed. Once the final deadline has passed the field asks for the earliest date and disables entirely, and submitApplication checks the same condition the button does rather than relying on the disabled attribute alone. The extension reason renders as markdown, and the dialog is tidied, with the task and current due date under the title and a reason field that grows with its text and warns near the limit.

Unit Hub. Announcements track read and unread state per user in localStorage, with a "New" chip and mark-as-read. Session cards highlight "Happening now" and "Starts in N min" off one shared clock, pin an "Up next" card, and render cancelled sessions as clearly cancelled in both the feed and the details dialog. Announcement and session bodies render through a small markdown pipe, a private marked instance plus DOMPurify with an allow-list of basic tags, href as the only attribute, an https:/mailto: URI regexp and rel="noopener noreferrer" on links. Notifications for Unit Hub items deep-link to /unit-hub with the announcement or session id and open its details directly. The notification settings page gained a fourth category row for Unit Hub, with its own Email, Push and Session reminders toggles under a receiveUnitHubNotifications master switch. Several passes converge the page's cards, dialog and Teams composer onto the app's own labelled-card and filled/outlined button styles.

Accessibility and contrast. Commit edfd7d0 brings seven failing contrast pairs up to WCAG in both themes, each ratio computed from the token values with the WCAG relative-luminance formula. Among them: the phone bottom nav's active tab (2.18:1 to 4.55:1 in dark), a focused Material form field's outline, which fell back to M2's primary and drew dimmer than its own resting state in dark (2.40:1 against a 3.08:1 resting edge), muted text in solid cards at 3.64:1, chip labels against their own glass fill at 3.18:1, an outlined control's edge inside a solid card at 1.95:1, and the burndown chart's hidden-series legend (2.31:1 to 5.34:1). A separate commit, 41a3ab0, fixes the outlined-button treatment in the dark theme, where the outline mixed to #385A8D and landed at 1.60:1 against a card or dialog, now 3.66:1 off the link and control-border tokens, with light mode unchanged to the byte. Two residuals are left in place because they are unreachable in current markup, noted below.

Three controls get real accessible names: the summary-email select, where Material renders the select as the combobox itself so a <label for> pointed at nothing, a status icon that duplicated its own text as an aria-label, and the per-unit "Hide Completed" filter checkbox. Seven templates ask a textarea to autosize and set a minimum of three rows, but TextFieldModule was never imported, so cdkTextareaAutosize had always been an inert attribute and all seven rendered at the browser default of two rows.

Panel layout and inbox. startResize kept a drag's teardown in one field and overwrote it without looking, so a second pointer grabbing the resize handle before the first lifted stranded the first drag's document listeners, and the panel went on resizing from a pointer with no button held for the rest of the session. stopDragging is idempotent, so calling it before a new drag starts is the whole fix. narrowTaskInbox stopped being a getter that measured the DOM inside change detection, which could answer differently on the check pass than on the verify pass, an NG0100. It is now a plain field kept by a ResizeObserver and torn down on destroy. Panels gained a full-screen button, reused on the inbox and task dashboards, which holds notes, history and task details to a 960px reading measure while full screen and leaves the PDF viewers the whole width.

Performance and startup. The skeleton shimmer no longer drives off an inherited :root custom property recomputed on every element behind a :has(). It is a plain transform on the placeholder's own pseudo-element now, same look, no document-wide recalculation. The splash screen is redrawn, with a stroke-drawn mark, a progress line and a pre-boot loader in index.html matching it so the hand-off does not jump.

Profile and smaller fixes. Institution-managed accounts show first name, last name and sign-in email as read-only facts instead of inputs, driven by the institutionalIdentityManaged flag that already exists on the user model, and readOnlyIdentityKeys omits them from the update body so nothing unsendable gets sent back. The save bar only shows "Save changes" when the form is dirty, and a save response landing after the view is gone no longer arms a timer against a destroyed component. Students can choose how often the summary email arrives, with off, daily, weekly and monthly options against digestFrequency. The user icon falls back to initials when the avatar photo hash is unavailable, and buttons across the app converge on one filled and one outlined style.

Test hygiene. Commit cd841d4 removes eighteen tests that could not fail: peer-progress-indicator.fixtures.spec.ts asserted only on constants declared in the same file, unit.reviewed.spec.ts carried a verbatim copy of unit.spec.ts that arrived in a merge-reconciliation commit, one test inside describe('waiting label') was a byte-identical copy of a previousTask test, and a not.toThrow wrapped three inputs already asserted three lines above. Each is replaced with an assertion against real behaviour where coverage was missing. A Node 24 regression is fixed in the vitest setup: Node 22.4 added its own localStorage behind --localstorage-file, and from Node 24 on globalThis.localStorage exists as a getter returning undefined, which vitest's jsdom environment then skips copying over. The setup defines a store only when there is not one already. Commit e62b1f7 records this as 31 specs failing locally across panel-layout, pdf-viewer and tutor-discussion, and notes CI runs Node 22 without the flag and never saw it.

Deliberately not changed

  • A remembered panel width restored on layout still does not call notifyResize, so a PDF viewer inside it will not know to re-measure. Flagged in the layout fix as a behaviour change, not a fix to that defect.
  • togglePushNotifications still does not store its own subscription. Cancelling it mid-chain could leave the browser and server disagreeing about whether the device is subscribed. The worst a late response does today is show a snackbar on the wrong page, judged the better trade.
  • Two contrast residuals are left as-is because they are unreachable in current markup: a control on a solid card's inner surface at 2.74:1 and a warn-coloured button there at 1.89:1. The error token is not re-pointed by that scope.
  • The per-unit "Hide Completed" filter still uses role="menuitem" with no checked state, so it announces the same whether it is on or off. Fixing it means moving to menuitemcheckbox and hiding the visual box, which is a redesign rather than a label fix.
  • Unit Hub markdown rendering is client-side only. The API still stores plain text, so Teams sync and outbound emails show the raw markdown characters.
  • Unit Hub read state has no server-side read-receipt API behind it, so read times live in localStorage per user and per browser rather than syncing across devices.

Testing

43 spec files are touched in this diff. 13 are new: animated-check, milestone-dialog, portfolio-celebration.service, submission-timing and task-status-seen.service under common/celebrate/; announcement-read.store, hub-markdown, session-timing and status-guide under unit-hub/; plus about-doubtfire-modal-content, user-icon.component.secure-context, burndown-hover and progress-burndown-chart.hover. One is deleted, peer-progress-indicator.fixtures.spec.ts, as part of the dead-test cleanup.

Counting it( lines in the diff gives 193 added against 23 removed, a net of 170, plus one added test( line. Those counts come from git diff --stat and a grep over the diff. No suite was run for this description, so nothing here claims a pass or a fail.

Review guidance

Three areas carry most of the behavioural risk.

  • src/app/api/models/task.ts with src/app/common/celebrate/submission-celebration.service.ts and submission-timing.ts. processTaskStatusChange sits on the path every task status transition goes through app-wide, and this slice adds a claimCelebration parameter, changes the return type to SubmissionCelebration | null, and puts the statusBeforeSubmission bookkeeping in front of it.
  • src/app/tasks/modals/upload-submission-modal/upload-submission-modal.component.ts and src/app/common/file-uploader/file-uploader.component.ts. The closePredicate, the uploader's 800ms completion timer and the ExternalName subscription all interact here. The dialog's destroy ordering is the thing to check.
  • src/app/visualisations/progress-burndown-chart/burndown-hover.ts (new, 229 lines) and progress-burndown-chart.component.ts (+488). New touch, keyboard and live-region interaction layered onto the existing chart render.

Everything under src/app/unit-hub/, the profile form restyle, the button unification, the study-essentials grid and the About and splash restyles are large by line count but are mostly markup and stylesheet passes over data and logic that already existed.

Merge order

Base is ui/notification-links. The full order is theme/status-and-globals (#192), then the panel layout and accessibility PRs in either order, then notification-links, then this one.

This is the last and largest slice. It is the only one that should be read as a whole feature rather than a refactor, and the review guidance above names the files that carry the risk.

…ed cards

Announcement and session cards get a meta or facts row with icons and one
footer row of outlined actions on the content inset. Notices share one
callout style with a leading icon. The manage forms split into Details,
When, Where and Visibility groups and each existing item is its own card.
Dividers between sections are gone, including the one under the header.
Card edges use a shared mixin so cards stay visible on dark surfaces.
…d grouping

The body and description get their own quiet card on the same inset as the
facts card, the cancelled notice becomes a callout with an icon, and the
dialog footer loses its top rule. The Teams composer groups the meeting and
invitation fields under labels and gives its button an icon.
Pinned is shown by its chip. Every announcement card now uses the neutral
card edge at rest.
…k them read

Unread cards get a left accent bar, a bold title and a New chip, and the
heading shows the new count with a Mark all as read button. Opening the
details or pressing the card's mark as read button records a read time.
An announcement edited after that time counts as new again.

There is no read-receipt API, so read times are kept per user in
localStorage and are per browser. Storage failures count as all unread,
ids that leave the feed are pruned, and demo content is never stored.
…cal overrides

The header actions lose their pill radius and hand-set primary fill, show
more and mark all read lose their pill radius, and New session becomes
outlined so each group has one filled action. Buttons in the page, the
details dialog and the Teams composer share a 44px height, and Try again
and the show more buttons get icons.
…n-status store

Shared pieces for task milestone moments. The check draws a ring then a
tick with stroke-dashoffset, the particles are a single burst of
transformed dots, and both fall back to a static fade under reduced
motion. The seen-status service keeps the last status a student saw per
task under ontrack.taskStatusSeen.<userId>.<projectId>, and treats a
first visit or blocked storage as nothing to celebrate.
…cess moment

When a student's submission lands, a small confirmation draws a check at
the bottom of the screen and leaves after 2.5s, or on click or Escape.
The copy follows the task dates: on time before the target date, a
calmer Ready for Feedback after it, and a neutral note after the due
date. A move from Fix and Resubmit reads as a resubmission. It replaces
the success snackbar for that case only, and staff never see it.
…the unit

Opening a project dashboard compares task statuses with the ones stored
on the last visit. Newly completed tasks open a centred dialog with a
staggered list, the progress toward the target grade moving from its old
share to the new one, and a quieter list of tasks now ready to discuss,
needing changes or with new feedback. A first visit records silently,
staff and the portfolio view never check, and ?celebrate=preview replays
it in a development build without touching storage.
The details dialog renders bodies and descriptions through a small
standalone pipe that parses with marked and sanitises with DOMPurify:
basic formatting only, no images, frames, styles or handlers, and links
open in a new tab with noopener. Typed HTML still shows as text. Long
bodies sit at a 68ch measure and clamp at 40vh behind a Show full toggle.
Feed cards show a plain text excerpt, and the staff editor gets a
formatting hint with a Write and Preview toggle.

The API still stores plain text, so Teams sync and emails show the raw
markdown characters.
…ialog

A cancelled card sinks toward the page with an error-tinted edge and a
left accent bar, replaces the chip and red line with one banner that says
Cancelled: this session will not run, and its type chip goes neutral so
cancellation is the only colour. The title stays struck through at full
text colour and screen readers hear Cancelled first. Cancelled sessions
sort after active ones on the same day, and the details dialog header
gets the same bar, banner and struck title.
…ashboard

The progress dashboard reports tasks complete by count, so a weighted
share in the dialog showed a different number beside it.
… and open deep links

Session cards show Happening now with a gently pulsing dot and a success
accent, Starts in N min with a primary accent inside the hour, or Today,
driven by one minute clock that stops on destroy. Cancelled sessions get no
highlight. A live session's Join becomes the filled action, and an Up next
card at the top of Upcoming shows the next active session with its join
and calendar actions.

?announcement= and ?session= open that item's details once the feed loads,
mark an announcement read, and are then removed with replaceUrl without
reloading the feed. A missing item shows a snackbar. The header gets a Get
notified link to the notification settings on the profile page.
…ped sections

A compact header with the logo, name and a one-line description. Lead
contributors show as equal-height cards in a 3, 2 or 1 column grid with
named avatars, text-colour names that wrap cleanly and a labelled profile
link. The contributor table and licence notes sit under their own section
labels, and Close is an outlined button in a right-aligned footer. Cards
rise in with a short stagger that is skipped under reduced motion.
Draw the OnTrack mark stroke by stroke with a calm sheen loop, fade the
wordmark up after it and crossfade the status text. Exit faster than it
enters. A pre-boot loader in index.html matches the splash so the hand-off
does not jump. Data loading shows a 2px indeterminate line with a caption
after 1.5s. Skeletons share one sheen phase so they move together. Reduced
motion keeps everything static.
Node 22.4 added its own localStorage behind --localstorage-file, so from
Node 24 on, globalThis.localStorage exists as a getter that returns
undefined. Vitest's jsdom environment skips any key already present on the
global, so jsdom's real localStorage never gets copied across and every
spec that touches it dies on the first line of its beforeEach.
sessionStorage is copied normally, which is what made this look like a
jsdom bug rather than a Node one.

Locally that was 31 tests across panel-layout, pdf-viewer and
tutor-discussion failing for a reason none of them had anything to do
with. CI runs Node 22 without the flag and never saw it, so the suite
meant two different things depending on where it ran.

Define a store only when there is not one already, so CI is untouched.
Three faults that all start from the same place: the uploader keeps
isUploading set after the request settles, because the panels that report
the outcome render under it.

The dialog read that as "still uploading" and refused to close. After a
failed upload the student could use Try again or Cancel inside the error
panel, but Escape and the backdrop did nothing, and the dialog's own
Submit was locked out too. Ask the uploader whether bytes are actually on
the wire instead.

The confirmation panel swapped Cancel upload for Done, and rendered
neither for the second or so in between. That window is the only thing in
the dialog anyone can focus, so a keyboard user lost their place in the
middle of the one moment the panel is asking them to look at. One button
throughout, changing its label.

Done is therefore on screen from the moment the bytes land, which can be
before the uploader reports completion. Leaving in that window closed the
dialog without ever folding the response into the task. The status change
now runs at most once and either path can be the one to run it, and
leaving early hands the celebration to the dashboard rather than spending
it on a dialog that is already closing.

Also drops color="warn" from Cancel. It is a dismissal, not a destructive
action, and nothing rendered it differently anyway.
Seven templates ask a textarea to autosize and set a minimum of three
rows. TextFieldModule was never imported, so cdkTextareaAutosize has
always been an inert attribute and every one of them rendered at the
browser default of two rows. The extension and feedback-review dialogs
are the two a student meets.

The outlined-button treatment mixes its outline from --ot-color-primary
and --ot-color-border. Both are dark in the dark theme, so the outline
landed on #385A8D: 1.60:1 against a card or dialog, where WCAG 1.4.11
asks 3:1 of a control edge, and 2.19:1 at its best anywhere in the app.
--ot-color-link and --ot-color-control-border are the lighter pair the
dark theme already defines for this, and give 3.66:1. The icon had the
same problem against its own hover wash at 2.21:1. Light mode is
unchanged to the byte, because there link equals primary and
control-border equals border.

The treatment also painted every icon in primary with no guard, so
Delete portfolio had a blue trash icon. Warn now gets the same shape of
treatment in its own colour, matching the filled rule that already
excludes it. The label stays in the text colour: error over an
error-tinted fill tops out near 4.3:1, and no mix of error and text
clears 4.5:1 in both themes without going pink. The outline and the icon
carry the warning, and only have to reach 3:1.

Deny on an extension request wanted to be red and said so with a local
border and label, but never set color="warn", so it drew a red outline
over the primary-tinted fill every other outlined button gets. It says
warn now and the global rules supply the whole set. Four dismissal
buttons that claimed warn and are not destructive give it up.
The shimmer was one registered custom property, inherits:true, animated on
:root behind a :has(). Two costs came with that. The property inherits, so
its computed value changed on every element in the document sixty times a
second for as long as any skeleton was on screen, and only a handful of
::before pseudo-elements ever read it. The :has() then put a
whole-document invalidation on the root as well, re-evaluated on DOM
mutation.

Neither bought anything. The library's own shimmer is a fixed 200px
pseudo-element travelling 100vw inside each placeholder's own clip box on
ease-in-out, which is why a narrow placeholder flashed for a sliver of the
cycle and a wide one lit up for most of it. What fixes that is sizing the
sheen to the placeholder and moving it by a percentage of itself, which
the rule already did. The shared clock only lined up start times, and
those already line up because a loading state mounts its skeletons in one
frame.

So the sweep is a plain transform animation on the pseudo-element. Same
look, no document-wide recalc, and the reduced-motion branch loses a rule
it no longer needs.
…assed

hasDateRange goes false when the deadline is behind us, and isDateInRange
answered that by returning true outright. That did not relax the range, it
switched the check off. The field has no form control, so Material's own
min and max validators never ran either, and the calendar being empty only
stops the calendar: the date can be typed.

So a student past the deadline could type 1 Jan 2030 and submit it. Proved
it against the component, which sent a 173 week extension request.

With no range left, the only date the request can carry is the earliest
one, so that is what the check now asks for. The field is disabled in that
state as well, which carries the datepicker and its toggle with it, so
there is nothing to type into in the first place. submitApplication checks
what the button checks rather than only the reason, so the date is no
longer guarded by a disabled attribute alone.

The error line also read "Pick a date from ." there, because the range it
names is empty once there is no range. It says what is actually wrong now.
Follow-up to the previous commit, from reviewing it.

The date guard was enforced by a disabled field and a validity getter, so
the value could still be written and every getter downstream had to keep
catching it. addEvent now refuses the write outright once there is no
range. extensionDuration can then only ever be derived from a date that
was allowed, which also removes a mismatch between the guard comparing
calendar days and the duration counting whole days.

dateErrorText branched on hasDateRange while dateRangeText empties on a
different condition. They disagree for a task with no deadline, where
hasDateRange is true and the range text is empty, so the error read "Pick
a date from ." there and the field rendered no hint at all. Both now ask
about the text they are going to use.

The two tests are rewritten around what actually has to hold: the typed
date never reaches the request, and no message names a range that resolved
to nothing. The guard keeps its own test, since it is what would stop a
far-future date if the field is ever re-enabled.

Also corrects the skeleton comment. It said the shared clock only lined up
start times "which already line up": true within one loading state, not in
general. The better argument is that a common phase was never visible, a
percentage transform ties speed to width, so a 600px bar crossed at about
750px/s beside a 30px circle at about 37px/s. In phase, never in step.
…utliving it

canClose is wired as the dialog's closePredicate, and Material runs a
closePredicate for programmatic closes as well as Escape and the backdrop.
So it decides whether the dialog may close itself, which the previous
commit did not account for.

The effect was a student submitting "I need help", "New Evidence" or a
test submission seeing the upload succeed and then being asked by the
browser whether to discard the files. Answering Cancel, the natural answer
to a prompt you believe is wrong, vetoed the close and kept them in the
dialog. Once the server has the submission there is nothing to discard, so
that is now the first thing the predicate says.

The uploader reports completion on an 800ms timer it never cancelled and
never had an ngOnDestroy to cancel it in. Closing the dialog inside that
window left the timer to call back into a destroyed component, which
applied the submission and claimed its celebration, so the dashboard
suppressed the fallback and the student was told nothing at all. The timer
is cancelled on destroy, and the dialog records the submission itself when
it goes, leaving the celebration unclaimed for the dashboard.

Its subscription to ExternalName, a BehaviorSubject on a root service that
never completes, is unsubscribed in the same place. Without it every dialog
that ever held an uploader stayed in memory for the session.

Two more from the same review. The Submit button stayed locked after a
failed upload because only the success path cleared it, so the uploader's
own Try again was the only way back; the modal now listens to onFailure.
And the shared in-flight panel said "Uploading your work" to a tutor
importing an enrolment CSV, so the wording is an input.
…ndle

startResize keeps a drag's teardown in one field and overwrote it without
looking. A second pointerdown before the first pointerup therefore added a
second pointermove/pointerup/pointercancel trio to the document and
stranded the first one: the pointerup that followed removed only the newer
set, and ngOnDestroy could only ever reach the latest closure either.

The abandoned move handler keeps the dead drag's startX and startWidth, so
the panel goes on resizing from a pointer with no button held, for the rest
of the session, writing the hijacked width to storage on every later click.
The button check above it does not help: touch reports button 0 for every
finger, so a two-finger grab on the divider is enough.

stopDragging is idempotent, so calling it first is the whole fix. The test
fails without it.
…ble name

The "Summary email" select had no accessible name at all. It was paired with a
<label for>, but Material renders the select as the combobox element itself, and
a label can only name a form-associated element, so the association did nothing
and the name fell back to the div holding the current value. A screen reader
announced "Weekly, combobox" with no hint of what it controlled. The label is now
a span with an id and the select points at it, which is how the submission dialog
already names its own select. The seven checkboxes in that form were checked too
and are correctly named by Material's own label element.

The task status card announced every option twice, because status-icon carries
role="img" with an aria-label repeating the text beside it. The icon is
decorative at both call sites, so it is hidden from the tree. The third
status-icon in that template sits behind `triggers?.length < 0`, which can never
be true, so it was left alone rather than decorating dead code.

The per-unit filter checkbox had no label text and no aria-label. It toggles the
unit's Hide Completed filter, so it takes that name, and the menu item around it
takes the same name explicitly, otherwise naming the box alone would have made
the item announce the string twice.

Still open in that menu: role="menuitem" carries no checked state, so the filter
announces the same whether it is on or off. Fixing that means moving to
menuitemcheckbox and hiding the visual box, which is a redesign, not a label.
…emes

Every ratio below was computed from the token values with the WCAG relative
luminance formula. Thresholds are 4.5:1 for normal text and 3:1 for large text
and for meaningful non-text, which covers control edges and focus rings.

The phone bottom nav marked its active tab at 2.18:1 in dark. Label and 3px
underline both move to the link token, which is byte-identical in light and one
step lighter in dark: 4.55:1.

Only the resting half of the Material form field outline was ever pinned, so a
focused field fell back to M2's primary and drew DIMMER than at rest in dark,
2.40:1 against a 3.08:1 resting edge. The focus token now points at the same
colour the global focus ring uses, so a keyboard user sees one colour app wide,
and Material already draws focus at 2px against the resting 1px. The profile
form restated the same declaration locally, worse in dark and identical in
light, so it is deleted and inherits the global rule.

Inside a solid card nothing can sit lighter than the on-colour, so every
translucent white there was spending contrast rather than buying it. Muted text
at 82% measured 3.64:1 and now uses the on-colour outright, with size, weight
and case carrying the step down. The chips had the same problem in reverse:
their glass fill lifted the ground under their own white label to 3.18:1, and
any fill at all fails, so they are outline chips now and the label reads against
the card's own fill.

The control edge was the one token that scope forgot, so an outlined button in a
solid card mixed a scope-local accent with the global slate border: 1.95:1. It
is re-pointed alongside the border token, which fixes every control in the scope
rather than just buttons.

The burndown chart's hidden-series legend sat at 2.31:1. It is still an
interactive control, so the text moves up one token to 5.34:1 and the swatch
empties to a ring, which carries "hidden" without leaning on contrast.

Two residuals, both unreachable in current markup and both recorded here:
a control on a solid card's inner surface would read 2.74:1, and a warn-coloured
button there 1.89:1, because the error token is not re-pointed by that scope.
narrowTaskInbox measured the panel with getBoundingClientRect inside a getter
bound to a child input. The view query resolves after the template pass, so the
first check returned false without measuring anything and the verification pass
measured and returned true, which is NG0100 on first render. Past that it kept
forcing a synchronous reflow once per change detection cycle per binding, and
the child's own rendering could change the width it read. It is a plain field
now, written from a ResizeObserver on the measured element and torn down on
destroy. The view query became a setter because the ref moves between the
desktop panel and the phone section when the breakpoint flips.

Reuse was checked first and is not available: notifyResize only dispatches a
window resize event and is not called from updateAutoRails, so the case that
actually narrows this panel, the layout railing it for space, emits nothing.

The panel emitted widthChange from ngOnInit while restoring a remembered width.
That runs inside the parent's check, after the parent has already written and
stored the input, so a two-way binding moved parent state that had just been
verified. The clamp stays synchronous because the restored width has to be on
the host before the first paint; only the emit defers to a microtask, which is
the idiom the layout already uses for settling rails after registration. It
bails if the panel was destroyed before the microtask ran.

There is exactly one widthChange consumer and it feeds two more levels of parent
state, so the old emit moved a chain of three mid-check.

Not changed: this path still does not call notifyResize, so nothing tells a PDF
viewer to re-measure when a remembered width is restored. That was true before
and fixing it is a behaviour change, not a fix to this defect.
The component already cleared its confirmation timer on destroy, but that clear
only ever gets one chance and the save response lands after it. A response
arriving once the view has gone confirmed the save, which armed a fresh timer
against a component nothing will destroy again, and wrote to its fields on the
way past.

The handlers check a destroyed flag instead. Unsubscribing would have been the
shorter fix and is the commoner idiom here, but the entity service's update is a
cold unicast put, so dropping the subscription aborts the request: an admin who
saves and immediately closes the dialog would lose the save with no sign of it.
Letting the request finish and refusing to touch the view costs nothing.

The error alert still goes up after destroy, because a save that failed is worth
saying wherever the user ended up and the message names the form. Only the
in-form state is skipped, since there is no form left to show it in.

Left alone deliberately: togglePushNotifications never stores its subscription.
Cancelling it mid-chain can leave the browser and the server disagreeing about
whether this device is subscribed, and the worst a late handler does is show a
snackbar on the wrong page. That is the better trade.
…hey claimed

peer-progress-indicator.fixtures.spec.ts asserted only on constants declared in
the same file, and its fixture file had no importer anywhere in the tree but that
spec. Several of the sixteen tests were named as privacy coverage, which is worse
than no coverage because it reads as a guarantee that was never there. The
behaviour is genuinely covered against real code elsewhere: the service spec
flushes a response and asserts the whole mapped object, so a leaked field fails
there, and an it.each pushes NaN, infinite, negative and out-of-range
percentages through the real mapper. Both files are deleted.

The test inside describe('waiting label') was a byte-identical copy of a
previousTask test and never touched the label. The label's warning branch was
uncovered, so the test now covers it.

unit.reviewed.spec.ts carried a verbatim copy of unit.spec.ts alongside its own
distinct describe. The copy is gone. All eight *.reviewed.spec.ts files arrived
in one merge-reconciliation commit, so the suffix is a renaming artefact rather
than a convention, which is why the body was duplicated wholesale.

The not.toThrow in the tutorials spec wrapped three inputs that were already
asserted three lines above, so it could not fail independently. It now asserts
the seconds-bearing time format the API actually sends, which nothing covered.
@Clupai8o0

Copy link
Copy Markdown
Author

Closing: this work is now on integration/t2-2026 (web 8fcf4135a), together with #224, the rest of the UI stack and feature/ui-polish. PR intake has closed for the trimester, so the integration branch is the base for the demo and for the upstream PRs to thoth-tech. The full web suite (2235 tests), lint and build pass on it.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant