Skip to content

EditableField: model the four length-limit props as one optional object #2034

Description

@steilerDev

[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

  • 1 The four props are replaced by a single optional object whose required members cannot be omitted when the object is present.
  • 2 Omitting the object entirely preserves today's unbounded behaviour exactly — the AC9 regression tests from Report wizard: editable override fields have no length limit #1941 pass unmodified in intent.
  • 3 A test pins that the partial-configuration hole is now unrepresentable, i.e. the previously-possible "limit set, no hints" state cannot be constructed.
  • 4 composes used for the shared counter properties; the redundant maxLength! assertions removed.
  • 5 All call sites updated; tsc --noEmit clean (a prop-surface change across eight call sites is exactly the shape that ts-jest cannot catch).

Notes

Metadata

Metadata

Assignees

No one assigned

    Labels

    tech-debtTechnical debt cleanup work (lint, refactors, etc.)

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions