Skip to content

BbDrawerTrigger and BbDrawerClose are a bare div with @onclick — no button, no tabindex, no AsChild branch #507

Description

@mathewtaylor

Found while working #459. BbDrawerTrigger and BbDrawerClose are on that issue's list of components with no focus styling, and the reason they cannot have any is that they are not focusable at all.

What they render

Both are the same, in full:

<div @onclick="HandleClick" class="@CssClass" @attributes="AdditionalAttributes">
    @ChildContent
</div>

No tabindex, no role="button", no @onkeydown, and no AsChild branch.

Why it is not always broken

In the demos the trigger wraps a BbButton:

<BbDrawerTrigger>
    <BbButton Variant="ButtonVariant.Outline">Open Drawer</BbButton>
</BbDrawerTrigger>

That inner button is focusable, and Enter or Space on it fires a native click that bubbles to the outer div's @onclick. So the common path works, which is presumably why this has not been reported.

When it breaks

The API places no constraint on ChildContent. Pass anything that is not itself focusable — plain text, an icon, a <span>, a styled <div> — and the trigger becomes unreachable by keyboard and is not exposed as an interactive element to assistive technology. It is a <div> that happens to respond to a mouse.

That is a WCAG 2.1.1 (Keyboard) failure in a shape the component invites, and it fails silently: it looks correct, and it works for anyone testing with a mouse.

Drawer is the odd one out

Every sibling renders a real <button> in its non-AsChild branch and cascades a TriggerContext in the AsChild branch:

component renders <button> has AsChild
BbDialogTrigger yes yes
BbSheetTrigger yes yes
BbPopoverTrigger yes yes
BbDialogClose yes (with @onkeydown) yes
BbDrawerTrigger no no
BbDrawerClose no no

BbDialogClose even carries an explicit @onkeydown handler, so the intended pattern is well established in the codebase — Drawer simply never followed it.

Suggested fix

Give both the same shape as the Dialog and Sheet equivalents: a <button type="button"> in the default branch, and an AsChild branch cascading TriggerContext for the wrap-a-BbButton usage the demos rely on.

This is a behaviour change worth thinking about, which is why it is a separate issue rather than folded into #459. Today's markup nests a focusable child inside a clickable div; switching the default to a real <button> would nest a BbButton inside a <button>, which is invalid HTML. The AsChild branch is what avoids that, so the demos and any consumer using the wrap pattern need moving to AsChild="true" at the same time — or AsChild needs to default to true for these two, unlike its siblings.

Once they are focusable, they also need the ring from #459, which is currently blocked on this.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions