[orchestrator] From the product-architect's review of #1941 / PR #2033. Non-blocking there; filed rather than churning eight call sites for a hole nothing can currently fall into.
Problem
#1941 added four independent optional props to the shared EditableField:
maxLength?: number;
maxLengthHint?: string;
overMaxLengthHint?: string;
maxLengthReachedAnnouncement?: string;
They are a cohesive group modelled as four unrelated optionals. hasMaxLength alone gates both srOnly spans and both aria-describedby ids, so maxLength={200} with no hint strings compiles clean, passes every test, and produces an aria-describedby pointing at two empty elements — a silently broken accessibility contract with no compiler or test signal.
It is unreachable today because EditableField has exactly one consumer and all eight call sites pass all four props. It becomes reachable the moment a second consumer adopts the feature, which is the whole point of AC7 having put the limit on the shared component.
Proposed fix
One optional object prop:
lengthLimit?: {
max: number;
hint: string;
overHint?: string;
reachedAnnouncement: string;
};
This makes the group compiler-enforced, turns AC9's "absent means unbounded" into a single discriminant rather than four independent checks, and collapses the prop threading at each call site.
Keep the no-useTranslation() convention
The architect was explicit that the component should not start translating its own chrome copy, even though that would reduce the threading. The keys live under budget → sourceReports.editable; owning translation would force them into a shared namespace and make every future consumer inherit the report wizard's phrasing. This is the same injection-only locale contract enforced on reportPdf/* in #2007, for the same reason.
Two smaller items from the same review, folded in
.counter / .counterOverLimit duplicate three properties instead of using composes, which EditableField.module.css already uses elsewhere.
- A redundant non-null assertion (
maxLength!) at EditableField.tsx lines 61 and 63, where hasMaxLength has already narrowed it.
Acceptance Criteria
Notes
[orchestrator] From the
product-architect's review of #1941 / PR #2033. Non-blocking there; filed rather than churning eight call sites for a hole nothing can currently fall into.Problem
#1941 added four independent optional props to the shared
EditableField:They are a cohesive group modelled as four unrelated optionals.
hasMaxLengthalone gates bothsrOnlyspans and botharia-describedbyids, somaxLength={200}with no hint strings compiles clean, passes every test, and produces anaria-describedbypointing at two empty elements — a silently broken accessibility contract with no compiler or test signal.It is unreachable today because
EditableFieldhas exactly one consumer and all eight call sites pass all four props. It becomes reachable the moment a second consumer adopts the feature, which is the whole point of AC7 having put the limit on the shared component.Proposed fix
One optional object prop:
This makes the group compiler-enforced, turns AC9's "absent means unbounded" into a single discriminant rather than four independent checks, and collapses the prop threading at each call site.
Keep the no-
useTranslation()conventionThe architect was explicit that the component should not start translating its own chrome copy, even though that would reduce the threading. The keys live under
budget → sourceReports.editable; owning translation would force them into a shared namespace and make every future consumer inherit the report wizard's phrasing. This is the same injection-only locale contract enforced onreportPdf/*in #2007, for the same reason.Two smaller items from the same review, folded in
.counter/.counterOverLimitduplicate three properties instead of usingcomposes, whichEditableField.module.cssalready uses elsewhere.maxLength!) atEditableField.tsxlines 61 and 63, wherehasMaxLengthhas already narrowed it.Acceptance Criteria
composesused for the shared counter properties; the redundantmaxLength!assertions removed.tsc --noEmitclean (a prop-surface change across eight call sites is exactly the shape thatts-jestcannot catch).Notes
frontend-developer.