Skip to content

fix(bulma-ui): name Autocomplete's suggestion list from a surrounding Field's label - #1008

Merged
allxsmith merged 26 commits into
mainfrom
fix/998-autocomplete-list-name
Oct 10, 2026
Merged

allxsmith merged 26 commits into
mainfrom
fix/998-autocomplete-list-name

Conversation

@allxsmith

@allxsmith allxsmith commented Oct 9, 2026 •

Copy link
Copy Markdown
Owner

Inside a labeled Field, Autocomplete's input took the Field's label as its name, but the list that opens under it didn't. #979 named the listbox from the label the component's own label prop renders and fell back to "Suggestions" everywhere else. So <Field label="Country"><Autocomplete … /></Field> announced the open list as "Suggestions" instead of "Country", and two Autocompletes in one form had lists with the same name.

Now the list takes its name from whichever label names the input. For the component's own label that's the label it renders, same as before. Inside a Field it's the Field's label. The Field already tells the control inside it which id its label points at; it now passes the label's own id alongside, so the list reads both from the same place and only uses the id while that label actually names the input. The Field's markup doesn't change. A label without an id never gets pointed at, so the list never names an id that isn't on the page. aria-controls still points at the list only while it's on screen, and the closed list isn't in the DOM at all, so nothing dangles there either.

Both paths are gated on the label actually naming the input, using the labelled flag useAutoLabelId already returns. That's what keeps "Suggestions" for an Autocomplete nothing labels, and it changes one thing from #979. An Autocomplete whose own label is opted out with labelProps={{ htmlFor: undefined }}, or pointed at another control, used to name its list from that label anyway. Now its list gets "Suggestions", the same way its input gets no name. Likewise an Autocomplete with its own id inside a labeled Field, where the Field's for misses it. The rule is the same in every case: the list says what the input says.

On caller precedence, there isn't a caller name for the list to defer to. An aria-label or aria-labelledby passed to Autocomplete lands on the wrapper div through the rest props, not on the input or the list, so it names neither. That was already the case before this change, and routing those to the input feels like its own issue.

That covers the horizontal layout the Autocomplete page teaches too. A Field label wired by hand with labelProps={{ htmlFor, id }} names the input through its for and the list through its id, and the page's horizontal examples now give their labels an id. A hand-wired label with no id still names only the input, and the list keeps "Suggestions" there: giving every hand-wired label a generated id would change Field's markup for every caller, which is a bigger change than this one.

