[Checkbox, Menu, Radio] Avoid applying hidden attr when `keepMounte…

…d=true` for indicators (#1329) Additionally, sets `keepMounted=false` for `Radio` by default to align with other indicators
mui · Jan 22, 2025 · 6fdba6b · 6fdba6b
1 parent 4803e59
commit 6fdba6b
Show file tree

Hide file tree

Showing 10 changed files with 34 additions and 58 deletions.
diff --git a/docs/reference/generated/radio-indicator.json b/docs/reference/generated/radio-indicator.json
@@ -8,7 +8,7 @@
     },
     "keepMounted": {
       "type": "boolean",
-      "default": "true",
+      "default": "false",
       "description": "Whether to keep the HTML element in the DOM when the radio button is inactive."
     },
     "render": {

diff --git a/packages/react/src/checkbox/indicator/CheckboxIndicator.test.tsx b/packages/react/src/checkbox/indicator/CheckboxIndicator.test.tsx
@@ -71,7 +71,6 @@ describe('<Checkbox.Indicator />', () => {
       );
       const indicator = container.querySelector('span');
       expect(indicator).not.to.equal(null);
-      expect(indicator).to.have.attribute('hidden');
     });
 
     it('should keep indicator mounted when checked', async () => {
@@ -82,7 +81,6 @@ describe('<Checkbox.Indicator />', () => {
       );
       const indicator = container.querySelector('span');
       expect(indicator).not.to.equal(null);
-      expect(indicator).not.to.have.attribute('hidden');
     });
 
     it('should keep indicator mounted when indeterminate', async () => {
@@ -93,7 +91,6 @@ describe('<Checkbox.Indicator />', () => {
       );
       const indicator = container.querySelector('span');
       expect(indicator).not.to.equal(null);
-      expect(indicator).not.to.have.attribute('hidden');
     });
   });
 
@@ -108,22 +105,22 @@ describe('<Checkbox.Indicator />', () => {
         <div>
           <button onClick={() => setChecked(false)}>Close</button>
           <Checkbox.Root checked={checked}>
-            <Checkbox.Indicator data-testid="indicator" keepMounted />
+            <Checkbox.Indicator data-testid="indicator" />
           </Checkbox.Root>
         </div>
       );
     }
 
     const { user } = await render(<Test />);
 
-    expect(screen.getByTestId('indicator')).not.to.have.attribute('hidden');
+    expect(screen.getByTestId('indicator')).not.to.equal(null);
 
     const closeButton = screen.getByText('Close');
 
     await user.click(closeButton);
 
     await waitFor(() => {
-      expect(screen.getByTestId('indicator')).to.have.attribute('hidden');
+      expect(screen.queryByTestId('indicator')).to.equal(null);
     });
   });
 
@@ -172,16 +169,13 @@ describe('<Checkbox.Indicator />', () => {
     }
 
     const { user } = await render(<Test />);
-
-    expect(screen.getByTestId('indicator')).not.to.have.attribute('hidden');
+    expect(screen.getByTestId('indicator')).not.to.equal(null);
 
     const closeButton = screen.getByText('Close');
     await user.click(closeButton);
 
     await waitFor(() => {
-      expect(screen.getByTestId('indicator')).to.have.attribute('hidden');
+      expect(animationFinished).to.equal(true);
     });
-
-    expect(animationFinished).to.equal(true);
   });
 });
