Repository navigation
fix(Combobox, Popover): point trigger aria-controls at the popup that actually opens - #594
Merged
Merged
Conversation
…utes and aria-invalid (#574) Add TriggerId, TriggerAttributes and IsInvalid to BbCombobox and BbMultiSelect. They land on the trigger button, so a <label for> can point at it, test ids reach the control, and the invalid state is announced. AdditionalAttributes stay on the root div. IsInvalid falls back to the EditContext when unset, and both triggers now show the destructive border like other inputs. BbPopoverTrigger now folds an id it is given into PopoverContext.TriggerId and renders it once, so focus restore and click-outside use the element actually rendered. This also fixes focus restore for BbTreeSelect and BbCascader, which already passed an id.
… actually opens
The popover trigger rendered its own aria-haspopup="true" and aria-controls after the caller's attributes, so Combobox, MultiSelect, TreeSelect and Cascader never got the values they asked for. A caller's aria-haspopup and aria-controls now win.
Combobox asked for {Id}-listbox, which nothing rendered: its list takes the inner command's id. The combobox now cascades its id to the command, so the list renders the id the trigger names. Cascader drops its aria-controls so it keeps naming the dialog.
# Conflicts: # tests/browser/fixtures/StateChanges/Smoke.razor # tests/browser/state-changes.checks.mjs
# Conflicts: # src/BlazorBlueprint.Components/Components/Combobox/BbCombobox.razor.cs # src/BlazorBlueprint.Components/Components/MultiSelect/BbMultiSelect.razor.cs # src/BlazorBlueprint.Primitives/Primitives/Popover/BbPopoverTrigger.razor # tests/browser/fixtures/StateChanges/Smoke.razor # tests/browser/state-changes.checks.mjs
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
1 participant
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.
What was wrong
BbComboboxasked foraria-controls="{Id}-listbox"on its trigger, and nothing renders that id: the open list is aBbCommandList, whose id comes from the command's context (command-{guid}-list).The rendered trigger never carried that value anyway. The popover trigger primitive writes its own
aria-haspopup="true"andaria-controls="{popover content id}"after the caller's attributes, and the last attribute wins. So Combobox, MultiSelect, TreeSelect and Cascader all renderedaria-haspopup="true"(which means a menu) and named the popover panel, whatever they asked for.What changes
aria-haspopupandaria-controlsnow win over the defaults. With neither given, the output is the same as before.BbCommandthrough an internal named cascading value, so the list renders{Id}-listand the trigger names it throughCommandContext.GetListId(Id). The search input'saria-controlsnames the same list.CommandContext.Idgains an internal setter, applied once inOnInitialized, before any child renders.aria-controls. It named an unlabelled innerdiv, and the popover itself is the dialog itsaria-haspopup="dialog"describes, which is what the trigger already names by default.What each trigger renders now:
aria-haspopuparia-controlsnameslistbox(wastrue)role="listbox"(was the popover)listbox(wastrue){Id}-listbox,role="listbox"(was the popover)tree(wastrue)role="tree"(was the popover)dialog(wastrue)role="dialog"(unchanged)trueNo public API changes. The API snapshot gains one line for
BbCommand's new non-publicHostIdcascading parameter.Tests
PopupTriggerAriaTests(bUnit, with aBbPortalHost) opens each of the five triggers and checks thataria-haspopupis right and that exactly one element has the idaria-controlsnames, with the expected role. For Combobox it also checks that the list holds the options, that the search input names the same list, and that two comboboxes name different lists.Before the fix, the four component tests fail on
aria-haspopup="true". With only the popover trigger fixed, the Combobox test times out waiting for the{Id}-listboxelement. Full .NET suite: 1111 passed, 0 failed. The state-changes browser checks (Chrome): 13 passed.