Repository navigation
Conversation
Input, textarea, checkbox, select and multi-select get the properties invalid and error. error shows a message under the control (part "error", announced with aria-live="polite") and marks the control as invalid: red border, aria-invalid and aria-describedby. The select triggers take the state from their select and keep a screen reader copy of the message, as aria-describedby can't cross shadow roots. Refs #535 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Add server-agnostic utilities for RFC 9457 problem details: readProblemDetails() and parseProblemDetails() read the standard members, the field errors (errors[] with detail and pointer, as in the RFC's example; plain pointers and parameter accepted) and the extension members; mapFieldErrors() maps the errors to a component's fields by pointer or parameter (closest containing field otherwise) and gives a general error for the rest, for problem details without errors and for other responses. Refs #535 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Add a Field errors page (response format, utilities, example) and the invalid and error properties to the pages of the form controls. Refs #535 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- mapFieldErrors() takes parsed problem details only, instead of telling them from bodies by a hidden registry of parsed objects. - Field keys that are names of object members (e.g. toString) map correctly; locations in the general error have their keys unescaped (RFC 6901). - The checkbox message no longer widens or shifts the box next to its label; the select triggers' copy for aria-describedby is hidden, so screen readers don't read the message twice. - Docs: errors aren't passed down to contained fields; the example handles network errors; FOCA's problem type linked. - More tests for the select controls. Refs #535 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The checkbox message no longer catches clicks meant for content below it, and the docs say that long messages don't wrap. mapFieldErrors() gives a general error for problem details without an errors array instead of throwing, e.g. for untyped callers. Refs #535 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
Reviewer's GuideAPIExpose RFC 9457 problem-details parsing, response reading, pointer normalization, and field-error mapping as public package utilities.
Add the
UIRender accessible inline errors and destructive invalid states for the form controls, including shadow-root-safe descriptions for select triggers and non-shifting checkbox messages.
ConfigurationAdd the design package changeset describing the minor release feature.
DocumentationDocument the response format, utility behavior, and an example of mapping API field errors into a form.
Document the new invalid and error properties and their accessibility behavior on each supported control page.
Add the field-errors page to the design documentation navigation metadata.
TestsTest invalid and error states, accessibility wiring, attribute handling, and select-trigger behavior across the form controls.
Test problem-details parsing and reading, pointer normalization, and field-error mapping across matched, unmatched, and fallback cases.
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Documentation build overview
49 files changed ·
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #576 +/- ##
==========================================
+ Coverage 73.53% 73.60% +0.06%
==========================================
Files 33 45 +12
Lines 8123 10133 +2010
Branches 381 496 +115
==========================================
+ Hits 5973 7458 +1485
- Misses 2141 2665 +524
- Partials 9 10 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
uniqueg
marked this pull request as ready for review
November 13, 2026 09:07
Contributor
Linked issuesChecked at commit 742a508 #535 feat(design): show field errors from the API · 5 of 5 met
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 9 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="packages/ecc-utils-design/src/utils/problem-details.ts" line_range="157" />
<code_context>
+ method: 'POST',
+ headers: { 'Content-Type': 'application/json' },
+ body: JSON.stringify(body),
+ });
+ } catch {
+ this.generalError = 'The registry could not be reached.';
</code_context>
<issue_to_address>
**API response forges inherited extensions**
When an API response contains an object-valued `__proto__` extension and a caller later reads properties from `problem.extensions`, `extensions[key] = body[key]` invokes the `__proto__` setter when a response contains an object-valued `__proto__` extension, changing the prototype of the returned `extensions` object instead of storing that member as data. Downstream callers then see attacker-controlled inherited properties when reading extension values.
Build `extensions` with a null prototype or define extension keys as own data properties so `__proto__` cannot alter its prototype.
</issue_to_address>
### Comment 2
<location path="apps/documentation/pages/docs/design/field-errors.mdx" line_range="126-127" />
<code_context>
+ FIELDS,
+ `The service could not be registered (${response.status}).`,
+ );
+ this.fieldErrors = mapped.fieldErrors;
+ this.generalError = mapped.generalError ?? '';
+ }
+
</code_context>
<issue_to_address>
**Older responses overwrite newer errors**
When two submissions overlap and the older request resolves last, `submit()` applies the older response’s mapped errors after the newer submission completes, so the form shows outdated validation state or errors after a newer successful response.
Ignore a submission’s result when a newer submission has already started.
</issue_to_address>
### Comment 3
<location path="packages/ecc-utils-design/src/utils/problem-details.ts" line_range="118" />
<code_context>
+ return undefined;
+ }
+ }
+ if (plain !== "" && !plain.startsWith("/")) return undefined;
+ return plain;
+}
</code_context>
<issue_to_address>
**Malformed locations match the wrong field**
When an API returns a pointer with an invalid tilde escape that matches a configured field location, `normalizePointer()` checks only that the pointer starts with `/`, so it accepts invalid RFC 6901 escapes; `mapFieldErrors()` can then attach the error to the wrong field instead of treating it as unmatched.
Reject pointers containing any `~` not followed by `0` or `1`.
</issue_to_address>
### Comment 4
<location path="packages/ecc-utils-design/src/utils/problem-details.ts" line_range="152" />
<code_context>
+ */
+export function parseProblemDetails(body: unknown): ProblemDetails | undefined {
+ if (!isObject(body)) return undefined;
+ if (!["type", "title", "status", "detail", "errors"].some((m) => m in body))
+ return undefined;
+ const extensions: Record<string, unknown> = {};
</code_context>
<issue_to_address>
**Valid problems with only instance are discarded**
When a problem-details response contains `instance` but none of the other members checked by the recognition guard, `parseProblemDetails()` omits `instance` from its recognition check and returns `undefined`, so `readProblemDetails()` discards the valid problem and callers lose its occurrence URI and other parsed details.
Include `instance` among the members that qualify a body as problem details.
</issue_to_address>
### Comment 5
<location path="packages/ecc-utils-design/src/components/input/input.ts" line_range="53" />
<code_context>
const checkboxClasses = cn(
- "peer h-4 w-4 shrink-0 rounded-[4px] border border-primary shadow focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-ring disabled:cursor-not-allowed disabled:opacity-50",
+ "peer h-4 w-4 shrink-0 rounded-[4px] border shadow focus-visible:outline-none focus-visible:ring-1 disabled:cursor-not-allowed disabled:opacity-50",
+ isInvalid
+ ? "border-destructive focus-visible:ring-destructive"
+ : "border-primary focus-visible:ring-ring",
</code_context>
<issue_to_address>
**Input focus rings disappear**
When a user keyboard-focuses an input or textarea, keyboard focus on these controls has no visible ring: the new classes set only the ring color, while `focus-visible:outline-none` removes the browser outline and the old `focus-visible:ring-1` width utility was removed. Users lose the prior focus indicator on both controls.
Keep `focus-visible:ring-1` in the base classes for both controls.
Also at `packages/ecc-utils-design/src/components/textarea/textarea.ts:36`.
</issue_to_address>
### Comment 6
<location path="packages/ecc-utils-design/src/utils/problem-details.ts" line_range="257" />
<code_context>
+ .split("/")
+ .map((key) => key.replace(/~1/g, "/").replace(/~0/g, "~"))
+ .join("/");
+ return location ? `${location}: ${error.detail}` : error.detail;
+}
+
</code_context>
<issue_to_address>
**Empty-key locations are omitted**
When an unmatched error points to an object member with an empty-string key, `describeUnmatched()` treats the valid pointer `/` as an absent location, so the general error shows only the detail and omits which field it concerns.
Distinguish an absent location from a valid location that decodes to the empty string.
</issue_to_address>
### Comment 7
<location path="packages/ecc-utils-design/src/form-controls.test.ts" line_range="142" />
<code_context>
+ expect(button().getAttribute("aria-invalid")).to.equal("true");
+ const describedBy = button().getAttribute("aria-describedby")!;
+ expect(
+ trigger.shadowRoot!.getElementById(describedBy)!.textContent?.trim()
+ ).to.equal("Must be one of: a, b.");
+ expect(button().className).to.contain("border-destructive");
</code_context>
<issue_to_address>
**Duplicate error copy stays exposed**
When a select or multi-select displays an error, `shows the error and marks the trigger invalid` checks only the described-by copy's text at `form-controls.test.ts:142`; removing `hidden` from that copy still passes, leaving an extra exposed message that can be announced alongside the live-region error.
Assert that the described-by copy is hidden as well as containing the error text.
</issue_to_address>
### Comment 8
<location path="packages/ecc-utils-design/src/form-controls.test.ts" line_range="81-83" />
<code_context>
+ el.error = "";
+ await el.updateComplete;
+ const control = el.shadowRoot!.querySelector(selector)!;
+ expect(control.hasAttribute("aria-invalid")).to.equal(false);
+ expect(errorElement(el).textContent?.trim()).to.equal("");
+ expect(control.className).to.not.contain("border-destructive");
+ });
+ });
</code_context>
<issue_to_address>
**Stale error description passes**
When an error is cleared after being set, `clears the error` checks the invalid state, message text and border at `form-controls.test.ts:81-83`, and the select test checks only invalid state and border at `:149-150`; both pass if `aria-describedby="error"` remains after clearing, leaving assistive technology pointed at an empty message.
Assert that `aria-describedby` is absent after clearing the error on each control and select trigger.
Also at `packages/ecc-utils-design/src/form-controls.test.ts:149-150`.
</issue_to_address>
### Comment 9
<location path="packages/ecc-utils-design/src/utils/problem-details.test.ts" line_range="153" />
<code_context>
+ expect(problem?.errors).to.have.length(2);
+ expect(
+ await readProblemDetails(response(body, "application/json"))
+ ).to.not.equal(undefined);
+ });
+
</code_context>
<issue_to_address>
**JSON parsing accepts an empty result**
When the `application/json` branch returns a non-`undefined` result without parsing the body, when `readProblemDetails()` returns a non-`undefined` placeholder for an `application/json` body instead of parsing its problem details, `reads problem+json and JSON bodies` passes at `problem-details.test.ts:153`; it never checks the JSON response's parsed errors or other content.
Assert that the parsed `application/json` response contains the expected errors or another known member from `RFC_EXAMPLE`.
</issue_to_address>Sourcery assessment
Approval pending. 9 findings to address first.
Blocking findings: packages/ecc-utils-design/src/utils/problem-details.ts:157, apps/documentation/pages/docs/design/field-errors.mdx:127, packages/ecc-utils-design/src/utils/problem-details.ts:118, packages/ecc-utils-design/src/utils/problem-details.ts:152, packages/ecc-utils-design/src/components/input/input.ts:53, and 4 more
- Extension members and field keys named __proto__ are kept as own data instead of changing prototypes. - Pointers with invalid ~ escapes are rejected (RFC 6901). - Bodies with only instance are recognized as problem details. - The general error names the member with the empty key (/). - Docs example: only the latest submission shows its errors. - Tests check that described-by copies are hidden, that aria-describedby is removed with the error, and the parsed JSON. Refs #535 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Adds a changeset for the design package, following the changeset convention that arrived on dev. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
anuragxxd
requested changes
Nov 14, 2026
anuragxxd
left a comment
anuragxxd
left a comment
Member
There was a problem hiding this comment.
Thanks, the problem-details part looks solid.
- Needed: please add a
./utilssubpath export (and a tsup entry). Right now the utilities are only exported from the root, which registers every component, including Ace. - Suggested: drop
errorfrom the checkbox and keep onlyinvalid. The message doesn't wrap, so it overflows when the checkbox is at the right of a row, as in the TES run-create form.
This branch has not been deployed
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
Generic support in
@elixir-cloud/designfor showing the field errors that an API returns, without validation rules in the components:invalid(Boolean, reflected) anderror(string).errorshows the message under the control (parterror,aria-live="polite") and marks the control as invalid: red border (destructivetoken),aria-invalid="true"andaria-describedbypointing to the message.invalidmarks it as invalid without a message. For select and multi-select, the properties are on the root element; the trigger shows the invalid state and keeps a hidden copy of the message foraria-describedby, which can't refer to another shadow root. The checkbox message is positioned under the box without taking space, so that it doesn't widen or shift the box next to its label.src/utils/problem-details.ts, exported from the package root), server-agnostic, for RFC 9457 (application/problem+json):readProblemDetails(response)/parseProblemDetails(body): standard members (typedefaulting toabout:blank,title,status,detail,instance), field errors in the form of the RFC's example (errors[]withdetailandpointeras URI fragment, e.g.#/organization/url; plain pointers andparameteraccepted too;codekept; each entry as received inraw), extension members (e.g.code) inextensions.mapFieldErrors(problem, fields, fallback?): maps the errors to the component's fields by pointer or parameter (exact match, else the closest containing field, e.g./tags/0to/tags). Errors that match no field, problem details withouterrorsand responses that aren't problem details givegeneralError(the unmatched messages with their locations, elsedetail/title, else the fallback).normalizePointer(pointer).invalid/erroron the pages of the five controls.No client package is changed; components can adopt this in follow-ups (e.g. the Cloud Registry create form). Changeset:
.changeset/design-field-errors.md(@elixir-cloud/design: minor), following the convention that arrived ondevwith #516 (merged into this branch).Closes #535
Verification
npm run lint(repo root; builds all packages and the docs site, then ESLint): 19/19 tasks successful, no errors (warnings only, all pre-existing kinds).web-test-runner,--group default): Chromium 58 passed, Firefox 58 passed (1 skipped). New:src/form-controls.test.ts(states, aria wiring, attribute, clearing, select/multi-select triggers, trigger without select) andsrc/utils/problem-details.test.ts(pointer normalization, parsing incl. wrong member types and non-problem bodies,readProblemDetailscontent types, mapping incl. unknown pointers/parameters, ancestors, responses withouterrors, fallback, RFC 6901 escapes, field keys liketoString).GlobalStyles > code editor theme > draws fold arrows) failed locally on Chromium only because the sandbox blocks the Ace worker download from cdn.jsdelivr.net; unrelated to this change.dev): all checks pass (CI passed, Lint, Test on Node 22 and 24 incl. WebKit, Conflict markers, PR title, CodeQL, Codecov patch/project, Read the Docs, Sourcery review). Sourcery's 9 threads were addressed (8 fixed, 1 answered: the focus ring width is unchanged) and resolved; Sourcery approved.Declined review findings
detailare dropped (not shown at a field): RFC 9457's example and FOCA always senddetail; inventing a message would be guesswork.cn) in the same way.requestUpdate()on their triggers instead of extending the polledselectState: it updates at once and is explained in a comment; the shared state is for open/value.__proto__would lose its message: not a realistic field key.::part(error)for other layouts.field-errors.mdxis formatted with the docs app's Prettier settings (single quotes), while the component pages keep their existing style; reformatting those pages is out of scope.#-pointer errors without location are covered by the general-error test;normalizePointeris kept public and documented, as components need it to compare their own pointers.Open questions
None.
🤖 Generated with Claude Code