Fix issues with Modal component and update tests

ifmeorg · Nov 29, 2024 · ee50bcc · ee50bcc
1 parent a4ecbae
commit ee50bcc
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 23 deletions.
diff --git a/client/app/components/Modal/__tests__/Modal.spec.jsx b/client/app/components/Modal/__tests__/Modal.spec.jsx
@@ -112,9 +112,7 @@ describe('Modal', () => {
         ).not.toBeInTheDocument();
         expect(screen.queryByRole('dialog')).toBeNull();
 
-        await userEvent.click(
-          screen.getAllByRole('button', { name: 'Hello' })[1],
-        );
+        await userEvent.click(screen.getByRole('button', { name: 'Hello' }));
         expect(container.querySelector('.modalBackdrop')).toBeInTheDocument();
         expect(screen.getByRole('dialog')).toBeInTheDocument();
 
@@ -142,9 +140,7 @@ describe('Modal', () => {
         ).not.toBeInTheDocument();
         expect(screen.queryByRole('dialog')).toBeNull();
 
-        await userEvent.click(
-          screen.getAllByRole('button', { name: 'Hello' })[1],
-        );
+        await userEvent.click(screen.getByRole('button', { name: 'Hello' }));
         expect(window.alert).toHaveBeenCalledWith("Hey look it's listening");
         expect(container.querySelector('.modalBackdrop')).toBeInTheDocument();
         expect(screen.getByRole('dialog')).toBeInTheDocument();
@@ -324,9 +320,7 @@ describe('Modal', () => {
         ).not.toBeInTheDocument();
         expect(screen.queryByRole('dialog')).toBeNull();
 
-        await userEvent.click(
-          screen.getAllByRole('button', { name: 'Hello' })[1],
-        );
+        await userEvent.click(screen.getByRole('button', { name: 'Hello' }));
         expect(container.querySelector('.modalBackdrop')).toBeInTheDocument();
         expect(screen.getByRole('dialog')).toBeInTheDocument();
       });
@@ -353,9 +347,7 @@ describe('Modal', () => {
         ).not.toBeInTheDocument();
         expect(screen.queryByRole('dialog')).toBeNull();
 
-        await userEvent.click(
-          screen.getAllByRole('button', { name: 'Hello' })[1],
-        );
+        await userEvent.click(screen.getByRole('button', { name: 'Hello' }));
         expect(window.alert).toHaveBeenCalledWith("Hey look it's listening");
         expect(container.querySelector('.modalBackdrop')).toBeInTheDocument();
         expect(screen.getByRole('dialog')).toBeInTheDocument();

diff --git a/client/app/components/Modal/index.jsx b/client/app/components/Modal/index.jsx
@@ -50,6 +50,7 @@ export const Modal = (props: Props): Node => {
   } = props;
 
   const [open, setOpen] = useState(!!openProps);
+  const [isInteractive, setIsInteractive] = useState(false);
   const [modalHasFocus, setModalHasFocus] = useState(true);
   const modalEl = useRef(null);
 
@@ -153,24 +154,20 @@ export const Modal = (props: Props): Node => {
     }
   };
 
-  const resolveElement = () => {
-    let renderComponent;
-    let isInteractive = false;
-
+  useEffect(() => {
     if (element && element.component) {
-      const { component, props: elementProps } = element;
-      renderComponent = React.createElement(resolveComponent(component), {
-        ...elementProps,
-      });
-      isInteractive = !!elementProps.onClick || !!elementProps.role;
+      const { props: elementProps } = element;
+      setIsInteractive(!!elementProps.onClick);
     } else if (
       element
       && typeof element === 'object'
       && element.type === 'button'
     ) {
-      isInteractive = true;
+      setIsInteractive(true);
     }
+  }, [element]);
 
+  const resolveElement = () => {
     if (element) {
       return (
         <div
@@ -183,7 +180,25 @@ export const Modal = (props: Props): Node => {
             !isInteractive ? (event) => handleKeyPress(event, 'Enter') : undefined
           }
         >
-          {renderComponent || Utils.renderContent(element)}
+          {element && element.component
+            ? React.createElement(resolveComponent(element.component), {
+              ...element.props,
+              tabIndex: !isInteractive ? 0 : undefined,
+              onClick: !isInteractive ? toggleOpen : undefined,
+              onKeyDown: !isInteractive
+                ? (event) => handleKeyPress(event, 'Enter')
+                : undefined,
+            })
+            : Utils.renderContent(
+              element,
+              typeof element === 'object' && element.type === 'button'
+                ? {
+                  tabIndex: 0,
+                  onClick: toggleOpen,
+                  onKeyDown: (event) => handleKeyPress(event, 'Enter'),
+                }
+                : {},
+            )}
         </div>
       );
     }