Textfield Migration - #6724
Draft
miwha-adobe wants to merge 7 commits into
Draft
Textfield Migration#6724miwha-adobe wants to merge 7 commits into
miwha-adobe wants to merge 7 commits into
Conversation
* chore(text-field): migration plan * docs(text-field): update migration documentation * docs(text-field): created decision log and addressed feedback * docs(text-field): update resolved/deferred items in the migration plan * docs(text-field): reference shared render templates
* feat(text-field): migrate API and add delegatesFocus * docs(textfield): update comment to accurately represent the future work * fix(textfield): add story and additional API details * docs(textfield): comment update * fix(textfield): is defined aria-label * docs(textfield): storyook accessible-label
🦋 Changeset detectedLatest commit: f1fa92d The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
miwha-adobe
added this pull request to stack #6725
September 9, 2026 16:47
… into miwha/swc-2323-text-field-migration
miwha-adobe
removed this pull request from stack #6725
September 9, 2026 17:06
Contributor
📚 Branch Preview Links🔍 Gen1 Visual Regression Test ResultsWhen a visual regression test fails (or has previously failed while working on this branch), its results can be found in the following URLs:
Deployed to Azure Blob Storage: If the changes are expected, update the |
Collaborator
Coverage Report for CI Build 34624329513Warning Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes. Warning No base build found for commit Coverage: 96.26%Details
Uncovered Changes
Coverage RegressionsRequires a base build to compare against. How to fix this → Coverage Stats
💛 - Coveralls |
…icipation (#6699) * feat(field-association-controller): share ElementInternals plumbing for form fields Extract the ElementInternals boilerplate every form-associated field (textfield, checkbox, combobox, radio) shares into a reusable FieldAssociationController: form value with null-exclusion, the defaultValue reset target, the form/fieldset disabled cascade, and the validity pass-throughs. - Plain class taking (internals, options); the host keeps formAssociated, attachInternals, and setValidity, which cannot be delegated. - Three throwaway harnesses (text, radio, combobox) plus an interactive bench exercise the controller in isolation, independent of any swc-* component. - Behavioral tests cover form value inclusion, null exclusion, defaultValue reset, the disabled cascade (onDisabledChange fires once per change), and the validity pass-throughs. - Per-controller MDX docs with a hand-authored API table. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(text-field): adopt FieldAssociationController for form participation Wire swc-text-field into native forms via FieldAssociationController: form value with disabled-exclusion, defaultValue reset, the form/fieldset disabled cascade, and the validity pass-throughs. formAssociated, attachInternals, and the browser form callbacks stay on the element and delegate to the controller. Add a form-participation smoke test. Document adopting the controller in the washing machine workflow (Phase 3) and the forms strategy RFC, recording that setValidity stays on the host with validity population deferred to the labelling and render work. Trim the controller's JSDoc (drop the example that duplicates its docs page and the getter docs that restate name/type) and cover the reportValidity pass-through in tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor(field-association-controller): align demo harnesses and tighten docs - Align the demo harnesses' form-value push (now in update(), like TextField.base) and their comments; keep reset-target capture in firstUpdated (house convention). - Tighten JSDoc/comments across the controller, base, stories, and harnesses; drop migration phase refs (keep SWC ticket refs); reword "throwaway". - Fix a stories argTypes type error by annotating with Meta['argTypes']. - Correct the mdx import specifier to match the package export map. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(text-field): refine migration plan; revert forms-strategy RFC edit - Correct the migration-plan bullet: static formAssociated and attachInternals stay on the element (a plain controller cannot own them); the host restores the controller's defaultValue on formResetCallback() and delegates formDisabledCallback(); centralizing into a mixin/base is deferred to a second consumer, per LinearProgressMixin. - Record the native validation-bubble vs inline-error conflict under Q19. - Revert the forms-strategy RFC change (out of scope for this branch). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(text-field): update comment on disabled cascade * docs(field-association-controller): address demo and docs review feedback - Suppress native bubble: `novalidate` on demo forms, silent `checkValidity()` in the bench, documented in the mdx. - Make readouts submit-driven instead of mirroring live keystrokes. - Fix radio group: second same-name radio, mutual exclusivity, visible `<label>` per option. - Cap demo width and wrap long readout lines. - Remove buggy "Toggle Username disabled" button; use `<fieldset disabled>` for the cascade story and reuse FormValue for Accessibility. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(text-field): address review feedback on form participation * fix(text-field): address review feedback on events, imports, and form value * fix(text-field): resolve FieldAssociationController via subdir subpath * fix(core): restore drag-and-drop-controller export dropped in merge * style(field-association-controller): fix demo-hosts formatting --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* feat(core): add LabellingMixin and refactor text-field to use it
Adds LabellingMixin (visible-label rendering plus precedence-ordered
accessible-name wiring: accessible-labelledby > accessible-label > slotted
label) and its renderFieldLabel directive, replacing the never-built
LabellingController's labelling half. Refactors swc-text-field to consume it
as the proving-ground component.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(core): fixes accessible label conflict
* fix(core): resolves label ID SSR issues
* fix(core): makes accessible label optional
* chore: added changeset
* docs(textfield): update docs
* fix(core): more duplicate label warnings
* fix(core): more duplicate label warnings
* test(core): addressing failing axe tests
* refactor(core): let the browser own accessible-name precedence
Wire each accessible-name source (accessible-labelledby, accessible-label,
slotted <label>) independently and let the browser's accessible-name
computation choose the winner, instead of adjudicating precedence in JS.
Why (from review):
- A visible label paired with a programmatic name is the recommended
WCAG 2.5.3 (Label in Name) pattern, not a conflict. The old code demoted
the visible label to a non-clickable <span> and warned on it. (Rajdeepc, 5t3ph)
- The accessible-name computation already ranks labelledby > aria-label >
<label> deterministically, so the JS precedence logic was redundant. (5t3ph)
Changes:
- render-label always renders a real <label for> when slotted label content
exists; the <span> fallback is gone (forId is now non-null).
- _syncLabelling wires each source independently; else-branches only clear
stale values.
- Conflict warning narrowed to fire only when BOTH accessible-labelledby and
accessible-label are set; a visible label with either no longer warns.
- Deleted unused precedence machinery (_presentNameSources, _activeNameSource,
_describeNameSource).
- Trimmed the class doc to match; updated NameSourcePrecedence + LabelConflict
tests to the new contract.
Addresses the <span>-removal, conflict-warning-scope, _presentNameSources, and
class-doc-verbosity review threads.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs(core): trim verbose comments in LabellingMixin
Cut the docsHref and _resolvedLabelledbyElements comments to essentials;
doc-critical detail lives in the MDX page.
Addresses review feedback that inline comments were too verbose. (5t3ph)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs(core): align labelling docs with browser-owned precedence
Update the labelling mixin and text-field docs to match the accessible-name
refactor: each source is wired independently and the browser's accessible-name
computation picks the winner, the visible label always renders as a real
<label for>, and the conflict warning only fires for accessible-label plus
accessible-labelledby.
- Fix stale "exactly one source is wired", <span> demotion, and "warns on any
two sources" descriptions in both MDX files.
- Add the W3C accessible-name computation reference. (5t3ph)
- Trim the text-field axe-core note to drop implementation-specific detail so
it reads as public-facing docs. (5t3ph)
- Update the ConflictingLabelSources demo to show only genuine conflicts; a
visible label plus a programmatic name no longer warns.
- Replace em dashes per the docs formatting rules.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* style(text-field): use token() for the field gap spacing
Swap the raw `var(--swc-spacing-75, 4px)` gap for `token("spacing-75")`,
matching the design-token convention used across 2nd-gen CSS.
Addresses review feedback. (5t3ph)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(core): guard accessible-labelledby resolution against a detached host
_resolvedLabelledbyElements cast getRootNode() to Document | ShadowRoot and
called getElementById on it. On a host that was connected and then removed,
getRootNode() returns the host element itself, which has no getElementById, so
a reactive update flushed after disconnect threw "root.getElementById is not a
function".
Use optional chaining (root.getElementById?.(id) ?? null), matching the guard
in utils/resolve-trigger.ts, and add a regression test that mutates a reactive
property on a removed host.
Note: help-text-mixin has the identical getter and needs the same fix on its
branch (#6720).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* refactor(text-field): query the field input by id
Match the `<label for>` target: the label references the input by its
`id`, so `roleElement` now looks it up the same way (`input#input`)
instead of by class. No behavior change.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* test(text-field): cover readonly, aria-invalid, value round-trip, focus, and enum warnings
Adds coverage for text-field behavior that lacked direct tests:
`readonly` reflection, `aria-invalid` on `invalid`, the `@input` value
round-trip that keeps the host `value` in sync with user edits, focus
delegation to the native input, and the dev warnings emitted for invalid
`type` / `label-position` enum values. `disabled` and constraint
validity are already exercised elsewhere (form-participation tests and
the FieldAssociationController tests, respectively).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(core): warn on unresolved accessible-labelledby idref
* docs(core): trim labelling mixin JSDoc and note the Firefox reflection gap
Condense the mixin JSDoc to the implementer contract; precedence and the
usage example already live in the MDX. Document that accessible-labelledby
relies on ariaLabelledByElements (unsupported in Firefox as of v143) and is
a silent no-op there, with no same-root IDREF fallback.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs(core): correct outdated Firefox support claim for accessible-labelledby
`Element.ariaLabelledByElements` shipped unflagged in Firefox 136, Chrome 135,
and Safari 16.4 (MDN Baseline 2025), and SWC targets the last 2 versions of
every browser, so every supported engine has the ARIA element-reflection API.
The prior notes claiming Firefox (as of v143) does not support it were wrong.
- Remove the @todo in LabellingMixin._syncLabelling describing a nonexistent gap
- Remove the mixin MDX "Important" callout warning against accessible-labelledby
(keep the axe-core false-positive note; that limitation is real and unrelated)
- Align the changeset with the shipped behavior and its actual dev-mode warnings
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs(core): dedupe labelling mixin precedence framing
The "What it does" summary re-enumerated the full name-source precedence
list already covered under "Name source precedence," and its highest/lowest
framing read as if the mixin ranks the sources. Collapse the summary to a
one-line "browser decides" statement that links to the detailed section.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs(core): tighten labelling mixin comments
Collapse single-tag `@internal` blocks to one line, trim the mixin's getter and
warning JSDoc to the implementer contract, shorten the render-label directive
JSDoc, and condense the verbose INPUT_ID rationale. No behavior change.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(core): add placeholder/host-aria-label warnings and labelInternals hook
Address review feedback on LabellingMixin:
- Warn when a field is named only by its `placeholder` (unreliable in the
accessible-name computation), via a new overridable `placeholderText` hook
the host exposes; text-field maps it to its `placeholder`.
- Warn when `aria-label`/`aria-labelledby` is set on the host, where it does
not name the field. Skipped for internals-based hosts, which honor host ARIA.
- Add a `labelInternals` hook (defaults to `null`) so a control with no inner
role element (e.g. a future radio group) can wire the resolved name onto the
host's `ElementInternals`; `_syncLabelling` branches to it when present. No
consumer yet — the concrete getter lands with radio group.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: Miwha Bonini <mbonini@adobe.com>
* feat(core): add HelpTextMixin and refactor text-field to use it Adds HelpTextMixin (description/error-text rendering plus accessible description wiring: a slotted description combines with an external accessible-describedby reference into ariaDescribedByElements, and error-text is wired to ariaErrorMessageElements only while invalid) and its renderFieldHelpText directive, replacing the never-built LabellingController's description/error-text half. Refactors swc-text-field to consume it as the proving-ground component. Also adds ariaErrorMessageElements to the project's local ARIAMixin type augmentation. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(core): refactored to avoid aria error message * test(core): fix shadow-boundary assertion in help-text-mixin combined description test * fix(core): guard help-text describedby resolution and warn on unresolved idref Guard `getElementById` against a detached host (getRootNode returns the element itself, which lacks the method) and add a dev-mode warning when `accessible-describedby` references an id that resolves to nothing, mirroring LabellingMixin. Also tightens the mixin JSDoc to the implementer contract (detail lives in the MDX) and adds the missing changeset. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(core): tighten help-text comments and replace em dashes Trim the render-help-text directive JSDoc and the text-field roleElement comment, and replace em dashes in the help-text mixin and text-field MDX per the text-formatting rule. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(text-field): record aria-describedby error-association decision (B9) The help-text mixin associates the error via aria-describedby (folded in while invalid) rather than aria-errormessage, following React Spectrum. Record this as decision B9 in the migration plan and update the accessibility migration analysis, which had recommended setting both. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(core): cover the unresolved accessible-describedby warning Add HelpTextMixin coverage for `_warnUnresolvedDescribedby`: warns and names the id when `accessible-describedby` resolves to nothing, stays silent when it resolves, and names only the unresolved id in a mixed set. Also aligns two docs with the delivered design: the `@slot label` JSDoc and the migration plan now name `LabellingMixin` instead of the never-built `LabellingController`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(core): swap description for error in help-text describedby Following PR review, align HelpTextMixin with React Spectrum's TextField: while `invalid`, the error message replaces the description in both the rendered output and `ariaDescribedByElements`, rather than folding both in together. This keeps `aria-describedby` referencing only the message on screen, so no hidden description text is announced. An external `accessible-describedby` reference is unaffected and stays associated in both states, matching react-aria's useField composition (verified against source). Simplify `_syncHelpText` to mirror the rendered elements instead of re-deriving the invalid state: the render directive is the single authority on which of description/error is shown. Update stories, tests, MDX, the changeset, and the text-field migration plan (decision B9) to describe the swap. Add coverage for the invalid + external combo. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(core): support group host attachment in help-text mixin Add a `describedByInternals` seam mirroring LabellingMixin's `labelInternals` so a group host with no inner role element (e.g. a radio group) can carry the description/error on its own ElementInternals. `_syncHelpText` writes to the internals when present, else the `roleElement`. Cover the path with a demo group host and test. Also address two story review nits: promote the visible-label-from-`label`-slot path in the text-field Overview story, and reword the mixin a11y story's error-text example so it does not imply error messages are assistive-technology only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(core): missed slot in overview * refactor(core): stabilize help-text ref callbacks Hoist the description/error `ref` callbacks out of renderHelpText() into stable private fields so Lit does not detach and reattach the refs on every render (a fresh arrow each render would). Addresses PR review. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor(core): stabilize help-text refs and share docsHref helper Address cdransf's review notes: - Hoist the description/error `ref` callbacks out of renderHelpText() into stable private fields so Lit does not detach and reattach the refs on every render. - Extract the docs-page URL builder, duplicated identically across the help-text, labelling, and linear-progress mixins, into a shared `componentDocsHref` util; each `docsHref` getter now delegates to it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(core): drop WebKit-flaky help-text internals test The group-host ElementInternals test asserted `ariaDescribedByElements` reflection, which WebKit does not support for references into a host's own shadow DOM, so it failed only on WebKit CI. Remove the test and its demo host; the `describedByInternals` seam remains (untested, like LabellingMixin's `labelInternals`). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: Miwha Bonini <mbonini@adobe.com>
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
3 participants
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.
Description
Motivation and context
Related issue(s)
Screenshots (if appropriate)
Author's checklist
Reviewer's checklist
patch,minor, ormajorfeaturesManual review test cases
Descriptive Test Statement
Descriptive Test Statement
Device review
Accessibility testing checklist
Required: Complete each applicable item and document your testing steps (replace the placeholders with your component-specific instructions).
Keyboard (required — document steps below) — What to test for: Focus order is logical; Tab reaches the component and all interactive descendants; Enter/Space activate where appropriate; arrow keys work for tabs, menus, sliders, etc.; no focus traps; Escape dismisses when applicable; focus indicator is visible.
Screen reader (required — document steps below) — What to test for: Role and name are announced correctly; state changes (e.g. expanded, selected) are announced; labels and relationships are clear; no unnecessary or duplicate announcements.