[material-ui] Performance: lazy Ripple (#41061)

Author: romgrk

packages-internal/test-utils/package.json

@@ -39,6 +39,7 @@
     "@emotion/react": "^11.11.4",
     "@testing-library/dom": "^10.3.1",
     "@testing-library/react": "^16.0.0",
+    "@testing-library/user-event": "^14.5.2",
     "chai": "^4.4.1",
     "chai-dom": "^1.12.0",
     "dom-accessibility-api": "^0.6.3",

packages-internal/test-utils/src/createRenderer.tsx

@@ -15,6 +15,7 @@ import {
   within,
   RenderResult,
 } from '@testing-library/react/pure';
+import { userEvent } from '@testing-library/user-event';
 import { useFakeTimers } from 'sinon';
 
 interface Interaction {
@@ -268,6 +269,7 @@ interface ServerRenderConfiguration extends RenderConfiguration {
 export type RenderOptions = Partial<RenderConfiguration>;
 
 export interface MuiRenderResult extends RenderResult<typeof queries & typeof customQueries> {
+  user: ReturnType<typeof userEvent.setup>;
   forceUpdate(): void;
   /**
    * convenience helper. Better than repeating all props.
@@ -296,6 +298,7 @@ function render(
   );
   const result: MuiRenderResult = {
     ...testingLibraryRenderResult,
+    user: userEvent.setup(),
     forceUpdate() {
       traceSync('forceUpdate', () =>
         testingLibraryRenderResult.rerender(

packages-internal/test-utils/src/focusVisible.ts

@@ -14,6 +14,10 @@ export function simulatePointerDevice() {
   fireEvent.pointerDown(document.body);
 }
 
+export function simulateKeyboardDevice() {
+  fireEvent.keyDown(document.body, { key: 'TAB' });
+}
+
 /**
  * See https://bugs.chromium.org/p/chromium/issues/detail?id=1127875 for more details.
  */

packages-internal/test-utils/src/index.ts

@@ -8,6 +8,7 @@ export * from './createRenderer';
 export {
   default as focusVisible,
   simulatePointerDevice,
+  simulateKeyboardDevice,
   programmaticFocusTriggersFocusVisible,
 } from './focusVisible';
 export {} from './initMatchers';

packages/mui-joy/src/MenuItem/MenuItem.test.tsx

@@ -92,18 +92,18 @@ describe('Joy <MenuItem />', () => {
     ];
 
     events.forEach((eventName) => {
-      it(`should fire ${eventName}`, () => {
+      it(`should fire ${eventName}`, async () => {
         const handlerName = `on${eventName[0].toUpperCase()}${eventName.slice(1)}`;
         const handler = spy();
         render(<MenuItem {...{ [handlerName]: handler }} />);
 
-        fireEvent[eventName](screen.getByRole('menuitem'));
+        await act(async () => fireEvent[eventName](screen.getByRole('menuitem')));
 
         expect(handler.callCount).to.equal(1);
       });
     });
 
-    it(`should fire focus, keydown, keyup and blur`, () => {
+    it(`should fire focus, keydown, keyup and blur`, async () => {
       const handleFocus = spy();
       const handleKeyDown = spy();
       const handleKeyUp = spy();
@@ -119,17 +119,17 @@ describe('Joy <MenuItem />', () => {
       );
       const menuitem = screen.getByRole('menuitem');
 
-      act(() => {
+      await act(async () => {
         menuitem.focus();
       });
 
       expect(handleFocus.callCount).to.equal(1);
 
-      fireEvent.keyDown(menuitem);
+      await act(async () => fireEvent.keyDown(menuitem));
 
       expect(handleKeyDown.callCount).to.equal(1);
 
-      fireEvent.keyUp(menuitem);
+      await act(async () => fireEvent.keyUp(menuitem));
 
       expect(handleKeyUp.callCount).to.equal(1);
 

packages/mui-material/src/Button/Button.test.js

@@ -1,11 +1,12 @@
 import * as React from 'react';
 import { expect } from 'chai';
-import { act, createRenderer, fireEvent, screen } from '@mui/internal-test-utils';
+import { createRenderer, screen, simulateKeyboardDevice } from '@mui/internal-test-utils';
 import { ClassNames } from '@emotion/react';
 import { ThemeProvider, createTheme } from '@mui/material/styles';
 import Button, { buttonClasses as classes } from '@mui/material/Button';
 import ButtonBase, { touchRippleClasses } from '@mui/material/ButtonBase';
 import describeConformance from '../../test/describeConformance';
+import * as ripple from '../../test/ripple';
 
 describe('<Button />', () => {
   const { render, renderToString } = createRenderer();
@@ -555,23 +556,23 @@ describe('<Button />', () => {
     expect(endIcon).not.to.have.class(classes.startIcon);
   });
 
-  it('should have a ripple by default', () => {
+  it('should have a ripple', async () => {
     const { getByRole } = render(
       <Button TouchRippleProps={{ className: 'touch-ripple' }}>Hello World</Button>,
     );
     const button = getByRole('button');
-
+    await ripple.startTouch(button);
     expect(button.querySelector('.touch-ripple')).not.to.equal(null);
   });
 
-  it('can disable the ripple', () => {
+  it('can disable the ripple', async () => {
     const { getByRole } = render(
       <Button disableRipple TouchRippleProps={{ className: 'touch-ripple' }}>
         Hello World
       </Button>,
     );
     const button = getByRole('button');
-
+    await ripple.startTouch(button);
     expect(button.querySelector('.touch-ripple')).to.equal(null);
   });
 
@@ -582,7 +583,7 @@ describe('<Button />', () => {
     expect(button).to.have.class(classes.disableElevation);
   });
 
-  it('should have a focusRipple by default', function test() {
+  it('should have a focusRipple', async function test() {
     if (/jsdom/.test(window.navigator.userAgent)) {
       // JSDOM doesn't support :focus-visible
       this.skip();
@@ -595,15 +596,13 @@ describe('<Button />', () => {
     );
     const button = getByRole('button');
 
-    fireEvent.keyDown(document.body, { key: 'TAB' });
-    act(() => {
-      button.focus();
-    });
+    simulateKeyboardDevice();
+    await ripple.startFocus(button);
 
     expect(button.querySelector('.pulsate-focus-visible')).not.to.equal(null);
   });
 
-  it('can disable the focusRipple', function test() {
+  it('can disable the focusRipple', async function test() {
     if (/jsdom/.test(window.navigator.userAgent)) {
       // JSDOM doesn't support :focus-visible
       this.skip();
@@ -619,10 +618,8 @@ describe('<Button />', () => {
     );
     const button = getByRole('button');
 
-    act(() => {
-      fireEvent.keyDown(document.body, { key: 'TAB' });
-      button.focus();
-    });
+    simulateKeyboardDevice();
+    await ripple.startFocus(button);
 
     expect(button.querySelector('.pulsate-focus-visible')).to.equal(null);
   });
@@ -676,7 +673,7 @@ describe('<Button />', () => {
     expect(container.firstChild.querySelector(`.${touchRippleClasses.root}`)).to.equal(null);
   });
 
-  it("should disable ripple when MuiButtonBase has disableRipple in theme's defaultProps but override on the individual Buttons if provided", () => {
+  it("should disable ripple when MuiButtonBase has disableRipple in theme's defaultProps but override on the individual Buttons if provided", async () => {
     const theme = createTheme({
       components: {
         MuiButtonBase: {
@@ -693,6 +690,7 @@ describe('<Button />', () => {
         <Button>Disabled ripple 2</Button>
       </ThemeProvider>,
     );
+    await ripple.startTouch(container.querySelector('button'));
     expect(container.querySelectorAll(`.${touchRippleClasses.root}`)).to.have.length(1);
   });
 

packages/mui-material/src/ButtonBase/ButtonBase.js

@@ -10,6 +10,7 @@ import { styled } from '../zero-styled';
 import { useDefaultProps } from '../DefaultPropsProvider';
 import useForkRef from '../utils/useForkRef';
 import useEventCallback from '../utils/useEventCallback';
+import useLazyRipple from '../useLazyRipple';
 import TouchRipple from './TouchRipple';
 import buttonBaseClasses, { getButtonBaseUtilityClass } from './buttonBaseClasses';
 
@@ -109,8 +110,8 @@ const ButtonBase = React.forwardRef(function ButtonBase(inProps, ref) {
 
   const buttonRef = React.useRef(null);
 
-  const rippleRef = React.useRef(null);
-  const handleRippleRef = useForkRef(rippleRef, touchRippleRef);
+  const ripple = useLazyRipple();
+  const handleRippleRef = useForkRef(ripple.ref, touchRippleRef);
 
   const [focusVisible, setFocusVisible] = React.useState(false);
   if (disabled && focusVisible) {
@@ -128,19 +129,13 @@ const ButtonBase = React.forwardRef(function ButtonBase(inProps, ref) {
     [],
   );
 
-  const [mountedState, setMountedState] = React.useState(false);
+  const enableTouchRipple = ripple.shouldMount && !disableRipple && !disabled;
 
   React.useEffect(() => {
-    setMountedState(true);
-  }, []);
-
-  const enableTouchRipple = mountedState && !disableRipple && !disabled;
-
-  React.useEffect(() => {
-    if (focusVisible && focusRipple && !disableRipple && mountedState) {
-      rippleRef.current.pulsate();
+    if (focusVisible && focusRipple && !disableRipple) {
+      ripple.pulsate();
     }
-  }, [disableRipple, focusRipple, focusVisible, mountedState]);
+  }, [disableRipple, focusRipple, focusVisible, ripple]);
 
   function useRippleHandler(rippleAction, eventCallback, skipRippleAction = disableTouchRipple) {
     return useEventCallback((event) => {
@@ -149,8 +144,8 @@ const ButtonBase = React.forwardRef(function ButtonBase(inProps, ref) {
       }
 
       const ignore = skipRippleAction;
-      if (!ignore && rippleRef.current) {
-        rippleRef.current[rippleAction](event);
+      if (!ignore) {
+        ripple[rippleAction](event);
       }
 
       return true;
@@ -212,9 +207,9 @@ const ButtonBase = React.forwardRef(function ButtonBase(inProps, ref) {
 
   const handleKeyDown = useEventCallback((event) => {
     // Check if key is already down to avoid repeats being counted as multiple activations
-    if (focusRipple && !event.repeat && focusVisible && rippleRef.current && event.key === ' ') {
-      rippleRef.current.stop(event, () => {
-        rippleRef.current.start(event);
+    if (focusRipple && !event.repeat && focusVisible && event.key === ' ') {
+      ripple.stop(event, () => {
+        ripple.start(event);
       });
     }
 
@@ -243,15 +238,9 @@ const ButtonBase = React.forwardRef(function ButtonBase(inProps, ref) {
   const handleKeyUp = useEventCallback((event) => {
     // calling preventDefault in keyUp on a <button> will not dispatch a click event if Space is pressed
     // https://codesandbox.io/p/sandbox/button-keyup-preventdefault-dn7f0
-    if (
-      focusRipple &&
-      event.key === ' ' &&
-      rippleRef.current &&
-      focusVisible &&
-      !event.defaultPrevented
-    ) {
-      rippleRef.current.stop(event, () => {
-        rippleRef.current.pulsate(event);
+    if (focusRipple && event.key === ' ' && focusVisible && !event.defaultPrevented) {
+      ripple.stop(event, () => {
+        ripple.pulsate(event);
       });
     }
     if (onKeyUp) {
@@ -291,20 +280,6 @@ const ButtonBase = React.forwardRef(function ButtonBase(inProps, ref) {
 
   const handleRef = useForkRef(ref, buttonRef);
 
-  if (process.env.NODE_ENV !== 'production') {
-    // eslint-disable-next-line react-hooks/rules-of-hooks
-    React.useEffect(() => {
-      if (enableTouchRipple && !rippleRef.current) {
-        console.error(
-          [
-            'MUI: The `component` prop provided to ButtonBase is invalid.',
-            'Please make sure the children prop is rendered in this custom component.',
-          ].join('\n'),
-        );
-      }
-    }, [enableTouchRipple]);
-  }
-
   const ownerState = {
     ...props,
     centerRipple,
@@ -345,7 +320,6 @@ const ButtonBase = React.forwardRef(function ButtonBase(inProps, ref) {
     >
       {children}
       {enableTouchRipple ? (
-        /* TouchRipple is only needed client-side, x2 boost on the server. */
         <TouchRipple ref={handleRippleRef} center={centerRipple} {...TouchRippleProps} />
       ) : null}
     </ButtonBaseRoot>