feat(accordion): ✨ improve accordion state with better aria & tests

src/accordion/Accordion.ts

@@ -0,0 +1,35 @@
+import { createHook, createComponent } from "reakit-system";
+import { CompositeOptions, CompositeHTMLProps, useComposite } from "reakit";
+
+import { ACCORDION_KEYS } from "./__keys";
+
+export type AccordionOptions = CompositeOptions;
+
+export type AccordionHTMLProps = CompositeHTMLProps;
+
+export type AccordionProps = AccordionOptions & AccordionHTMLProps;
+
+export const useAccordion = createHook<AccordionOptions, AccordionHTMLProps>({
+  name: "Accordion",
+  compose: useComposite,
+  keys: ACCORDION_KEYS,
+
+  useComposeProps(options, htmlProps) {
+    const compositeHtmlProp = useComposite(options, htmlProps);
+
+    return {
+      ...compositeHtmlProp,
+
+      // When none selected i.e, selectedId={null}
+      // as per composite https://github.com/reakit/reakit/blob/master/packages/reakit/src/Composite/Composite.ts#L372
+      // it applies tabindex={0} which we need to remove it.
+      tabIndex: undefined,
+    };
+  },
+});
+
+export const Accordion = createComponent({
+  as: "div",
+  useHook: useAccordion,
+  memo: true,
+});

src/accordion/AccordionItem.tsx

@@ -1,84 +1,150 @@
-import * as React from "react";
-import { useForkRef } from "reakit-utils";
-import { createContext } from "@chakra-ui/utils";
-import { createComponent, createHook } from "reakit-system";
 import {
-  unstable_IdHTMLProps,
-  unstable_IdOptions,
-  unstable_useId,
+  useCompositeItem,
+  CompositeItemOptions,
+  CompositeItemHTMLProps,
+  useButton,
+  ButtonOptions,
+  ButtonHTMLProps,
 } from "reakit";
+import * as React from "react";
+import { useLiveRef } from "reakit-utils";
+import { ariaAttr } from "@chakra-ui/utils";
+import { createHook, createComponent } from "reakit-system";
 
 import { ACCORDION_ITEM_KEYS } from "./__keys";
-import { AccordionStateReturn, Item } from "./AccordionState";
+import { AccordionStateReturn } from "./AccordionState";
 
-export type AccordionItemOptions = unstable_IdOptions &
-  Pick<AccordionStateReturn, "registerItem" | "activeItems" | "items"> & {
-    isOpen?: boolean;
-  };
+export type AccordionItemOptions = ButtonOptions &
+  CompositeItemOptions &
+  Pick<Partial<AccordionStateReturn>, "manual"> &
+  Pick<
+    AccordionStateReturn,
+    | "panels"
+    | "selectedId"
+    | "selectedIds"
+    | "select"
+    | "unSelect"
+    | "allowMultiple"
+    | "allowToggle"
+  >;
 
-export type AccordionItemHTMLProps = unstable_IdHTMLProps;
+export type AccordionItemHTMLProps = ButtonHTMLProps & CompositeItemHTMLProps;
 
 export type AccordionItemProps = AccordionItemOptions & AccordionItemHTMLProps;
 
