Skip to content

Commit

Permalink
Fix aria-activedescendant not being removed when Dropdown or ComboBox…
Browse files Browse the repository at this point in the history
… is closed (#4426)
  • Loading branch information
joshwooding authored Nov 20, 2024
1 parent ef4d7be commit bae6882
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 12 deletions.
5 changes: 5 additions & 0 deletions .changeset/eighty-bears-chew.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@salt-ds/core": patch
---

Fixed aria-activedescendant not being removed when Dropdown or ComboBox is closed.
17 changes: 17 additions & 0 deletions packages/core/src/__tests__/__e2e__/combo-box/ComboBox.cy.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -751,4 +751,21 @@ describe("Given a ComboBox", () => {
.its("duration", { timeout: 0 })
.should("be.lessThan", 5000);
});

it("should remove aria-activedescendant when closed", () => {
cy.mount(<Default />);
cy.findByRole("combobox").realClick();
cy.findByRole("option", { name: "Alabama" }).should("be.activeDescendant");
cy.findByRole("option", { name: "Alaska" }).realClick();
cy.findByRole("combobox").should("not.have.attr", "aria-activedescendant");
cy.findByRole("combobox").realClick();
cy.findByRole("option", { name: "Alaska" }).should("be.activeDescendant");
cy.findByRole("combobox").clear();
cy.findByRole("option", { name: "Alabama" }).realClick();
cy.findByRole("combobox").should("not.have.attr", "aria-activedescendant");
cy.findByRole("combobox").realClick();
cy.findByRole("option", { name: "Alabama" }).should("be.activeDescendant");
cy.realPress("Escape");
cy.findByRole("combobox").should("not.have.attr", "aria-activedescendant");
});
});
16 changes: 16 additions & 0 deletions packages/core/src/__tests__/__e2e__/dropdown/Dropdown.cy.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -473,4 +473,20 @@ describe("Given a Dropdown", () => {
cy.findAllByRole("option").eq(0).realClick();
cy.get("@blurSpy").should("not.have.been.called");
});

it("should remove aria-activedescendant when closed", () => {
cy.mount(<Default />);
cy.findByRole("combobox").realClick();
cy.findByRole("option", { name: "Alabama" }).should("be.activeDescendant");
cy.findByRole("option", { name: "Alaska" }).realClick();
cy.findByRole("combobox").should("not.have.attr", "aria-activedescendant");
cy.findByRole("combobox").realClick();
cy.findByRole("option", { name: "Alaska" }).should("be.activeDescendant");
cy.findByRole("option", { name: "Alabama" }).realClick();
cy.findByRole("combobox").should("not.have.attr", "aria-activedescendant");
cy.findByRole("combobox").realClick();
cy.findByRole("option", { name: "Alabama" }).should("be.activeDescendant");
cy.realPress("Escape");
cy.findByRole("combobox").should("not.have.attr", "aria-activedescendant");
});
});
12 changes: 6 additions & 6 deletions packages/core/src/combo-box/ComboBox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,12 @@ export const ComboBox = forwardRef(function ComboBox<Item>(

// biome-ignore lint/correctness/useExhaustiveDependencies: We only want this to run when the list's openState or the displayed options change.
useEffect(() => {
// If the list is closed we should clear the active item
if (!openState) {
setActive(undefined);
return;
}

// We check the active index because the active item may have been removed
const activeIndex = activeState ? getIndexOfOption(activeState) : -1;
let newActive = undefined;
Expand All @@ -362,12 +368,6 @@ export const ComboBox = forwardRef(function ComboBox<Item>(
return;
}

// If the list is closed we should clear the active item
if (!openState) {
setActive(undefined);
return;
}

// If we have selected an item, we should make that the active item
if (selectedState.length > 0) {
newActive = getOptionsMatching(
Expand Down
12 changes: 6 additions & 6 deletions packages/core/src/dropdown/Dropdown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,12 @@ export const Dropdown = forwardRef(function Dropdown<Item>(

// biome-ignore lint/correctness/useExhaustiveDependencies: We only want this to run when the list's openState or the displayed options change.
useEffect(() => {
// If the list is closed we should clear the active item
if (!openState) {
setActive(undefined);
return;
}

// We check the active index because the active item may have been removed
const activeIndex = activeState ? getIndexOfOption(activeState) : -1;
let newActive = undefined;
Expand All @@ -381,12 +387,6 @@ export const Dropdown = forwardRef(function Dropdown<Item>(
return;
}

// If the list is closed we should clear the active item
if (!openState) {
setActive(undefined);
return;
}

// If we have selected an item, we should make that the active item
if (selectedState.length > 0) {
newActive = getOptionsMatching(
Expand Down

0 comments on commit bae6882

Please sign in to comment.