Skip to content

Commit

Permalink
Fix Accordion becoming inaccessible when an id is passed to Accordion…
Browse files Browse the repository at this point in the history
…Header or AccordionPanel (#3912)
  • Loading branch information
joshwooding authored Aug 1, 2024
1 parent e8d923c commit 0eb21ae
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 14 deletions.
5 changes: 5 additions & 0 deletions .changeset/spicy-lemons-crash.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@salt-ds/core": patch
---

Fixed `Accordion` becoming inaccessible when an id is passed to `AccordionPanel` or `AccordionHeader`.
23 changes: 23 additions & 0 deletions packages/core/src/__tests__/__e2e__/accordion/Accordion.cy.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -138,4 +138,27 @@ describe("GIVEN an Accordion", () => {
});
});
});

it("SHOULD allow a custom id to be set on the header", () => {
cy.mount(
<Accordion value="example" expanded>
<AccordionHeader id="custom-header-id">Summary Text</AccordionHeader>
<AccordionPanel id="custom-panel-id">Panel content</AccordionPanel>
</Accordion>,
);

cy.findByRole("button").should("have.attr", "id", "custom-header-id");
cy.findByRole("region").should(
"have.attr",
"aria-labelledby",
"custom-header-id",
);

cy.findByRole("region").should("have.attr", "id", "custom-panel-id");
cy.findByRole("button").should(
"have.attr",
"aria-controls",
"custom-panel-id",
);
});
});
9 changes: 8 additions & 1 deletion packages/core/src/accordion/Accordion.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
type ComponentPropsWithoutRef,
type SyntheticEvent,
forwardRef,
useState,
} from "react";
import { makePrefixer, useControlled, useId } from "../utils";
import accordionCss from "./Accordion.css";
Expand Down Expand Up @@ -59,6 +60,9 @@ export const Accordion = forwardRef<HTMLDivElement, AccordionProps>(
} = props;

const id = useId(idProp);
const [headerId, setHeaderId] = useState(`${id}-header`);
const [panelId, setPanelId] = useState(`${id}-panel`);

const targetWindow = useWindow();
useComponentCssInjection({
testId: "salt-accordion",
Expand Down Expand Up @@ -86,7 +90,10 @@ export const Accordion = forwardRef<HTMLDivElement, AccordionProps>(
expanded,
indicatorSide,
disabled: Boolean(disabled),
id: id ?? "",
headerId,
setHeaderId,
panelId,
setPanelId,
status,
}}
>
Expand Down
10 changes: 8 additions & 2 deletions packages/core/src/accordion/AccordionContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ export interface AccordionContextValue {
toggle: (event: SyntheticEvent<HTMLButtonElement>) => void;
disabled: boolean;
indicatorSide: "left" | "right";
id: string;
headerId: string;
setHeaderId: (id: string) => void;
panelId: string;
setPanelId: (id: string) => void;
status?: "error" | "warning" | "success";
}

Expand All @@ -19,7 +22,10 @@ export const AccordionContext = createContext<AccordionContextValue>(
toggle: () => undefined,
disabled: false,
indicatorSide: "left",
id: "",
headerId: "",
setHeaderId: () => undefined,
panelId: "",
setPanelId: () => undefined,
},
);

Expand Down
27 changes: 21 additions & 6 deletions packages/core/src/accordion/AccordionHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
} from "react";
import { StatusIndicator } from "../status-indicator";

import { makePrefixer } from "../utils";
import { makePrefixer, useIsomorphicLayoutEffect } from "../utils";

import { useAccordion } from "./AccordionContext";
import accordionHeaderCss from "./AccordionHeader.css";
Expand All @@ -37,9 +37,18 @@ export const AccordionHeader = forwardRef<
HTMLButtonElement,
AccordionHeaderProps
>(function AccordionHeader(props, ref) {
const { children, className, onClick, ...rest } = props;
const { value, expanded, toggle, indicatorSide, disabled, id, status } =
useAccordion();
const { children, className, onClick, id, ...rest } = props;
const {
value,
expanded,
toggle,
indicatorSide,
disabled,
headerId,
panelId,
setHeaderId,
status,
} = useAccordion();

const targetWindow = useWindow();
useComponentCssInjection({
Expand All @@ -53,6 +62,12 @@ export const AccordionHeader = forwardRef<
onClick?.(event);
};

useIsomorphicLayoutEffect(() => {
if (id) {
setHeaderId(id);
}
}, [id]);

return (
<button
ref={ref}
Expand All @@ -64,8 +79,8 @@ export const AccordionHeader = forwardRef<
disabled={disabled}
onClick={handleClick}
aria-expanded={expanded}
id={`${id}-header`}
aria-controls={`${id}-panel`}
id={headerId}
aria-controls={panelId}
value={value}
type="button"
{...rest}
Expand Down
17 changes: 12 additions & 5 deletions packages/core/src/accordion/AccordionPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
forwardRef,
} from "react";

import { makePrefixer } from "../utils";
import { makePrefixer, useIsomorphicLayoutEffect } from "../utils";
import { useAccordion } from "./AccordionContext";
import accordionPanelCss from "./AccordionPanel.css";

Expand All @@ -22,7 +22,7 @@ const withBaseName = makePrefixer("saltAccordionPanel");

export const AccordionPanel = forwardRef<HTMLDivElement, AccordionPanelProps>(
function AccordionPanel(props, ref) {
const { children, className, ...rest } = props;
const { children, className, id, ...rest } = props;

const targetWindow = useWindow();
useComponentCssInjection({
Expand All @@ -31,15 +31,22 @@ export const AccordionPanel = forwardRef<HTMLDivElement, AccordionPanelProps>(
window: targetWindow,
});

const { id, expanded, indicatorSide } = useAccordion();
const { headerId, panelId, setPanelId, expanded, indicatorSide } =
useAccordion();

useIsomorphicLayoutEffect(() => {
if (id) {
setPanelId(id);
}
}, [id]);

return (
<div
ref={ref}
className={clsx(withBaseName(), className)}
role="region"
id={`${id}-panel`}
aria-labelledby={`${id}-header`}
id={panelId}
aria-labelledby={headerId}
aria-hidden={!expanded ? "true" : undefined}
hidden={!expanded}
{...rest}
Expand Down

0 comments on commit 0eb21ae

Please sign in to comment.