-type TAccordionItemContext = {
-  isOpen: boolean;
-  item: Item | undefined;
-};
-
-export const [AccordionItemProvider, useAccordionItemContext] = createContext<
-  TAccordionItemContext
->({
-  name: "useAccordionItemContext",
-  errorMessage:
-    "The `useAccordionItem` hook must be called from a descendent of the `AccordionItemProvider`.",
-  strict: true,
-});
+function useAccordionPanelId(options: AccordionItemOptions) {
+  return React.useMemo(
+    () =>
+      options.panels?.find(panel => panel.groupId === options.id)?.id ||
+      undefined,
+    [options.panels, options.id],
+  );
+}
 
 export const useAccordionItem = createHook<
   AccordionItemOptions,
   AccordionItemHTMLProps
 >({
-  name: "AccordionItem",
-  compose: unstable_useId,
+  name: "Accordion",
+  compose: [useButton, useCompositeItem],
   keys: ACCORDION_ITEM_KEYS,
 
+  useOptions({ focusable = true, ...options }) {
+    return { focusable, ...options };
+  },
+
   useProps(
-    { id, registerItem, activeItems, items, isOpen: isOpenOption },
-    { ref: htmlRef, children: htmlChildren, ...htmlProps },
+    options,
+    { onClick: htmlOnClick, onFocus: htmlOnFocus, ...htmlProps },
   ) {
-    const ref = React.useRef<HTMLElement>(null);
+    const selected = isAccordionSelected(options);
+    const accordionPanelId = useAccordionPanelId(options);
+    const onClickRef = useLiveRef(htmlOnClick);
+    const onFocusRef = useLiveRef(htmlOnFocus);
+
+    const onClick = React.useCallback(
+      (event: React.MouseEvent) => {
+        onClickRef.current?.(event);
+        if (event.defaultPrevented) return;
+        if (options.disabled) return;
+        if (!options.id) return;
+
+        if (selected) {
+          if (options.allowToggle && !options.allowMultiple) {
+            // Do not send null to make the toggle because that will also reset
+            //  the current Id in composite hence thats handled directly in state
+            options.select(options.id);
+            return;
+          }
+
+          options.unSelect(options.id);
+          return;
+        }
 
-    React.useLayoutEffect(() => {
-      if (!id) return undefined;
+        options.select?.(options.id);
+      },
 
-      registerItem?.({ id, ref });
-    }, [id, registerItem]);
+      // eslint-disable-next-line react-hooks/exhaustive-deps
+      [options.disabled, selected, options.select, options.id],
+    );
+
+    const onFocus = React.useCallback(
+      (event: React.FocusEvent) => {
+        onFocusRef.current?.(event);
+        if (event.defaultPrevented) return;
+        if (options.disabled) return;
+        if (options.manual) return;
+        if (!options.id) return;
+
+        if (selected) {
+          if (options.allowToggle && !options.allowMultiple) {
+            // Do not send null to make the toggle because that will also reset
+            //  the current Id in composite hence thats handled directly in state
+            options.select(options.id);
+            return;
+          }
+
+          options.unSelect(options.id);
+          return;
+        }
+
+        options.select?.(options.id);
+      },
 
-    const isOpenLocal = id ? activeItems.includes(id) : false;
-    const isOpen = isOpenOption ?? isOpenLocal;
-    const item = items.find(({ id: itemId }) => itemId === id);
-    const children = (
-      <AccordionItemProvider value={{ item, isOpen }}>
-        {htmlChildren}
-      </AccordionItemProvider>
+      // eslint-disable-next-line react-hooks/exhaustive-deps
+      [options.id, options.disabled, options.manual, selected, options.select],
     );
 
     return {
-      ref: useForkRef(ref, htmlRef),
-      children,
+      "aria-expanded": selected,
+      "aria-controls": accordionPanelId,
+      "aria-disabled": ariaAttr(!options.allowToggle && selected),
+      onClick,
+      onFocus,
       ...htmlProps,
     };
   },
 
-  useComposeProps(_, htmlProps) {
-    // We don't want to run `unstable_useId` hook in compose.
-    // So that we only use it for useOptions & use the id to register the item.
-    return htmlProps;
+  useComposeProps(options, htmlProps) {
+    const buttonHtmlProps = useButton(options, htmlProps);
+    const compositeHtmlProps = useCompositeItem(options, buttonHtmlProps);
+
+    return {
+      ...compositeHtmlProps,
+      tabIndex: 0,
+    };
   },
 });
 
 export const AccordionItem = createComponent({
-  as: "div",
+  as: "button",
   memo: true,
   useHook: useAccordionItem,
 });
+
+function isAccordionSelected(options: AccordionItemOptions) {
+  const { id, allowMultiple, selectedId, selectedIds } = options;
+
+  if (!allowMultiple) return selectedId === id;
+  return selectedIds?.includes(id);
+}

src/accordion/AccordionPanel.ts

@@ -1,55 +1,140 @@
-import * as React from "react";
-import { useForkRef } from "reakit-utils";
-import { createComponent, createHook } from "reakit-system";
 import {
-  unstable_IdHTMLProps,
-  unstable_IdOptions,
+  DisclosureContentOptions,
+  DisclosureContentHTMLProps,
+  useDisclosureContent,
   unstable_useId,
+  unstable_IdOptions,
+  unstable_IdHTMLProps,
 } from "reakit";
+import * as React from "react";
+import { useForkRef } from "reakit-utils";
+import { createHook, createComponent } from "reakit-system";
 
 import { ACCORDION_PANEL_KEYS } from "./__keys";
 import { AccordionStateReturn } from "./AccordionState";
-import { useAccordionItemContext } from "./AccordionItem";
 
-export type AccordionPanelOptions = unstable_IdOptions &
-  Pick<AccordionStateReturn, "registerPanel" | "items" | "activeItems">;
+export type AccordionPanelOptions = DisclosureContentOptions &
+  unstable_IdOptions &
+  Pick<
+    AccordionStateReturn,
+    | "selectedId"
+    | "selectedIds"
+    | "registerPanel"
+    | "unregisterPanel"
+    | "panels"
+    | "items"
+    | "allowMultiple"
+  > & {
+    /**
+     * Accordion's id
+     */
+    accordionId?: string;
+  };
 
-export type AccordionPanelHTMLProps = unstable_IdHTMLProps;
+export type AccordionPanelHTMLProps = DisclosureContentHTMLProps &
+  unstable_IdHTMLProps;
 
 export type AccordionPanelProps = AccordionPanelOptions &
   AccordionPanelHTMLProps;
 
+function getAccordionsWithoutPanel(
+  accordions: AccordionPanelOptions["items"],
+  panels: AccordionPanelOptions["panels"],
+) {
+  const panelsAccordionIds = panels.map(panel => panel.groupId).filter(Boolean);
+
+  return accordions.filter(
+    item => panelsAccordionIds.indexOf(item.id || undefined) === -1,
+  );
+}
+
+function getPanelIndex(
+  panels: AccordionPanelOptions["panels"],
+  panel: typeof panels[number],
+) {
+  const panelsWithoutAccordionId = panels.filter(p => !p.groupId);
+  return panelsWithoutAccordionId.indexOf(panel);
+}
+
+/**
+ * When <AccordionPanel> is used without accordionId:
+ *
+ *  - First render: getAccordionId will return undefined because options.panels
+ * doesn't contain the current panel yet (registerPanel wasn't called yet).
+ * Thus registerPanel will be called without groupId (accordionId).
+ *
+ *  - Second render: options.panels already contains the current panel (because
+ * registerPanel was called in the previous render). This means that we'll be
+ * able to get the related accordionId with the accordion panel index. Basically,
+ * we filter out all the accordions and panels that have already matched. In this
+ * phase, registerPanel will be called again with the proper groupId (accordionId).
+ *
+ *  - In the third render, panel.groupId will be already defined, so we just
+ * return it. registerPanel is not called.
+ */
+function getAccordionId(options: AccordionPanelOptions) {
+  const panel = options.panels?.find(p => p.id === options.id);
+  const accordionId = options.accordionId || panel?.groupId;
+  if (accordionId || !panel || !options.panels || !options.items) {
+    return accordionId;
+  }
+
+  const panelIndex = getPanelIndex(options.panels, panel);
+  const accordionsWithoutPanel = getAccordionsWithoutPanel(
+    options.items,
+    options.panels,
+  );
+  return accordionsWithoutPanel[panelIndex]?.id || undefined;
+}
+
 export const useAccordionPanel = createHook<
   AccordionPanelOptions,
   AccordionPanelHTMLProps
 >({
   name: "AccordionPanel",
-  compose: unstable_useId,
+  compose: [unstable_useId, useDisclosureContent],
   keys: ACCORDION_PANEL_KEYS,
 
-  useProps({ id, registerPanel }, { ref: htmlRef, ...htmlProps }) {
+  useProps(options, { ref: htmlRef, ...htmlProps }) {
     const ref = React.useRef<HTMLElement>(null);
+    const accordionId = getAccordionId(options);
+    const { id, registerPanel, unregisterPanel } = options;
 
     React.useEffect(() => {
       if (!id) return undefined;
+      registerPanel?.({ id, ref, groupId: accordionId });
 
-      registerPanel?.({ id, ref });
-    }, [id, registerPanel]);
-
-    const { item, isOpen } = useAccordionItemContext();
-    const buttonId = item?.button?.id;
+      return () => {
+        unregisterPanel?.(id);
+      };
+    }, [accordionId, id, registerPanel, unregisterPanel]);
 
     return {
-      role: "region",
-      "aria-labelledby": buttonId ?? buttonId,
       ref: useForkRef(ref, htmlRef),
-      hidden: !isOpen,
+      role: "region",
+      "aria-labelledby": accordionId,
       ...htmlProps,
     };
   },
+
+  useComposeOptions(options) {
+    return {
+      visible: isPanelVisible(options),
+      ...options,
+    };
+  },
 });
 
 export const AccordionPanel = createComponent({
   as: "div",
   useHook: useAccordionPanel,
 });
+
+function isPanelVisible(options: AccordionPanelOptions) {
+  const accordionId = getAccordionId(options);
+
+  if (!options.allowMultiple)
+    return accordionId ? options.selectedId === accordionId : false;
+
+  return accordionId ? options.selectedIds?.includes(accordionId) : false;
+}