diff --git a/packages/react/src/checkbox/indicator/CheckboxIndicator.tsx b/packages/react/src/checkbox/indicator/CheckboxIndicator.tsx
@@ -28,7 +28,7 @@ const CheckboxIndicator = React.forwardRef(function CheckboxIndicator(
 
   const rendered = rootState.checked || rootState.indeterminate;
 
-  const { mounted, transitionStatus, setMounted } = useTransitionStatus(rendered);
+  const { transitionStatus, setMounted } = useTransitionStatus(rendered);
 
   const indicatorRef = React.useRef<HTMLSpanElement | null>(null);
   const mergedRef = useForkRef(forwardedRef, indicatorRef);
@@ -65,10 +65,7 @@ const CheckboxIndicator = React.forwardRef(function CheckboxIndicator(
     state,
     className,
     customStyleHookMapping,
-    extraProps: {
-      hidden: !mounted,
-      ...otherProps,
-    },
+    extraProps: otherProps,
   });
 
   const shouldRender = keepMounted || rendered;

diff --git a/packages/react/src/menu/checkbox-item-indicator/MenuCheckboxItemIndicator.test.tsx b/packages/react/src/menu/checkbox-item-indicator/MenuCheckboxItemIndicator.test.tsx
@@ -43,7 +43,7 @@ describe('<Menu.CheckboxItemIndicator />', () => {
               <Menu.Positioner>
                 <Menu.Popup>
                   <Menu.CheckboxItem checked={checked}>
-                    <Menu.CheckboxItemIndicator data-testid="indicator" keepMounted />
+                    <Menu.CheckboxItemIndicator data-testid="indicator" />
                   </Menu.CheckboxItem>
                 </Menu.Popup>
               </Menu.Positioner>
@@ -55,14 +55,14 @@ describe('<Menu.CheckboxItemIndicator />', () => {
 
     const { user } = await render(<Test />);
 
-    expect(screen.getByTestId('indicator')).not.to.have.attribute('hidden');
+    expect(screen.queryByTestId('indicator')).not.to.equal(null);
 
     const closeButton = screen.getByText('Close');
 
     await user.click(closeButton);
 
     await waitFor(() => {
-      expect(screen.getByTestId('indicator')).to.have.attribute('hidden');
+      expect(screen.queryByTestId('indicator')).to.equal(null);
     });
   });
 
@@ -125,9 +125,7 @@ describe('<Menu.CheckboxItemIndicator />', () => {
     await user.click(closeButton);
 
     await waitFor(() => {
-      expect(screen.getByTestId('indicator')).to.have.attribute('hidden');
+      expect(animationFinished).to.equal(true);
     });
-
-    expect(animationFinished).to.equal(true);
   });
 });
diff --git a/packages/react/src/menu/checkbox-item-indicator/MenuCheckboxItemIndicator.tsx b/packages/react/src/menu/checkbox-item-indicator/MenuCheckboxItemIndicator.tsx
@@ -27,15 +27,14 @@ const MenuCheckboxItemIndicator = React.forwardRef(function MenuCheckboxItemIndi
   const indicatorRef = React.useRef<HTMLSpanElement | null>(null);
   const mergedRef = useForkRef(forwardedRef, indicatorRef);
 
-  const { mounted, transitionStatus, setMounted } = useTransitionStatus(item.checked);
+  const { transitionStatus, setMounted } = useTransitionStatus(item.checked);
 
   const getItemProps = React.useCallback(
     (externalProps = {}) =>
       mergeReactProps(externalProps, {
         'aria-hidden': true,
-        hidden: !mounted,
       }),
-    [mounted],
+    [],
   );
 
   useAfterExitAnimation({

diff --git a/packages/react/src/menu/radio-item-indicator/MenuRadioItemIndicator.test.tsx b/packages/react/src/menu/radio-item-indicator/MenuRadioItemIndicator.test.tsx
@@ -43,7 +43,7 @@ describe('<Menu.RadioItemIndicator />', () => {
                   <Menu.Popup>
                     <Menu.RadioGroup value={value}>
                       <Menu.RadioItem value="a">
-                        <Menu.RadioItemIndicator data-testid="indicator" keepMounted />
+                        <Menu.RadioItemIndicator data-testid="indicator" />
                       </Menu.RadioItem>
                       <Menu.RadioItem value="b">
                         <Menu.RadioItemIndicator keepMounted />
@@ -60,14 +60,14 @@ describe('<Menu.RadioItemIndicator />', () => {
 
     const { user } = await render(<Test />);
 
-    expect(screen.getByTestId('indicator')).not.to.have.attribute('hidden');
+    expect(screen.queryByTestId('indicator')).not.to.equal(null);
 
     const closeButton = screen.getByText('Close');
 
     await user.click(closeButton);
 
     await waitFor(() => {
-      expect(screen.getByTestId('indicator')).to.have.attribute('hidden');
+      expect(screen.queryByTestId('indicator')).to.equal(null);
     });
   });
 
@@ -129,15 +129,13 @@ describe('<Menu.RadioItemIndicator />', () => {
 
     const { user } = await render(<Test />);
 
-    expect(screen.getByTestId('indicator')).not.to.have.attribute('hidden');
+    expect(screen.getByTestId('indicator')).not.to.equal(null);
 
     const closeButton = screen.getByText('Close');
     await user.click(closeButton);
 
     await waitFor(() => {
-      expect(screen.getByTestId('indicator')).to.have.attribute('hidden');
+      expect(animationFinished).to.equal(true);
     });
-
-    expect(animationFinished).to.equal(true);
   });
 });
diff --git a/packages/react/src/menu/radio-item-indicator/MenuRadioItemIndicator.tsx b/packages/react/src/menu/radio-item-indicator/MenuRadioItemIndicator.tsx
@@ -27,15 +27,14 @@ const MenuRadioItemIndicator = React.forwardRef(function MenuRadioItemIndicator(
   const indicatorRef = React.useRef<HTMLSpanElement | null>(null);
   const mergedRef = useForkRef(forwardedRef, indicatorRef);
 
-  const { mounted, transitionStatus, setMounted } = useTransitionStatus(item.checked);
+  const { transitionStatus, setMounted } = useTransitionStatus(item.checked);
 
   const getItemProps = React.useCallback(
     (externalProps = {}) =>
       mergeReactProps(externalProps, {
         'aria-hidden': true,
-        hidden: !mounted,
       }),
-    [mounted],
+    [],
   );
 
   useAfterExitAnimation({

diff --git a/packages/react/src/radio-group/RadioGroup.test.tsx b/packages/react/src/radio-group/RadioGroup.test.tsx
@@ -314,10 +314,10 @@ describe('<RadioGroup />', () => {
       render(
         <RadioGroup>
           <Radio.Root value="a" data-testid="a">
-            <Radio.Indicator data-testid="indicator-a" />
+            <Radio.Indicator keepMounted data-testid="indicator-a" />
           </Radio.Root>
           <Radio.Root value="b" data-testid="b">
-            <Radio.Indicator data-testid="indicator-b" />
+            <Radio.Indicator keepMounted data-testid="indicator-b" />
           </Radio.Root>
         </RadioGroup>,
       );

diff --git a/packages/react/src/radio/indicator/RadioIndicator.test.tsx b/packages/react/src/radio/indicator/RadioIndicator.test.tsx
@@ -31,14 +31,10 @@ describe('<Radio.Indicator />', () => {
           <button onClick={() => setValue('b')}>Close</button>
           <RadioGroup value={value}>
             <Radio.Root value="a">
-              <Radio.Indicator
-                className="animation-test-indicator"
-                keepMounted
-                data-testid="indicator-a"
-              />
+              <Radio.Indicator className="animation-test-indicator" data-testid="indicator-a" />
             </Radio.Root>
             <Radio.Root value="a">
-              <Radio.Indicator className="animation-test-indicator" keepMounted />
+              <Radio.Indicator className="animation-test-indicator" />
             </Radio.Root>
           </RadioGroup>
         </div>
@@ -47,14 +43,14 @@ describe('<Radio.Indicator />', () => {
 
     const { user } = await render(<Test />);
 
-    expect(screen.getByTestId('indicator-a')).not.to.have.attribute('hidden');
+    expect(screen.getByTestId('indicator-a')).not.to.equal(null);
 
     const closeButton = screen.getByText('Close');
 
     await user.click(closeButton);
 
     await waitFor(() => {
-      expect(screen.getByTestId('indicator-a')).to.have.attribute('hidden');
+      expect(screen.queryByTestId('indicator-a')).to.equal(null);
     });
   });
 
@@ -109,15 +105,13 @@ describe('<Radio.Indicator />', () => {
 
     const { user } = await render(<Test />);
 
-    expect(screen.getByTestId('indicator-a')).not.to.have.attribute('hidden');
+    expect(screen.getByTestId('indicator-a')).not.to.equal(null);
 
     const closeButton = screen.getByText('Close');
     await user.click(closeButton);
 
     await waitFor(() => {
-      expect(screen.getByTestId('indicator-a')).to.have.attribute('hidden');
+      expect(animationFinished).to.equal(true);
     });
-
-    expect(animationFinished).to.equal(true);
   });
 });
diff --git a/packages/react/src/radio/indicator/RadioIndicator.tsx b/packages/react/src/radio/indicator/RadioIndicator.tsx
@@ -19,13 +19,13 @@ const RadioIndicator = React.forwardRef(function RadioIndicator(
   props: RadioIndicator.Props,
   forwardedRef: React.ForwardedRef<HTMLSpanElement>,
 ) {
-  const { render, className, keepMounted = true, ...otherProps } = props;
+  const { render, className, keepMounted = false, ...otherProps } = props;
 
   const rootState = useRadioRootContext();
 
   const rendered = rootState.checked;
 
-  const { mounted, transitionStatus, setMounted } = useTransitionStatus(rendered);
+  const { transitionStatus, setMounted } = useTransitionStatus(rendered);
 
   const state: RadioIndicator.State = React.useMemo(
     () => ({
@@ -43,10 +43,7 @@ const RadioIndicator = React.forwardRef(function RadioIndicator(
     ref: mergedRef,
     className,
     state,
-    extraProps: {
-      hidden: !mounted,
-      ...otherProps,
-    },
+    extraProps: otherProps,
     customStyleHookMapping,
   });
 
@@ -70,7 +67,7 @@ namespace RadioIndicator {
   export interface Props extends BaseUIComponentProps<'span', State> {
     /**
      * Whether to keep the HTML element in the DOM when the radio button is inactive.
-     * @default true
+     * @default false
      */
     keepMounted?: boolean;
   }
@@ -100,7 +97,7 @@ RadioIndicator.propTypes /* remove-proptypes */ = {
   className: PropTypes.oneOfType([PropTypes.func, PropTypes.string]),
   /**
    * Whether to keep the HTML element in the DOM when the radio button is inactive.
-   * @default true
+   * @default false
    */
   keepMounted: PropTypes.bool,
   /**