ebay-filter-menu-button: aria-pressed incorrectly rendered on menu item selection

[ArtBlue]

Fixes #693

---

Generated by Local Conductor · QA-validated before push

[changeset-bot]

🦋 Changeset detected

Latest commit: 8997df6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages

Name	Type
@ebay/skin	Minor
@ebay/ebayui-core	Patch
@ebay/ui-core-react	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[Copilot]

Pull request overview

Fixes an accessibility/markup bug where ebay-filter-menu-button incorrectly rendered aria-pressed (a toggle-button attribute) when menu items were selected, by removing the attribute and switching the “selected” visual styling to a dedicated BEM modifier class.

Changes:

• Remove aria-pressed from the Marko and React implementations and add filter-menu-button__button--active when any item is selected.
• Update Skin SCSS and Storybook story markup to use the new active modifier class instead of [aria-pressed="true"].
• Update React unit tests and add a changeset for package versioning.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
packages/skin/src/sass/filter-menu-button/stories/filter-menu-button.stories.js	Updates the “pressed” story markup to use the active modifier class (no aria-pressed).
packages/skin/src/sass/filter-menu-button/filter-menu-button.scss	Switches selected-state styling selectors from [aria-pressed="true"] to .filter-menu-button__button--active.
packages/ebayui-core/src/components/ebay-filter-menu-button/index.marko	Removes aria-pressed and conditionally adds the active modifier class when there are checked values.
packages/ebayui-core-react/src/ebay-filter-menu-button/filter-menu-button.tsx	Removes aria-pressed and conditionally adds the active modifier class based on hasChecked.
packages/ebayui-core-react/src/ebay-filter-menu-button/tests/index.spec.tsx	Updates assertions to check for the active modifier class and absence of aria-pressed.
.changeset/fix-filter-menu-button-aria-pressed.md	Adds versioning metadata and notes for the change.

Comments suppressed due to low confidence (1)

packages/skin/src/sass/filter-menu-button/filter-menu-button.scss:61

• The built CSS in packages/skin/dist/filter-menu-button/filter-menu-button.css still contains aria-pressed selectors, so the published Skin output will likely not reflect this SCSS change unless dist is regenerated and checked in (e.g., via npm run build:css). Please regenerate and commit the updated dist artifacts for this module.

button.filter-menu-button__button--active {
    @include token-mixins.border-color-token(
        filter-button-foreground-color,
        color-border-strong
    );

[LuLaValva]

This isn't a complete solution to the problem. Looks like we already do have a complete solution in skin (see here), but we haven't yet updated Marko or React components.

Here is what actually needs to happen:

1. Use filter-chip--selected instead of creating a new class
2. Add <span class="filter-chip__count">(+x)</span> and/or a way of letting devs modify __count themselves
3. Require i18n a11y text like "Filter Applied" and put it in <span class="clipped">

[ArtBlue]

This isn't a complete solution to the problem. Looks like we already do have a complete solution in skin (see here), but we haven't yet updated Marko or React components.

Here is what actually needs to happen:

1. Use filter-chip--selected instead of creating a new class
2. Add <span class="filter-chip__count">(+x)</span> and/or a way of letting devs modify __count themselves
3. Require i18n a11y text like "Filter Applied" and put it in <span class="clipped">

Great catch!

[LuLaValva]

A few things:

1. I don't see types for either of these new inputs, are you sure it passes CI?
2. If hasSelected is derived from the checked values, count should be too
   • If we think there are circumstances where count can't be derived, then there are certainly also cases where hasSelected can't be
   • We need make this opt-in if we want to prevent breaking changes (I assume some users have already added (+n) to their input.text)

[LuLaValva]

We should probably call this --selected to align with filter-chip--selected

[LuLaValva]

Why are we adding filter-chip and filter-chip--selected? I don't think adding chip styles is in-scope for this issue

[LuLaValva]

@ArtBlue I made a mistake, it was not a great catch! We had a solution for selected chip but not for filter-menu-button. We shouldn't be adding chip styles to this component

[github-actions]

PR Preview Deployed

Website • evo-marko • evo-react • ebayui-core • ebayui-core-react • skin

commit 8997df6