The tests run the component's own label (plain, wired by hand, and with horizontal), a Field's label (plain, over a dropped own label, through a Control, and through a horizontal Field.Body), and hand-wired Field labels with an id (plain, the docs' horizontal inner Field, and with a Control) through the same cases: the list's name, that the id it points at belongs to the rendered <label>, a labelProps.id of your own, two lists told apart, and no id reference left pointing at nothing as the list opens, highlights an option, and closes. The fallback cases cover no label, both opt-outs, a mismatched id, grouped, hasAddons, a nested unlabeled Field, and a hand-wired label without an id.

Docs: the label and labelProps TSDoc say how the list gets its name, and the Autocomplete page's Accessibility section and its Field wrapper section are updated to match. The MCP index is regenerated. No skill mentions the list's name, so skills/ is unchanged.

#1006 (range Slider thumb names) landed first and asked useAutoLabelId for the id of the label that names a control. Merging it in, both now read that id from the same place: the Field passes its label's id down beside the for target, and Autocomplete uses the hook's needsLabelId like Slider does instead of keeping its own copy. One thing changes for Slider as a result. A range Slider inside a hand-wired Field label that has an id (labelProps={{ htmlFor, id }}) now names its thumbs from that label, "Price Minimum value" and "Price Maximum value", where #1006 left the defaults. Without an id they keep the defaults, and the Slider and Field pages say so.

pnpm all passes locally.

Fixes #998

… Field's label

Inside a labelled Field the combobox took the Field's label as its name,
but the open list still fell back to "Suggestions", so two Autocompletes
in one form announced lists with the same name. The list now takes its
name from whichever label names the combobox, its own or the Field's,
pointing aria-labelledby at the Field's label through the id the Field
gives it while it wires that label itself.

"Suggestions" is left for a list whose combobox no label names, which now
includes an own label opted out of the association, and for a Field label
wired by hand, which carries no id to point at.

The tests run the component's own label and a Field's label through the
same cases, including that nothing points at a missing id as the list
opens and closes.

Refs #998
The Accessibility section said the list fell back to "Suggestions" inside
any outer Field. It now takes the name of whichever label names the
input, the component's own or a Field's, and the fallback is left for an
input no label names. The horizontal examples wire their label by hand,
which gives the list no id to point at, so the page says the list keeps
its fallback name there.

Refs #998
The Autocomplete API page's props table and its MCP index entry, from the
label and labelProps TSDoc and the page's Accessibility section.

Refs #998
@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 53 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository: allxsmith/bestax/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: c9a0f2d1-afd2-4cb7-a6c1-4db1a9164136

📥 Commits

Reviewing files that changed from the base of the PR and between c9438bc and 30b7dd2.


📒 Files selected for processing (20)
  • bestax-mcp/data/components/Autocomplete.json
  • bestax-mcp/data/components/Field.json
  • bestax-mcp/data/components/Slider.json
  • bestax-mcp/data/skills.json
  • bulma-ui/src/form/Autocomplete.stories.tsx
  • bulma-ui/src/form/Autocomplete.tsx
  • bulma-ui/src/form/Field.tsx
  • bulma-ui/src/form/FormContext.tsx
  • bulma-ui/src/form/Slider.stories.tsx
  • bulma-ui/src/form/Slider.tsx
  • bulma-ui/src/form/__tests__/Autocomplete.test.tsx
  • bulma-ui/src/form/__tests__/Field.test.tsx
  • bulma-ui/src/form/__tests__/Slider.test.tsx
  • bulma-ui/src/form/useAutoLabelId.ts
  • docs/docs/api/form/autocomplete.md
  • docs/docs/api/form/field.md
  • docs/docs/api/form/slider.md
  • skills/bestax-form/SKILL.md
  • skills/bestax-form/references/api.md
  • skills/bestax-form/references/patterns.md

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Preview Deployment

Preview URL: https://b89b7e97.bestax.pages.dev

Comment thread bulma-ui/src/form/Autocomplete.tsx Outdated
Comment thread bulma-ui/src/form/__tests__/Autocomplete.test.tsx
Comment thread bulma-ui/src/form/Autocomplete.tsx Outdated

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deep review — 0 blocking · 3 advisory

# Severity Area Finding Location
1 🔵 Advisory Accessibility #998's "two lists, same name" still happens in the hand-wired horizontal shape that every live example on the docs page uses; the follow-up it is deferred to has no issue link bulma-ui/src/form/Autocomplete.tsx:467-471
2 🔵 Advisory Coverage labelSources has no horizontal shape, so the one horizontal layout the fix does reach (Field.Body-direct, the WithFieldWrapper story) and Field's horizontal label branch go unasserted bulma-ui/src/form/__tests__/Autocomplete.test.tsx:693
3 🔵 Advisory API The label TSDoc's enumerated condition omits that a user id takes the Field's label out of play — the PR's own test at line 826 covers that shape bulma-ui/src/form/Autocomplete.tsx:45

Overall: The change is sound, and the mechanism is the right one. Deriving the list's name from labelled — the flag useAutoLabelId already computes as fieldLabelFor === controlId — means the list can never claim a name the input does not have, and reading the target through useFieldLabelElementId() means it can never point at an id Field did not render, since Field.tsx gates that provider on the same targetId that gates the label's own id. I traced every nesting I could construct — labelled Field, labelled Field through a Control, unlabelled inner Field inside a labelled one, grouped/hasAddons, both htmlFor opt-outs, a mismatched id — and found no path where aria-labelledby ends up stale, foreign or dangling: an inner Field always overwrites the element-id context while FieldLabelForProvider passes through, and that asymmetry is exactly what makes the hand-wired case degrade to "Suggestions" rather than to a broken reference. Empirically: 123/123 in Autocomplete.test.tsx, 2836/2836 across src/form/, Autocomplete.tsx at 99.34% statements and 99.31% branches, and gen:api-docs:check, gen:mcp:check, typecheck:tests, format:check and check:conformance all clean — conformance's only failure is version-regression, the shallow-clone artefact of this checkout rather than a violation. The riskiest part is not the code but the scope line, so finding 1 is what a human should look at first: the remaining gap sits in the pattern the docs page actually demonstrates. One caveat on finding 3 — its suggestion edits a TSDoc, so applying it needs pnpm gen:api-docs and pnpm gen:mcp in the same commit.

Residual risk:

  • A hand-wired Field label, the documented horizontal pattern, still falls back. Not refuted: confirmed, and posted as finding 1. Field hands out no label id while labelProps.htmlFor is present, so listLabelId is undefined there, and the new test at line 865 pins that as expected.
  • A user id inside a labelled Field loses both names. Confirmed by the PR's own test at line 826: controlId = id ?? adopted, so the Field's label for keeps pointing at its generated id and matches nothing — the input is unnamed and the label idref dangles. Pre-existing #495/#939 behaviour, outside this diff's lines, and the new danglingIdRefs sweep runs only over the five properly labelled sources, so it would not catch it.
  • Two Autocompletes in one plain labelled Field both adopt the same generated id, so both lists get the same name and the inputs get duplicate ids. Pre-existing; Field's own TSDoc says to give each an id, which then lands in the bullet above.
  • Taginput's dropdown at Taginput.tsx:694 carries role="listbox" with no accessible name at all, and Calendar.tsx:1155 likewise. Outside #998's scope, but the same failure class, and nothing guards either.
  • Refuted — a stale or foreign label id reaching aria-labelledby. Every Field writes FieldLabelElementIdProvider, so an inner Field always shadows an outer one, and labelled comes from the same Field's labelFor. The only divergence — an unlabelled inner Field passing labelFor through while resetting the element id — resolves to undefined, which is the fallback, not a bad reference.
  • Refuted — a reference left pointing at nothing as the list opens, highlights and closes. The new danglingIdRefs sweep asserts an empty list at all three points across all five label sources, and I ran it.
  • Refuted — staleness of the generated surfaces. gen:api-docs:check reports 0 updated and gen:mcp:check exits clean, and skills/ genuinely says nothing about the list's name: grepping for "Suggestions" hits only bestax-mcp/data.

🏄 Clean little swell, this one — paddles out, names the suggestion list off whatever label already names the box, and never drops in on an id that is not there. The horizontal break past the point is still fogged in with "Suggestions", but that is charted on the map, not hidden. Good to go.

…'s id

A horizontal form names an Autocomplete in an inner Field by wiring the
outer label by hand with labelProps.htmlFor, and there the suggestion
list still fell back to "Suggestions", so two such lists shared a name.
The Field now passes the id its label renders with alongside the label's
htmlFor target, through unlabeled Fields too, and the list points at that
id while the label names its input. A hand-wired label carries an id only
when the caller gives one in labelProps, so Field's markup is unchanged
and a label with no id still leaves the fallback name.

The label TSDoc now states when a Field's label names the input and when
it names the list, including that an Autocomplete's own id keeps it from
adopting the Field's.
…ed labels

The label-source matrix covered only plain layouts. It now also runs its
name, label identity, labelProps id, two-lists and no-dangling-id checks
over an own label in a horizontal layout, a horizontal Field's label
through its Field.Body, and a Field label wired by hand with an id, plain
and in the inner Field (with and without a Control) the docs' horizontal
examples use. The fallback case for a hand-wired label with no id now
covers the horizontal shape too.
… list

The With Field Wrapper and With Field and Control Wrappers examples wire
the outer label by hand, which named the input but left the open list
called "Suggestions". Each label now carries an id in labelProps, which
the list takes its name from. The Accessibility bullets say when a
Field's label names the input and when it names the list.
@allxsmith

Copy link
Copy Markdown
Owner Author

deep-review: verify

Every open thread has a reply: fixed, with the commit, or refuted with a reason. This settles them on bf9d2f5.

@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Preview Deployment

Preview URL: https://e31c01f0.bestax.pages.dev

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deep review (verify) — 3 resolved · 0 open

# Severity Area Thread Resolution
1 🔵 Advisory Accessibility Horizontal hand-wired shape still named both lists "Suggestions" (bulma-ui/src/form/Autocomplete.tsx:472) Verified fixed in ad46c064 / 93b59fdc; declined follow-up accepted on its reason
2 🔵 Advisory Coverage No horizontal shape in the labelSources matrix (bulma-ui/src/form/__tests__/Autocomplete.test.tsx:700) Verified fixed in 686e87b7
3 🔵 Advisory API The label TSDoc's conditions omitted the Autocomplete's own id (bulma-ui/src/form/Autocomplete.tsx:45) Verified fixed in ad46c064 / 1139975f

Overall: This pass settled threads and reviewed no commits. All three of my open advisory threads are addressed in the current code and resolved. Thread 1 was closed in-PR rather than deferred: FieldLabelForContext now carries the id the label renders with, next to its htmlFor (FormContext.tsx:253-276), an unlabeled Field passes both through (Field.tsx:251-258), and Autocomplete reads useFieldLabelFor().id (Autocomplete.tsx:201) — so both values come from one context entry and the id always belongs to the label labelled checked. The remaining case, a hand-wired Field label with no labelProps.id, keeps the "Suggestions" fallback and points at nothing, which is a reasoned trade-off (the alternative is Field generating an id for every hand-wired label) and is pinned by its own it.each. Ran jest over src/form/__tests__/Autocomplete.test.tsx and src/form/__tests__/Field.test.tsx in bulma-ui: 280/280 pass.

Residual risk:

  • Pre-fix useFieldLabelElementId() is targetId ? labelId : undefined, so every hand-wired shape resolved to undefined; the three new hand-wired matrix rows assert a real aria-labelledby and the label's text, so they cannot pass without ad46c064 — the regression guard is real.
  • Horizontal and non-horizontal renderedLabel branches in Field.tsx:259-287 both write the label's id; both shapes are now in labelSources, so losing the id on either fails the matrix.
  • Dangling idrefs on open and close are asserted for every matrix row and for both no-id fallback shapes (danglingIdRefs(container) is empty), including the Escape-closed state.
  • Not re-checked this pass, by design: anything outside these three threads. A reader who wants the current code reviewed should ask for a fresh pass.

🏄 The author didn't park the gnarly horizontal case on a follow-up — they paddled back out and closed it in the same set, then pinned ten label shapes in the matrix so it can't drift. All three of my threads are clean, tests are glassy at 280/280, good to go.

@github-actions github-actions Bot added the review-converged Deep review converged: no blocking findings, all review threads resolved, checks green label Oct 9, 2026
@allxsmith allxsmith added the needs-human-review Loop converged (or contested): awaiting owner review + manual merge label Oct 9, 2026
@github-actions github-actions Bot removed the review-converged Deep review converged: no blocking findings, all review threads resolved, checks green label Oct 9, 2026
@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

📸 Story screenshots at handoff — bf9d2f5

12 stories (light + dark) from this PR's changed files under bulma-ui/src/ (workflow run · artifact story-screenshots-pr-1008, 30-day retention).

form-autocomplete--default · Form/Autocomplete · Default
Light Dark
form-autocomplete--default light form-autocomplete--default dark
form-autocomplete--object-data · Form/Autocomplete · Object Data
Light Dark
form-autocomplete--object-data light form-autocomplete--object-data dark
form-autocomplete--clearable · Form/Autocomplete · Clearable
Light Dark
form-autocomplete--clearable light form-autocomplete--clearable dark
form-autocomplete--clear-in-loading-control · Form/Autocomplete · Clear button in a loading Control
Light Dark
form-autocomplete--clear-in-loading-control light form-autocomplete--clear-in-loading-control dark
form-autocomplete--open-on-focus · Form/Autocomplete · Open On Focus
Light Dark
form-autocomplete--open-on-focus light form-autocomplete--open-on-focus dark
form-autocomplete--keep-first · Form/Autocomplete · Keep First
Light Dark
form-autocomplete--keep-first light form-autocomplete--keep-first dark
form-field--default · Form/Field · Default
Light Dark
form-field--default light form-field--default dark
form-field--with-help-text · Form/Field · With Help Text
Light Dark
form-field--with-help-text light form-field--with-help-text dark
form-field--labels-custom-controls · Form/Field · Labels Custom Controls
Light Dark
form-field--labels-custom-controls light form-field--labels-custom-controls dark
form-field--validation-states · Form/Field · Validation States
Light Dark
form-field--validation-states light form-field--validation-states dark
form-field--label-sizes · Form/Field · Label Sizes
Light Dark
form-field--label-sizes light form-field--label-sizes dark
form-field--horizontal · Form/Field · Horizontal
Light Dark
form-field--horizontal light form-field--horizontal dark
> ✂ 27 more matched stories skipped by the cap: `form-autocomplete--custom-template`, `form-autocomplete--colors`, `form-autocomplete--sizes`, `form-autocomplete--loading`, `form-autocomplete--disabled`, `form-autocomplete--empty-state`, `form-autocomplete--header-footer`, `form-autocomplete--controlled`, `form-autocomplete--async-data`, `form-autocomplete--disabled-items`, … — full set in the artifact.

github-actions Bot added a commit that referenced this pull request Oct 9, 2026
@github-actions github-actions Bot added the review-converged Deep review converged: no blocking findings, all review threads resolved, checks green label Oct 9, 2026
# Conflicts:
#	bulma-ui/src/form/useAutoLabelId.ts
…abel's id

The label-id hook now reads the id of the Field label that names the
control from the same context it reads that label's target from, so a
Field label wired by hand with an id in its labelProps names a range
Slider's thumbs, as it already names Autocomplete's suggestion list.
Without that id the thumbs keep their default names, as before.

Autocomplete moves onto the hook's needsLabelId rather than reading the
context and generating its own label id, so both controls share one
mechanism for the label that names them.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deep review (verify) — 4 resolved · 0 open

Thread Severity Area Outcome Location
labelProps lumped the group path in with the two that work hand-wired 🔵 Advisory API Verified fixed in 514127f / 31f08ef — the clause is split, the label TSDoc and field.md gained the list, and a new it.each pins the group half bulma-ui/src/form/Field.tsx:37
Hand-wired path fires in grouped/hasAddons with nothing pinning it 🔵 Advisory Coverage Verified fixed in 80ea9b9 — grouped and addons rows in both matrices, and the intent written where the context is built bulma-ui/src/form/useAutoLabelId.ts:110
skills/bestax-form still taught the shape the docs had just fixed 🔵 Advisory Accessibility Verified fixed in 385f6a4 — SKILL.md, references/api.md and references/patterns.md all teach labelProps={{ htmlFor, id }} skills/bestax-form/SKILL.md:145
No story showed the new behaviour, and one still showed the old shape 🔵 Advisory Coverage Verified fixed in db86af2 / ff8bf90 — the stale shape is wired, three stories added, both wrapper stories wired bulma-ui/src/form/Autocomplete.stories.tsx:203

Overall: This pass settled threads and reviewed no commits. All four previously-open advisory threads are now resolved: each fix is present in the current tree, every claim in the replies checked out against the code it names, and Field + Slider + Autocomplete pass 448/448 with typecheck:tests green over the stories. Nothing of mine is left open, so a human picking this up needs only the usual once-over of the new story docstrings and the skill prose.

Residual risk:

  • A hand-wired label with no id still leaves the list named "Suggestions". Settled earlier as a deliberate choice — naming it would make Field generate an id for every hand-wired label, changing markup for every caller — and it is now pinned both ways: Autocomplete.test.tsx:969-1016 asserts the fallback across plain, horizontal, grouped and addons shapes, and the docs, the labelProps TSDoc and the skill all say an id is required.
  • FieldLabelForContext being narrowed back to targetId, silently dropping the grouped/addons half. Refuted: the grouped and addons rows added in 80ea9b9 assert the thumb names and the combobox name, both of which depend directly on labelled, so that narrowing fails them.
  • A generated artefact left stale by the skill edit. Refuted: bestax-mcp/data/components/Field.json and data/skills.json were regenerated in 31f08ef, and the Test job is green on ff8bf906, which runs gen:catalog:check, gen:mcp:check and conformance.
  • Open, not refuted: the staleness and conformance evidence above leans on that CI run rather than a local re-run. This session had no write access to the checkout, so neither the gen:*:check scripts nor the narrowing experiment could be executed here.

🏄 Four advisories paddled out, four came back clean — every label now calls its own list by name instead of each combobox shouting "Suggestions" at the same wave. Nothing left bobbing out there, 448 for 448 on the tests. Totally good to go, dude.

@github-actions github-actions Bot added the review-converged Deep review converged: no blocking findings, all review threads resolved, checks green label Oct 10, 2026
# Conflicts:
#	bestax-mcp/data/components/Field.json
#	bestax-mcp/data/skills.json
#	bulma-ui/src/form/Field.tsx
#	docs/docs/api/form/field.md
… list

FieldLabel's TSDoc offers two ways to name a horizontal row's control: a
<label htmlFor> placed in the column, or the Field's label prop. Since a
range Slider's thumbs and an Autocomplete's suggestion list point
aria-labelledby only at a label the label prop renders, the two are not
interchangeable for those controls: the placed label names the control
through its for alone, and the thumbs and list keep their fallback names.
Say so where the two ways are offered.
The skill now teaches two ways to wire a label by hand: a <label htmlFor>
placed in Field.Label for the explicit label/body pattern, and
labelProps={{ htmlFor, id }} on a Field's label prop when an inner Field
or a grouped/hasAddons row stops the automatic association. Only the
second reaches a range Slider's thumbs and an Autocomplete's suggestion
list, so say that a row holding either takes the label prop.
@allxsmith

Copy link
Copy Markdown
Owner Author

deep-review: fresh

Main is merged in to clear a conflict with #1007 in Field.tsx's label TSDoc and docs/docs/api/form/field.md, keeping both PRs' text. The merge also documents a gap between the two: a <label htmlFor> placed inside Field.Label (#1007's compound recipe) names its control through for alone and reaches no range Slider thumbs or Autocomplete list, so FieldLabel's TSDoc and the bestax-form skill now say to use the Field's label prop for those. Read the merged TSDoc, field.md, the skill and the stories together. This is a full review of 551d4cb, posting every finding, advisory included, as its own thread.

@github-actions github-actions Bot removed the review-converged Deep review converged: no blocking findings, all review threads resolved, checks green label Oct 10, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Preview Deployment

Preview URL: https://16869169.bestax.pages.dev

Comment thread bulma-ui/src/form/Field.tsx Outdated
Comment thread bulma-ui/src/form/Autocomplete.tsx

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deep review — 0 blocking · 2 advisory

# Severity Area Finding Location
1 🔵 Advisory API FieldLabel’s new “so use the prop for those” reads as sufficient, but in the inner-Field shape the docs examples use, the label prop alone leaves the input unnamed and the list and thumbs on their fallbacks bulma-ui/src/form/Field.tsx:102
2 🔵 Advisory Accessibility Gating on labelled removes a name 5.27.1 already ships: an own label opted out or pointed elsewhere used to name the list, and now gets “Suggestions” — a behaviour change under a patch scope bulma-ui/src/form/Autocomplete.tsx:467

Overall: The merge is clean and the mechanism is the right one. I diffed both merge parents against 2b311814 and nothing from #1007 was dropped: Field.tsx’s label TSDoc keeps the picker-base list and gains only the one Autocomplete clause, FieldLabel’s expanded block survives intact, and field.md keeps #1007’s compound section while adding this PR’s three Accessibility bullets. The gap the merge documents is real and I traced it to the code: FieldLabelForContext is populated only when label is truthy (Field.tsx:275), so a <label htmlFor> the caller places inside Field.Label never reaches useAutoLabelId’s labelId — the thumbs and the list genuinely cannot point at it, which is what the new TSDoc, field.md:548 and the skill now say. The riskiest part is the only part with no executable guard: the standalone FieldLabel doc block (finding 1), whose “use the prop” advice is incomplete for exactly the shape autocomplete.md and slider.md demonstrate. Start there, then skim finding 2 and decide whether the patch scope is the right vehicle for it. Verified empirically: src/form is 2935 of 2935 green; Field.tsx and useAutoLabelId.ts at 100 percent on all four metrics, Autocomplete.tsx at 99.33/99.27 and Slider.tsx at 99.59/99.48, with both uncovered lines untouched here; tsc --noEmit -p tsconfig.test.json clean over the tests and stories; prettier --check clean on every changed source, skill and docs file.

Residual risk: the failure class is two controls in one form sharing a fallback name — “Suggestions” for Autocomplete lists (#998), “Minimum value”/“Maximum value” for range thumbs (#981/#1006).

  • A <label htmlFor> placed in Field.Label leaves both fallbacks. Confirmed as the gap this merge documents, and confirmed deliberate: labelFor = label ? ownLabelFor : inheritedLabelFor means a Field with no label prop publishes nothing of its own, so a placed label is invisible to the hook. Open by design; finding 1 is only about how the advice reads.
  • A hand-wired Field label with no id. Still falls back, still deliberate (generating one would change Field markup for every caller), and pinned both ways — Autocomplete.test.tsx:971-1028 across plain, horizontal, grouped and addons shapes, Slider.test.tsx:1620-1632, and stated in both doc pages, the labelProps TSDoc and the skill.
  • Refuted — the swap from fieldLabelElementId to fieldLabelFor.id losing a case. I enumerated every shape where the two could diverge. Auto-associated Field: targetId is set, so labelId is identical in both contexts. Nested unlabeled Field: the old context read undefined, and the new one resolves to labelled === false because useFieldLabelId() is reset, so the control adopts nothing. Labeled grouped/hasAddons with no hand-wiring: labelForTarget is undefined, so labelled is false whether or not the control carries an id. No path gains or loses a name except the intended hand-wired-with-id one.
  • Refuted — pairing one label’s htmlFor with another label’s id. ownLabelFor bundles labelForTarget and labelId from the same Field in one useMemo (Field.tsx:265-268), and every labeled Field overwrites the provider, so a nested labeled Field shadows rather than mixes.
  • Refuted — a dangling aria-labelledby as the list opens, highlights and closes. danglingIdRefs sweeps aria-labelledby, aria-describedby, aria-controls, aria-activedescendant and for at all three points for all twelve label sources and for both no-id fallback shapes, and I ran it.
  • Open, pre-existing, not posted as findings: an explicit id on the control inside a plain labeled Field still leaves the input unnamed and the Field’s for dangling (the #495 shape — now stated in Autocomplete.tsx:45, though the danglingIdRefs sweep does not run over it). An outer hand-wired label whose row holds a labeled inner Field is shadowed, so the list says “Suggestions” while the outer for still names the input in a browser. Two Autocompletes in one plain labeled Field share both the adopted id and the list name. Taginput.tsx and Calendar.tsx render a role=listbox with no accessible name at all.
  • Open: an aria-label or aria-labelledby passed to Autocomplete lands on the wrapper <div> (Autocomplete.tsx:477), which carries no role, so it names neither the combobox nor the list — there is no caller escape hatch in any fallback case. Unchanged by this diff and acknowledged in the PR body as its own change, with no issue linked.
  • Not re-run locally: gen:mcp:check, gen:catalog:check and check:conformance — this session had no write access to the checkout. Checked by inspection instead: every generated region in bestax-mcp’s data/components/Autocomplete.json, Field.json and Slider.json, and the references/*.md byte counts in data/skills.json, matches the source it is derived from, and the docs prop tables match the TSDoc verbatim.

🏄 Merged two swells into one clean line — the label prop now calls the list and both thumbs by name, and where it cannot reach, the docs, the skill and the stories all say so instead of pretending. Only thing still foamy is one sentence in Field.Label telling you to paddle for a wave the inner Field already closed out. Good to go, dude.

…t in an inner Field

FieldLabel's TSDoc told readers to use the label prop for a range Slider or an
Autocomplete, which is not enough when an inner Field holds the control: that
Field starts its own scope, so the label names nothing until
labelProps={{ htmlFor, id }} and a matching id wire it. State the full
condition there, in the Field page's compound section and in the bestax-form
skill.
@allxsmith

Copy link
Copy Markdown
Owner Author

deep-review: verify

Every open thread has a reply: fixed, with the commit, or refuted with a reason. This settles them on 30b7dd2.

@github-actions

Copy link
Copy Markdown
Contributor

Preview Deployment

Preview URL: https://ba834e55.bestax.pages.dev

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deep review (verify) — 2 resolved · 0 open

No Severity Area Finding Location
1 🔵 Advisory API FieldLabel TSDoc read as a sufficient recipe in the inner-Field shape — fixed in 9b33d8d, resolved bulma-ui/src/form/Field.tsx:102
2 🔵 Advisory Accessibility Released list name changes in the labelProps.htmlFor opt-out rows, under a fix scope — reason accepted, resolved bulma-ui/src/form/Autocomplete.tsx:467

Overall: This pass settled threads and reviewed no commits. Both of the threads it found open were advisory, and both are now closed. The FieldLabel TSDoc finding was fixed by stating the whole condition, which I traced through the labelFor = label ? ownLabelFor : inheritedLabelFor ternary at Field.tsx:274 and through labelled/labelId in useAutoLabelId.ts:84-110; the same qualifier was carried into the hand-written compound paragraph on the Field page and into skills/bestax-form. The patch-scope finding was answered with a reasoned maintainer ruling that the two changed rows were a defect rather than a contract a caller could lean on — a deliberate choice, accepted as such. jest src/form over Autocomplete, Field and Slider is green at 466 tests on 30b7dd2b. Nothing of mine is left open; a reader who wants the current code reviewed afresh should ask for deep-review: fresh.

Residual risk: Not re-assessed on this pass — a verify pass settles threads and raises no new findings. The risks named in the original review stand as recorded there, each either refuted in its own thread or accepted on the record.

🏄 Paddled back out just to close the two advisories still bobbing in the lineup: one got patched clean, the other got a straight answer from the lifeguard. Water's flat, no threads left open, this one's good to go.

@github-actions github-actions Bot added the review-converged Deep review converged: no blocking findings, all review threads resolved, checks green label Oct 10, 2026
@allxsmith
allxsmith merged commit 363eea5 into main Oct 10, 2026
61 checks passed
@allxsmith
allxsmith deleted the fix/998-autocomplete-list-name branch October 10, 2026 11:40
@bestax-release-bot

Copy link
Copy Markdown

🎉 This PR is included in version 5.27.6 🎉

The release is available on:

Your semantic-release bot 📦🚀

@bestax-release-bot

Copy link
Copy Markdown

🎉 This PR is included in version 1.14.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deep-review needs-human-review Loop converged (or contested): awaiting owner review + manual merge released review-converged Deep review converged: no blocking findings, all review threads resolved, checks green

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Autocomplete: name the suggestion list from a surrounding Field's label

1 participant