Skip to content

[Frontend][A11y][Review] Decision-rail description residuals from PR #2568 review: per-note split for the Apply unavailable note, cleanup stack, prop doc, view-level coverage #2578

Description

@Chris0Jeky

Non-blocking residuals from the two fresh-context reviews of PR #2568 (closes #2461, Review decision-rail lock-reason association), tracked here rather than fix-cycled. File references are at that PR's head b6084319d.

  1. LOW, per-note split. ReviewMain.vue ~350 forwards the description ids to the rail only while busy is true. That fixes the round-1 MEDIUM (enabled Reject / Request edit / Defer no longer announce the evidence-unavailable note), but it is a per-control-state gate, not a per-note one: after waitForCoreBatch returns failed, activeRevisionReviewUnavailable stays true while busy clears, so the note "Review evidence could not be refreshed. No decision was made. Choose the current action again to retry." is on screen and genuinely about Apply, yet the enabled Apply control carries no description. Mitigations today: the note is role="status" aria-live="polite", focus returns to Apply, and the column wrapper keeps the association. A strictly better contract gates the refresh-lock id on busy and keeps the unavailable id on Apply; that needs the two ids separated in PaperReviewView.vue's reviewMainDescriptionIds.
  2. LOW, test hygiene. In ReviewMain.spec.ts ~260-262 and ReviewDecisionRail.spec.ts ~383-385 the cleanups stack is emptied with splice(0) before iterating, so a throwing cleanup (for example wrapper.unmount()) skips and permanently drops the remaining entries, leaking the injected note element into document.body for the rest of the file. Wrap each cleanup in try { } finally { } or iterate with while (cleanups.length) cleanups.pop()!().
  3. LOW, prop doc wording. ReviewDecisionRail.vue ~69-70 says the rail attaches the ids "to all four decision controls, unconditionally"; it drops whitespace-only input and the applyOnly / dismissable branches render one or none of the four.
  4. LOW, coverage. The union scenario (refresh fails, unavailable note persists, busy false, enabled controls undescribed) is proven only at component level with a stand-in id; a view-level case in PaperReviewView.spec.ts would catch a future change that folds activeRevisionReviewUnavailable into busy.

Refs #2461, #1964, PR #2568.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    Priority IVLater maturity/deepening; revisited at horizon planning.frontendPrimary implementation impact in Vue/TypeScript UI and client runtime.tech-debtDebt cleanup or deferred engineering quality work with limited product-surface change.testingTest coverage, harnesses, regression prevention, and verification pipeline work.uxDiscoverability, accessibility, interaction model, and user workflow quality.

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions