fix(a11y-filter-panel): allow users to choose the level of the filter panel section heading [AC-4758]

[Stefan3002]

Done

• Allowed users to choose what heading level they want to use when utilizing the SearchAndFilter component.
• This directly allows us to fix a11y violations such as the one in the screenshot down below (where we have an h1 and opening the search and filter component results in an h3 being added).
• Also marked the enclosing div of the chips as a group and linked it to the opening button id
• Improved a11y of the overflow counter component by using a more accessible name inside aria-label
• BROKEN TESTS: If you were testing the overflow control with statements like:

expect(
      document.querySelector(".p-filter-panel-section__chips"),
    ).toHaveAttribute("aria-expanded", "false");

Note that this will now fail as the aria-expanded attribute has been removed from the enclosing div

Instead, you can test it like so:

expect(
      document.querySelector(".p-filter-panel-section__counter"),
    ).not.toBeInTheDocument();

QA

Pinging @canonical/react-library-maintainers for a review.

• Run yarn docs and verify the added prop to the FilterPanelSection
• Run yarn test and verify the added test

Fixes

Fixes: #AC-4758.

[Copilot]

🟡 Changes recommended

One or more issues must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR aims to improve filter-panel heading accessibility by allowing configurable heading levels.

Changes:

• Adds configurable heading rendering to FilterPanelSection.
• Adds coverage for rendering an h2.

File summaries

File	Description
src/components/SearchAndFilter/FilterPanelSection/FilterPanelSection.tsx	Updated as part of this pull request.
src/components/SearchAndFilter/FilterPanelSection/FilterPanelSection.test.tsx	Updated as part of this pull request.

Review details

Suppressed comments (1)

src/components/SearchAndFilter/FilterPanelSection/FilterPanelSection.tsx:77

• The new contract also accepts "p", but the added test covers only the numeric heading path, leaving this paragraph branch unverified. Add an integration assertion (through SearchAndFilter once the prop is forwarded) that headingLevel="p" renders a <p> rather than a heading.

  const HeadingTag = headingLevel === "p" ? "p" : (`h${headingLevel}` as const);

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟢 Approval recommended

No unresolved issues were identified that would block approval.

Review details

• Files reviewed: 5/5 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

The implementation has a type-checking error and can emit a broken aria-labelledby reference when no heading is provided.

Get a fresh assessment by requesting another Copilot review.

Review details

• Files reviewed: 6/6 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

The group role is still assigned aria-expanded, creating an invalid role/state combination.

Get a fresh assessment by requesting another Copilot review.

Review details

• Files reviewed: 6/6 changed files
• Comments generated: 1
• Review effort level: Lite

[edlerd]

This is a bit odd, can we use ElementType here instead? We already use it in the Notification component.

Suggested change

	headingLevel?: 1 | 2 | 3 | 4 | 5 | 6 | "p";
	headingElement?: ElementType;

[Stefan3002]

Agreed @edlerd , looks cleaner, but how can we make sure users only pass valid arguments? Should we declare something like :

type SearchAndFilterHeadingElement =
  | "h1"
  | "h2"
  | "h3"
  | "h4"
  | "h5"
  | "h6"
  | "p";

[edlerd]

I don't think we need to validate it here. It should be ok to let the user provide any element they like.

[Stefan3002]

Addressed.

[edlerd]

Here as well, better use ElementType.

[Stefan3002]

Addressed.

[Stefan3002]

@edlerd Addressed comments and also improved a bit more the a11y aspects here (please have a look at the updated PR description). This comment is also useful. Can you have a look? There are still 3 a11y things to fix for this component, but I will separate those in another PR. Thanks!

[Copilot]

🟡 Changes recommended

The new/updated interactive “button-like” spans need correct keyboard activation semantics and the new headingElement API typing + updated tests should be made robust/type-safe before merging.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (5)

src/components/SearchAndFilter/SearchAndFilter.tsx:318

• p-search-and-filter__selected-count is a focusable element with role="button", but its onKeyDown handler triggers expansion for any keypress (not just Enter/Space). This can cause unexpected expansion when using the keyboard and isn’t consistent with expected button keyboard semantics.

            onClick={() => setSearchBoxExpanded(true)}
            onKeyDown={() => setSearchBoxExpanded(true)}
            role="button"
            tabIndex={0}
            aria-label={`Show ${overflowSearchTermCounter} more ${

src/components/SearchAndFilter/FilterPanelSection/FilterPanelSection.tsx:160

• The overflow counter uses role="button" but relies on onKeyPress={showAllChips}. onKeyPress is deprecated and also fires for keys that shouldn’t activate a button; for keyboard accessibility this should be handled via onKeyDown and limited to Enter/Space (with preventDefault() for Space).

                onKeyPress={showAllChips}

src/components/SearchAndFilter/FilterPanelSection/FilterPanelSection.tsx:39

• headingElement is typed as ElementType, but this component unconditionally applies intrinsic-element props (dangerouslySetInnerHTML, id, className). With ElementType (without a props generic), TypeScript won’t prevent callers from passing a component that can’t accept these props, which can break the aria-labelledby linkage at runtime. Restrict this to supported intrinsic tags ("h1"-"h6" | "p") to match the intended usage.

  /**
   * Element used for the section title. Supports heading levels or paragraph.
   */
  headingElement?: ElementType;
};

src/components/SearchAndFilter/SearchAndFilter.tsx:46

• headingElement is typed as ElementType, which implies consumers can pass arbitrary components. However, FilterPanelSection always passes DOM-only props like dangerouslySetInnerHTML, id, and className to this element, and the type currently won’t catch incompatible components. Since the prop is intended to support heading levels (and p), narrow the public API type to those intrinsic tags so callers can’t accidentally pass unsupported components.

  /**
   * Element used for each section title. Supports heading levels or paragraph.
   */
  headingElement?: ElementType;
};

src/components/SearchAndFilter/FilterPanelSection/FilterPanelSection.test.tsx:133

• Now that the overflow counter has role="button" and an accessible name via aria-label, these assertions/clicks can use Testing Library role queries instead of document.querySelector, making the test closer to user behavior and less tied to CSS selectors.

      document.querySelector(".p-filter-panel-section__counter"),
    ).toBeInTheDocument();
    await userEvent.click(
      // Use a query selector because the element's text is split up over
      // multiple elements so it can't be selected by its content.

• Files reviewed: 6/6 changed files
• Comments generated: 3
• Review effort level: Lite

[edlerd]

LGTM, thanks

[github-actions]

🎉 This PR is included in version 4.11.2 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