fix(accordion): correct trigger semantics and guard keyboard events i…

[goetzrobin]

…n editable fields

• Remove incorrect heading semantics from trigger
• Use proper button semantics with aria-expanded/aria-controls
• Validate ARIA structure: trigger must be a button and be wrapped by a semantic heading
• Add robust keyboard guarding: ignore navigation/toggle handling inside inputs,etc.
• Allow modifier combos (Ctrl/Meta/Alt) to pass through keyboard handling
• Prevent default only for relevant keys when the accordion has focus and based on orientation
• textareas accept newlines without toggling
• Expands e2e test coversage:
  • Add ARIA semantics checks (trigger/button, heading wrapper, region with aria-labelledby)
  • Cover single/multiple behavior, horizontal/vertical navigation, Home/End, focus wrapping
  • Verify form input interactions (input/textarea/select/contenteditable) don’t toggle accordion
  • Add inert behavior checks for focusables in closed panels
  • Run axe checks for default stories

Fixes: #792,#833

PR Checklist

Please check if your PR fulfills the following requirements:

• The commit message follows our
  guidelines: https://github.com/spartan-ng/spartan/blob/main/CONTRIBUTING.md#-commit-message-guidelines
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

• Bugfix
• Feature
• Code style update (formatting, local variables)
• Refactoring (no functional changes, no api changes)
• Build related changes
• CI related changes
• Documentation content changes
• Other... Please describe:

Which package are you modifying?

Primitives

• accordion
• alert
• alert-dialog
• aspect-ratio
• avatar
• badge
• breadcrumb
• button
• calendar
• card
• carousel
• checkbox
• collapsible
• combobox
• command
• context-menu
• data-table
• date-picker
• dialog
• dropdown-menu
• form-field
• hover-card
• icon
• input
• input-otp
• label
• menubar
• navigation-menu
• pagination
• popover
• progress
• radio-group
• scroll-area
• select
• separator
• sheet
• skeleton
• slider
• sonner
• spinner
• switch
• table
• tabs
• textarea
• toggle
• toggle-group
• tooltip
• typography

Others

• trpc
• nx
• repo
• cli

What is the current behavior?

See commit message above

Closes #792,#833

What is the new behavior?

Does this PR introduce a breaking change?

• Yes
• No

We remove the hard coded h3 in favor of prompting the user to wrap the trigger with an h[ANYLEVEL] element themselves. Updates the docs to reflect that

Other information

[goetzrobin]

@ashley-hunter @marcjulian curious to hear your thoughts on this. Should I write a migration for this? Feel like it should be doable and would definitely avoid users having to deal with breaking apps?

[MerlinMoos]

Hey @goetzrobin, short feedback on this. I'm not sure if we should but a button into a heading. Headings can only contain phrasing content, and a is interactive content that makes the HTML invalid and harms accessibility.

[goetzrobin]

Hey @goetzrobin, short feedback on this. I'm not sure if we should but a button into a heading. Headings can only contain phrasing content, and a is interactive content that makes the HTML invalid and harms accessibility.

Hey @MerlinMoos! I actually had the same gut reaction, but this is from the waria specs:

Each accordion header button is wrapped in an element with role heading that has a value set for aria-level that is appropriate for the information architecture of the page.
If the native host language has an element with an implicit heading and aria-level, such as an HTML heading tag, a native host language element may be used.
The button element is the only element inside the heading element. That is, if there are other visually persistent elements, they are not included inside the heading element.

Here is the full article for reference: https://www.w3.org/WAI/ARIA/apg/patterns/accordion/

But I guess we could make the role an input so people could turn it off?

[MerlinMoos]

Im fine by that and you are right. Sorry I was not thinking about it... Till the button is the only element its valid and recommended :) we can keep as it is, sorry.

[ashley-hunter]

Thanks @goetzrobin! Looks good! I think we should be able to write a healthcheck to automate this - would be great if possible 😊

[thatsamsonkid]

@goetzrobin since you are updating accordion here, another quick win I saw we have a few @hostlisteners in brn-accordion.ts, maybe we can move them to host property as well? Although now that I think about it, maybe there was a reason we didn't, vaguely recall maybe @elite-benni saying something about this but maybe I'm imagining things.

Also nice job on the keyboard guarding, I was just thinking if our select was inside the accordion if this would work with keyboard nav and I believe the role checks will cover that!