os-text-field: add hide-label so a compact field can have a name without a visible label - #792
Open
juanlentino wants to merge 1 commit into
Open
Conversation
`label` drove both the visible label and the accessible name from one value, so a compact field had two states and neither was right: omit it and the inner input gets `aria-label=""` (no accessible name), or set it and a label renders above a toolbar search that has no room for one. `placeholder` does not close the gap -- it is not an accessible name, and it disappears on the first keystroke. `hide-label` splits presentation from naming. The <label> is still rendered, still paired with the input by `for=`, and still the accessible name; it is only taken out of the visual flow, using the same idiom as `.os-constellation__row-note` in openstation-layout.css. Hiding it with `display: none` -- or dropping the element -- would take it out of the accessibility tree too, which is the opposite of the point. Four first-party search fields adopt it: comments, plugins, trash and my-wordpress. After this, no `<os-text-field>` under apps/ is unnamed. Fixes WordPress#789
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.
Fixes #789.
The problem
labeldrove both the visible label and the accessible name from one value, so a compact field had two reachable states and neither was right:labelomitted → the inner<input>getsaria-label="", i.e. no accessible namelabelset → the name is correct, and a label renders above a toolbar search with no room for oneplaceholderdoes not close the gap: it is not an accessible name, and it disappears on the first keystroke.The change
hide-labelseparates presentation from naming. The<label>is still rendered, still paired with the input byfor=, and still the accessible name — it is only taken out of the visual flow, using the same idiom as.os-constellation__row-noteinopenstation-layout.css.Hiding it with
display: none, or dropping the element and relying onaria-labelalone, would have been easier and worse: the first removes it from the accessibility tree, and the second gives up the<label for>association, which is the more robust of the two bindings. Keeping both is the point.Declared as
hideLabelinstatic props, soComponent.observedAttributeskebabs it tohide-label— matchingautoGrow,labelPositionand the other multi-word props in the kit.Call sites
Four first-party search fields adopt it: comments, plugins, trash, my-wordpress. After this, no
<os-text-field>underapps/is unnamed.A correction to the issue: #789 says seven. That number was measured against a checkout that has since moved; on current
trunkit is four. One of the seven I counted was also a false positive —os-number-field's helpsummarystring contains the literal text<os-text-field>, which my scanner read as a call site. It has its ownlabelprop and was never affected. I have not edited the issue body; happy to if you'd prefer the record corrected there.Tests
Three cases in
os-text-field.test.ts:<label for>with the input and names ithide-labelkeeps the label, the pairing and the name, and only adds the modifier classhide-labelwithoutlabelnames nothing, because it is a presentation switch and not a source of namesOne honest caveat on verification. I could not run your vitest suite locally:
devEnginespins Node to>=24 <25and this machine is on v26, sonpmrefuses before install. I validated what I could without the harness — the prop is declared, read as a boolean the same waydisabledis, the modifier is applied,aria-labelis untouched, the style rule exists and is notdisplay: none, andkebab( 'hideLabel' ) === 'hide-label'against your ownkebab()insrc/ui/core/component.ts. The vitest cases themselves are unexercised, so please let CI be the judge of those.Not included
os-textareaandos-number-fieldhave the same coupling. I left them alone rather than widen an accessibility fix into a kit-wide API change without your read on the attribute name first — ifhide-labelis the shape you want, extending it is mechanical.