fixed issue in Modal component with nested interactive element

client/.eslintrc

@@ -1,6 +1,6 @@
 {
   "plugins": ["flowtype", "jest", "react", "ft-flow"],
-  "extends": ["airbnb", "plugin:flowtype/recommended"],
+  "extends": ["airbnb", "plugin:flowtype/recommended", "plugin:react-hooks/recommended"],
   "parser": "@babel/eslint-parser",
   "parserOptions": {
     "requireConfigFile": false

client/app/components/Input/InputTextarea.jsx

@@ -117,7 +117,7 @@ export function InputTextarea(props: Props): Node {
       });
       editor.current.content.innerHTML = value;
     }
-  }, []);
+  }, [value]);
 
   return (
     <div

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();

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,25 +154,51 @@ export const Modal = (props: Props): Node => {
     }
   };
 
-  const resolveElement = () => {
-    let renderComponent;
+  useEffect(() => {
     if (element && element.component) {
-      const { component, props: elementProps } = element;
-      renderComponent = React.createElement(resolveComponent(component), {
-        ...elementProps,
-      });
+      const { props: elementProps } = element;
+      setIsInteractive(!!elementProps.onClick);
+    } else if (
+      element
+      && typeof element === 'object'
+      && element.type === 'button'
+    ) {
+      setIsInteractive(true);
     }
+  }, [element]);
+
+  const resolveElement = () => {
     if (element) {
       return (
         <div
           id={elementId}
           className={`modalElement ${css.modalElement} ${className || ''}`}
-          onClick={toggleOpen}
-          onKeyDown={(event) => handleKeyPress(event, 'Enter')}
-          role="button"
-          tabIndex={0}
+          role={!isInteractive ? 'button' : undefined}
+          tabIndex={!isInteractive ? 0 : undefined}
+          onClick={!isInteractive ? toggleOpen : undefined}
+          onKeyDown={
+            !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>
       );
     }

client/app/components/Toast/__tests__/Toast.spec.jsx

@@ -9,20 +9,14 @@ describe('Toast', () => {
   describe('Toast Type: Alert', () => {
     it('renders correctly', () => {
       render(
-        <Toast
-          alert="Invalid username or password."
-          appendDashboardClass="true"
-        />,
+        <Toast alert="Invalid username or password." appendDashboardClass />,
       );
       expect(screen).not.toBeNull();
     });
 
     it('closes correctly on button click', () => {
       const { getByRole, container } = render(
-        <Toast
-          alert="Invalid username or password."
-          appendDashboardClass="true"
-        />,
+        <Toast alert="Invalid username or password." appendDashboardClass />,
       );
 
       const toastContent = getByRole('alert');
@@ -35,10 +29,7 @@ describe('Toast', () => {
 
     it('closes automatically after 7 seconds', async () => {
       const { getByRole } = render(
-        <Toast
-          alert="Invalid username or password."
-          appendDashboardClass="true"
-        />,
+        <Toast alert="Invalid username or password." appendDashboardClass />,
       );
 
       const toastContent = getByRole('alert');

client/app/components/Toast/index.jsx

@@ -1,5 +1,5 @@
 // @flow
-import React, { useState } from 'react';
+import React, { useEffect, useState } from 'react';
 import type { Node } from 'react';
 import { I18n } from 'libs/i18n';
 import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
@@ -32,12 +32,20 @@ export const Toast = ({ alert, notice, appendDashboardClass }: Props): Node => {
   const hideAlert = () => {
     setShowAlert(false);
   };
-  if (showAlert || showNotice) {
-    setTimeout(() => {
-      hideNotice();
-      hideAlert();
-    }, 7000);
-  }
+
+  useEffect(() => {
+    let timer;
+    if (showAlert || showNotice) {
+      timer = setTimeout(() => {
+        hideNotice();
+        hideAlert();
+      }, 7000);
+    }
+    return () => {
+      clearTimeout(timer);
+    };
+  }, [showAlert, showNotice]);
+
   return (
     <>
       <div

client/app/hooks/useFocusTrap.js

@@ -5,40 +5,43 @@ export const useFocusTrap = (
   ref: { current: null | HTMLElement },
   isOpen: boolean,
 ) => {
-  const handleKeyDown = (e: any) => {
-    const focusableElements = 'button, [href], input, select, textarea, [tabindex]:not([tabindex="-1"])';
-    const element = ref.current;
-    if (!element) return;
+  useEffect(() => {
+    const handleKeyDown = (e: any) => {
+      const focusableElements = 'button, [href], input, select, textarea, [tabindex]:not([tabindex="-1"])';
+      const element = ref.current;
+      if (!element) return;
 
-    const focusableContent = element.querySelectorAll(focusableElements);
-    const visibleContent = Array.from(focusableContent).filter(
-      (node) => window.getComputedStyle(node).display !== 'none',
-    );
+      const focusableContent = element.querySelectorAll(focusableElements);
+      const visibleContent = Array.from(focusableContent).filter(
+        (node) => window.getComputedStyle(node).display !== 'none',
+      );
 
-    const firstFocusableElement = visibleContent[0];
-    const lastFocusableElement = visibleContent[visibleContent.length - 1];
-    const isTabPressed = e.key === 'Tab' || e.keyCode === 9;
+      const firstFocusableElement = visibleContent[0];
+      const lastFocusableElement = visibleContent[visibleContent.length - 1];
+      const isTabPressed = e.key === 'Tab' || e.keyCode === 9;
 
-    if (!isTabPressed) {
-      return;
-    }
+      if (!isTabPressed) {
+        return;
+      }
 
-    if (e.shiftKey && document.activeElement === firstFocusableElement) {
-      lastFocusableElement.focus();
-      e.preventDefault();
-    } else if (!e.shiftKey && document.activeElement === lastFocusableElement) {
-      firstFocusableElement.focus();
-      e.preventDefault();
-    }
-  };
+      if (e.shiftKey && document.activeElement === firstFocusableElement) {
+        lastFocusableElement.focus();
+        e.preventDefault();
+      } else if (
+        !e.shiftKey
+        && document.activeElement === lastFocusableElement
+      ) {
+        firstFocusableElement.focus();
+        e.preventDefault();
+      }
+    };
 
-  useEffect(() => {
     if (isOpen) {
       document.addEventListener('keydown', handleKeyDown);
     }
 
     return () => {
       document.removeEventListener('keydown', handleKeyDown);
     };
-  }, [isOpen]);
+  }, [isOpen, ref]);
 };

client/app/stories/Toast.stories.jsx

@@ -13,7 +13,7 @@ export const noticeToast = Template.bind({});
 
 noticeToast.args = {
   notice: 'Login successful.',
-  appendDashboardClass: 'true',
+  appendDashboardClass: true,
 };
 noticeToast.storyName = 'Toast Type: Notice';
 

client/app/utils/__tests__/index.spec.jsx

@@ -106,7 +106,11 @@ describe('Utils', () => {
     });
 
     it('should apply attributes to React elements', () => {
-      const reactElement = React.createElement('div', { className: 'test' }, 'Test content');
+      const reactElement = React.createElement(
+        'div',
+        { className: 'test' },
+        'Test content',
+      );
       const attributes = { 'aria-labelledby': 'test-id' };
       const renderedContent = Utils.renderContent(reactElement, attributes);
       expect(renderedContent.props['aria-labelledby']).toEqual('test-id');

client/app/widgets/Comments/__tests__/Comments.spec.jsx

@@ -87,7 +87,9 @@ describe('Comments', () => {
         screen.getByRole('button', { name: 'Submit' }),
       ).toBeInTheDocument();
       expect(
-        screen.getByLabelText(`Report comment by ${getComment().commentByName}`),
+        screen.getByLabelText(
+          `Report comment by ${getComment().commentByName}`,
+        ),
       ).toBeInTheDocument();
     });
 
@@ -109,7 +111,9 @@ describe('Comments', () => {
       expect(screen.getByRole('article')).toHaveTextContent('Hey');
 
       await userEvent.click(
-        screen.getByLabelText(`Delete comment by ${getComment().commentByName}`),
+        screen.getByLabelText(
+          `Delete comment by ${getComment().commentByName}`,
+        ),
       );
 
       await waitFor(() => expect(screen.queryByRole('article')).not.toBeInTheDocument());
@@ -152,7 +156,9 @@ describe('Comments', () => {
       expect(screen.getByRole('article')).toHaveTextContent('Hey');
 
       await userEvent.click(
-        screen.getByLabelText(`Delete comment by ${getComment().commentByName}`),
+        screen.getByLabelText(
+          `Delete comment by ${getComment().commentByName}`,
+        ),
       );
 
       await waitFor(() => expect(screen.queryByRole('article')).not.toBeInTheDocument());

client/app/widgets/Notifications/__tests__/Notifications.spec.jsx

@@ -10,21 +10,22 @@ const button = <button type="button">Notifications</button>;
 
 describe('Notifications', () => {
   it('gets notifications and clears them', async (done) => {
-    jest
+    const axiosGetSpy = jest
       .spyOn(axios, 'get')
       .mockReturnValueOnce(Promise.resolve({ data: { signed_in: 1 } }))
       .mockReturnValueOnce(
         Promise.resolve({ data: { fetch_notifications: ['Hello'] } }),
-      );
+      )
+      .mockReturnValue(Promise.resolve());
     jest
       .spyOn(axios, 'delete')
       .mockReturnValue(Promise.resolve({ data: { ok: true } }));
     render(<Notifications element={button} />);
-    await waitFor(() => expect(axios.get).toHaveBeenCalledWith('/notifications/signed_in'));
-    await waitFor(() => expect(axios.get).toHaveBeenCalledWith(
+    await waitFor(() => expect(axiosGetSpy).toHaveBeenCalledWith('/notifications/signed_in'));
+    await waitFor(() => expect(axiosGetSpy).toHaveBeenCalledWith(
       '/notifications/fetch_notifications',
     ));
-    expect(axios.get).toHaveBeenCalledTimes(2);
+    expect(axiosGetSpy).toHaveBeenCalledTimes(3);
     expect(screen.getByText('Hello')).toBeInTheDocument();
     fireEvent.click(screen.getByText('Clear'));
     await waitFor(() => {