[EuiModal] Temporary workaround for scroll-jumping behavior (#6327)

* Add removable workaround until react-focus-on supports focusOptions * unit tests - convert modal tests to RTL; add removable scroll/focus test * [misc bugfix] Remove body width & scroll jumping occurring in Chrome - use `react-focus-on`'s scrollLock API to prevent this (they manage preserving the width of the scrollbar for us) * changelog
elastic · Oct 28, 2022 · bdea710 · bdea710
1 parent dd580b1
commit bdea710
Show file tree

Hide file tree

Showing 4 changed files with 108 additions and 51 deletions.
diff --git a/src/components/modal/__snapshots__/modal.test.tsx.snap b/src/components/modal/__snapshots__/modal.test.tsx.snap
@@ -1,51 +1,61 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`renders EuiModal 1`] = `
-Array [
+exports[`EuiModal renders 1`] = `
+<body>
   <div
     aria-hidden="true"
     data-aria-hidden="true"
-    data-focus-guard="true"
-    style="width: 1px; height: 0px; padding: 0px; overflow: hidden; position: fixed; top: 1px; left: 1px;"
-    tabindex="0"
-  />,
+  />
   <div
-    aria-hidden="true"
-    data-aria-hidden="true"
-    data-focus-guard="true"
-    style="width: 1px; height: 0px; padding: 0px; overflow: hidden; position: fixed; top: 1px; left: 1px;"
-    tabindex="1"
-  />,
-  <div
-    data-focus-lock-disabled="false"
+    class="euiOverlayMask emotion-euiOverlayMask-aboveHeader"
+    data-euiportal="true"
+    data-relative-to-header="above"
   >
     <div
-      aria-label="aria-label"
-      class="euiModal euiModal--maxWidth-default testClass1 testClass2"
-      data-test-subj="test subject string"
+      aria-hidden="true"
+      data-aria-hidden="true"
+      data-focus-guard="true"
+      style="width: 1px; height: 0px; padding: 0px; overflow: hidden; position: fixed; top: 1px; left: 1px;"
       tabindex="0"
+    />
+    <div
+      aria-hidden="true"
+      data-aria-hidden="true"
+      data-focus-guard="true"
+      style="width: 1px; height: 0px; padding: 0px; overflow: hidden; position: fixed; top: 1px; left: 1px;"
+      tabindex="1"
+    />
+    <div
+      data-focus-lock-disabled="false"
     >
-      <button
-        aria-label="Closes this modal window"
-        class="euiButtonIcon euiButtonIcon--xSmall euiModal__closeIcon emotion-euiButtonIcon-empty-text-hoverStyles"
-        type="button"
+      <div
+        aria-label="aria-label"
+        class="euiModal euiModal--maxWidth-default testClass1 testClass2"
+        data-test-subj="test subject string"
+        tabindex="0"
       >
-        <span
-          aria-hidden="true"
-          class="euiButtonIcon__icon"
-          color="inherit"
-          data-euiicon-type="cross"
-        />
-      </button>
-      children
+        <button
+          aria-label="Closes this modal window"
+          class="euiButtonIcon euiButtonIcon--xSmall euiModal__closeIcon emotion-euiButtonIcon-empty-text-hoverStyles"
+          type="button"
+        >
+          <span
+            aria-hidden="true"
+            class="euiButtonIcon__icon"
+            color="inherit"
+            data-euiicon-type="cross"
+          />
+        </button>
+        children
+      </div>
     </div>
