fix: Support hidden="until-found" in DisclosureGroup (#7199)

* support hidden="until-found" in DisclosureGroup * typescript * cleanup * useLayoutEffect and RAF * add RAF/flushSync * lint * add comments * remove extra changes from merge * revert newlines * typescript * more typescript * use DisclosureTitle
adobe · Oct 21, 2024 · b887497 · b887497 · rspbot · Oct 21, 2024
1 parent 0ddbe6f
commit b887497
Show file tree

Hide file tree

Showing 5 changed files with 149 additions and 13 deletions.
diff --git a/packages/@react-aria/disclosure/src/useDisclosure.ts b/packages/@react-aria/disclosure/src/useDisclosure.ts
@@ -12,8 +12,9 @@
 
 import {AriaButtonProps} from '@react-types/button';
 import {DisclosureState} from '@react-stately/disclosure';
-import {HTMLAttributes, RefObject, useEffect} from 'react';
-import {useEvent, useId} from '@react-aria/utils';
+import {flushSync} from 'react-dom';
+import {HTMLAttributes, RefObject, useCallback, useEffect, useRef} from 'react';
+import {useEvent, useId, useLayoutEffect} from '@react-aria/utils';
 import {useIsSSR} from '@react-aria/ssr';
 
 export interface AriaDisclosureProps {
@@ -40,29 +41,55 @@ export interface DisclosureAria {
  * @param state - State for the disclosure, as returned by `useDisclosureState`.
  * @param ref - A ref for the disclosure content.
  */
-export function useDisclosure(props: AriaDisclosureProps, state: DisclosureState, ref?: RefObject<Element | null>): DisclosureAria {
+export function useDisclosure(props: AriaDisclosureProps, state: DisclosureState, ref: RefObject<Element | null>): DisclosureAria {
   let {
     isDisabled
   } = props;
   let triggerId = useId();
   let contentId = useId();
-  let isControlled = props.isExpanded !== undefined;
   let isSSR = useIsSSR();
   let supportsBeforeMatch = !isSSR && 'onbeforematch' in document.body;
 
+  let raf = useRef<number | null>(null);
+
+  let handleBeforeMatch = useCallback(() => {
+    // Wait a frame to revert browser's removal of hidden attribute
+    raf.current = requestAnimationFrame(() => {
+      if (ref.current) {
+        ref.current.setAttribute('hidden', 'until-found');
+      }
+    });
+    // Force sync state update
+    flushSync(() => {
+      state.toggle();
+    });
+  }, [ref, state]);
+
   // @ts-ignore https://github.com/facebook/react/pull/24741
-  useEvent(ref, 'beforematch', supportsBeforeMatch && !isControlled ? () => state.expand() : null);
+  useEvent(ref, 'beforematch', supportsBeforeMatch ? handleBeforeMatch : null);
 
-  useEffect(() => {
+  useLayoutEffect(() => {
+    // Cancel any pending RAF to prevent stale updates
+    if (raf.current) {
+      cancelAnimationFrame(raf.current);
+    }
     // Until React supports hidden="until-found": https://github.com/facebook/react/pull/24741
-    if (supportsBeforeMatch && ref?.current && !isControlled && !isDisabled) {
+    if (supportsBeforeMatch && ref.current && !isDisabled) {
       if (state.isExpanded) {
         ref.current.removeAttribute('hidden');
       } else {
         ref.current.setAttribute('hidden', 'until-found');
       }
     }
-  }, [isControlled, ref, props.isExpanded, state, supportsBeforeMatch, isDisabled]);
+  }, [isDisabled, ref, state.isExpanded, supportsBeforeMatch]);
+
+  useEffect(() => {
+    return () => {
+      if (raf.current) {
+        cancelAnimationFrame(raf.current);
+      }
+    };
+  }, []);
 
   return {
     buttonProps: {
@@ -87,7 +114,7 @@ export function useDisclosure(props: AriaDisclosureProps, state: DisclosureState
       // This can be overridden at the panel element level.
       role: 'group',
       'aria-labelledby': triggerId,
-      hidden: (!supportsBeforeMatch || isControlled) ? !state.isExpanded : true
+      hidden: supportsBeforeMatch ? true : !state.isExpanded
     }
   };
 }
diff --git a/packages/@react-aria/disclosure/test/useDisclosure.test.ts b/packages/@react-aria/disclosure/test/useDisclosure.test.ts
@@ -163,7 +163,7 @@ describe('useDisclosure', () => {
     });
 
     expect(result.current.state.isExpanded).toBe(false);
-    expect(ref.current.getAttribute('hidden')).toBeNull();
+    expect(ref.current.getAttribute('hidden')).toBe('until-found');
 
     // Simulate the 'beforematch' event
     act(() => {
@@ -172,8 +172,9 @@ describe('useDisclosure', () => {
     });
 
     expect(result.current.state.isExpanded).toBe(false);
-    expect(ref.current.getAttribute('hidden')).toBeNull();
-    expect(onExpandedChange).not.toHaveBeenCalled();
+    expect(ref.current.getAttribute('hidden')).toBe('until-found');
+    expect(onExpandedChange).toHaveBeenCalledTimes(1);
+    expect(onExpandedChange).toHaveBeenCalledWith(true);
 
     Object.defineProperty(document.body, 'onbeforematch', {
       value: originalOnBeforeMatch,

diff --git a/packages/@react-spectrum/s2/stories/Accordion.stories.tsx b/packages/@react-spectrum/s2/stories/Accordion.stories.tsx
@@ -11,6 +11,7 @@
  */
 
 import {Accordion, ActionButton, Disclosure, DisclosureHeader, DisclosurePanel, DisclosureTitle, TextField} from '../src';
+import {Key} from 'react-aria';
 import type {Meta, StoryObj} from '@storybook/react';
 import NewIcon from '../s2wf-icons/S2_Icon_New_20_N.svg';
 import React from 'react';
@@ -130,6 +131,68 @@ WithDisabledDisclosure.parameters = {
   }
 };
 
+function ControlledAccordion(props) {
+  let [expandedKeys, setExpandedKeys] = React.useState<Set<Key>>(new Set(['people']));
+  return (
+    <div className={style({font: 'body', display: 'flex', flexDirection: 'column', gap: 8})}>
+      <Accordion
+        onExpandedChange={setExpandedKeys}
+        expandedKeys={expandedKeys}
+        {...props}>
+        <Disclosure id="files">
+          <DisclosureTitle>
+            Files
+          </DisclosureTitle>
+          <DisclosurePanel>
+            Files content
+          </DisclosurePanel>
+        </Disclosure>
+        <Disclosure id="people">
+          <DisclosureTitle>
+            People
+          </DisclosureTitle>
+          <DisclosurePanel>
+            <TextField label="Name" />
+          </DisclosurePanel>
+        </Disclosure>
+      </Accordion>
+      <div>Expanded keys: {expandedKeys.size ? Array.from(expandedKeys).join(', ') : 'none'}</div>
+    </div>
+  );
+}
+
+export const Controlled: Story = {
+  render: () => <ControlledAccordion />
+};
+
+function ControlledOpenAccordion() {
+  return (
+    <Accordion
+      expandedKeys={['people']}>
+      <Disclosure id="files">
+        <DisclosureTitle>
+          Files
+        </DisclosureTitle>
+        <DisclosurePanel>
+          Files content
+        </DisclosurePanel>
+      </Disclosure>
+      <Disclosure id="people">
+        <DisclosureTitle>
+          People
+        </DisclosureTitle>
+        <DisclosurePanel>
+          <TextField label="Name" />
+        </DisclosurePanel>
+      </Disclosure>
+    </Accordion>
+  );
+}
+
+export const ControlledOpen: Story = {
+  render: () => <ControlledOpenAccordion />
+};
+
 export const WithActionButton: Story = {
   render: (args) => {
     return (
@@ -162,3 +225,4 @@ export const WithActionButton: Story = {
     );
   }
 };
+
diff --git a/packages/@react-spectrum/s2/stories/Disclosure.stories.tsx b/packages/@react-spectrum/s2/stories/Disclosure.stories.tsx
@@ -86,6 +86,49 @@ WithLongTitle.parameters = {
   }
 };
 
+function ControlledDisclosure(props) {
+  let [isExpanded, setExpanded] = React.useState(false);
+  return (
+    <Disclosure {...props} isExpanded={isExpanded} onExpandedChange={setExpanded}>
+      <DisclosureTitle>
+        Files
+      </DisclosureTitle>
+      <DisclosurePanel>
+        Files content
+      </DisclosurePanel>
+    </Disclosure>
+  );
+}
+
+export const Controlled: Story = {
+  render: (args) => <ControlledDisclosure {...args} />
+};
+
+Controlled.parameters = {
+  docs: {
+    disable: true
+  }
+};
+
+export const ControlledClosed: Story = {
+  render: (args) => (
+    <Disclosure isExpanded={false} {...args}>
+      <DisclosureTitle>
+        Files
+      </DisclosureTitle>
+      <DisclosurePanel>
+        Files content
+      </DisclosurePanel>
+    </Disclosure>
+  )
+};
+
+ControlledClosed.parameters = {
+  docs: {
+    disable: true
+  }
+};
+
 export const WithActionButton: Story = {
   render: (args) => {
     return (
@@ -105,3 +148,4 @@ export const WithActionButton: Story = {
     );
   }
 };
+
diff --git a/packages/react-aria-components/src/Disclosure.tsx b/packages/react-aria-components/src/Disclosure.tsx
@@ -108,7 +108,7 @@ const InternalDisclosureContext = createContext<InternalDisclosureContextValue |
 
 function Disclosure(props: DisclosureProps, ref: ForwardedRef<HTMLDivElement>) {
   [props, ref] = useContextProps(props, ref, DisclosureContext);
-  let groupState = useContext(DisclosureGroupStateContext);
+  let groupState = useContext(DisclosureGroupStateContext)!;
   let {id, ...otherProps} = props;
 
   // Generate an id if one wasn't provided.