-  </div>,
-  <div
-    aria-hidden="true"
-    data-aria-hidden="true"
-    data-focus-guard="true"
-    style="width: 1px; height: 0px; padding: 0px; overflow: hidden; position: fixed; top: 1px; left: 1px;"
-    tabindex="0"
-  />,
-]
+    <div
+      aria-hidden="true"
+      data-aria-hidden="true"
+      data-focus-guard="true"
+      style="width: 1px; height: 0px; padding: 0px; overflow: hidden; position: fixed; top: 1px; left: 1px;"
+      tabindex="0"
+    />
+  </div>
+</body>
 `;
diff --git a/src/components/modal/modal.test.tsx b/src/components/modal/modal.test.tsx
@@ -7,19 +7,44 @@
  */
 
 import React from 'react';
-import { mount } from 'enzyme';
-import { requiredProps, takeMountedSnapshot } from '../../test';
+import { fireEvent } from '@testing-library/dom';
+import { render } from '../../test/rtl';
+import { requiredProps } from '../../test';
 
 import { EuiModal } from './modal';
 
-test('renders EuiModal', () => {
-  const component = (
-    <EuiModal onClose={() => {}} {...requiredProps}>
-      children
-    </EuiModal>
-  );
+describe('EuiModal', () => {
+  it('renders', () => {
+    const { baseElement } = render(
+      <EuiModal onClose={() => {}} {...requiredProps}>
+        children
+      </EuiModal>
+    );
 
-  expect(
-    takeMountedSnapshot(mount(component), { hasArrayOutput: true })
-  ).toMatchSnapshot();
+    // NOTE: Using baseElement instead of container is required for components that use portals
+    expect(baseElement).toMatchSnapshot();
+  });
+
+  // TODO: Remove this onFocus scroll workaround after react-focus-on supports focusOptions
+  // @see https://github.com/elastic/eui/issues/6304
+  describe('focus/scroll workaround', () => {
+    it('scrolls back to the original window position on initial modal focus', () => {
+      window.scrollTo = jest.fn();
+
+      const { getByTestSubject } = render(
+        <EuiModal data-test-subj="modal" onClose={() => {}}>
+          children
+        </EuiModal>
+      );
+
+      // For whatever reason, react-focus-lock doesn't appear to trigger focus in RTL so we'll do it manually
+      fireEvent.focusIn(getByTestSubject('modal'));
+      // Confirm that scrolling does not occur more than once
+      fireEvent.focusIn(getByTestSubject('modal'));
+      fireEvent.focusIn(getByTestSubject('modal'));
+
+      expect(window.scrollTo).toHaveBeenCalledTimes(1);
+      jest.restoreAllMocks();
+    });
+  });
 });
diff --git a/src/components/modal/modal.tsx b/src/components/modal/modal.tsx
@@ -6,7 +6,13 @@
  * Side Public License, v 1.
  */
 
-import React, { FunctionComponent, ReactNode, HTMLAttributes } from 'react';
+import React, {
+  FunctionComponent,
+  ReactNode,
+  HTMLAttributes,
+  useRef,
+  useCallback,
+} from 'react';
 import classnames from 'classnames';
 
 import { keys } from '../../services';
@@ -52,6 +58,18 @@ export const EuiModal: FunctionComponent<EuiModalProps> = ({
   style,
   ...rest
 }) => {
+  // TODO: Remove this onFocus scroll workaround after react-focus-on supports focusOptions
+  // @see https://github.com/elastic/eui/issues/6304
+  const bodyScrollTop = useRef<undefined | number>(
+    typeof window === 'undefined' ? undefined : window.scrollY // Account for SSR
+  );
+  const onFocus = useCallback(() => {
+    if (bodyScrollTop.current != null) {
+      window.scrollTo({ top: bodyScrollTop.current });
+      bodyScrollTop.current = undefined; // Unset after first auto focus
+    }
+  }, []);
+
   const onKeyDown = (event: React.KeyboardEvent<HTMLDivElement>) => {
     if (event.key === keys.ESCAPE) {
       event.preventDefault();
@@ -73,7 +91,7 @@ export const EuiModal: FunctionComponent<EuiModalProps> = ({
 
   return (
     <EuiOverlayMask>
-      <EuiFocusTrap initialFocus={initialFocus}>
+      <EuiFocusTrap initialFocus={initialFocus} scrollLock>
         {
           // Create a child div instead of applying these props directly to FocusTrap, or else
           // fallbackFocus won't work.
@@ -82,6 +100,7 @@ export const EuiModal: FunctionComponent<EuiModalProps> = ({
           className={classes}
           onKeyDown={onKeyDown}
           tabIndex={0}
+          onFocus={onFocus}
           style={newStyle || style}
           {...rest}
         >

diff --git a/upcoming_changelogs/6327.md b/upcoming_changelogs/6327.md
@@ -0,0 +1,3 @@
+**Bug fixes**
+
+- Temporarily patched `EuiModal` to not cause scroll-jumping issues on modal open