From 130a43f5c47fb087a99b6b2c40b0b4bec8404106 Mon Sep 17 00:00:00 2001
From: mabashian <mabashia@redhat.com>
Date: Thu, 10 Sep 2020 15:31:32 -0400
Subject: [PATCH] Simplify kebab modal open logic

---
 .../DataListToolbar/DataListToolbar.jsx       | 37 ++++++++++---------
 .../JobList/JobListCancelButton.jsx           | 11 ++++--
 .../PaginatedDataList/ToolbarDeleteButton.jsx | 11 ++++--
 .../ToolbarDeleteButton.test.jsx              |  9 ++---
 .../TemplateList/TemplateList.test.jsx        |  8 +++-
 5 files changed, 43 insertions(+), 33 deletions(-)

diff --git a/awx/ui_next/src/components/DataListToolbar/DataListToolbar.jsx b/awx/ui_next/src/components/DataListToolbar/DataListToolbar.jsx
index 79e83bda77..f5137d905b 100644
--- a/awx/ui_next/src/components/DataListToolbar/DataListToolbar.jsx
+++ b/awx/ui_next/src/components/DataListToolbar/DataListToolbar.jsx
@@ -1,4 +1,4 @@
-import React, { useState } from 'react';
+import React, { useEffect, useState } from 'react';
 import PropTypes from 'prop-types';
 import { withI18n } from '@lingui/react';
 import { t } from '@lingui/macro';
@@ -42,15 +42,21 @@ function DataListToolbar({
   pagination,
 }) {
   const showExpandCollapse = onCompact && onExpand;
-  const [kebabIsOpen, setKebabIsOpen] = useState(false);
-  const [kebabModalIsOpen, setKebabModalIsOpen] = useState(false);
-  const [advancedSearchShown, setAdvancedSearchShown] = useState(false);
+  const [isKebabOpen, setIsKebabOpen] = useState(false);
+  const [isKebabModalOpen, setIsKebabModalOpen] = useState(false);
+  const [isAdvancedSearchShown, setIsAdvancedSearchShown] = useState(false);
 
   const onShowAdvancedSearch = shown => {
-    setAdvancedSearchShown(shown);
-    setKebabIsOpen(false);
+    setIsAdvancedSearchShown(shown);
+    setIsKebabOpen(false);
   };
 
+  useEffect(() => {
+    if (!isKebabModalOpen) {
+      setIsKebabOpen(false);
+    }
+  }, [isKebabModalOpen]);
+
   return (
     <Toolbar
       id={`${qsConfig.namespace}-list-toolbar`}
@@ -103,44 +109,39 @@ function DataListToolbar({
             </>
           </ToolbarGroup>
         )}
-        {advancedSearchShown && (
+        {isAdvancedSearchShown && (
           <ToolbarItem>
             <KebabifiedProvider
               value={{
                 isKebabified: true,
-                onKebabModalChange: isOpen => {
-                  if (kebabIsOpen && kebabModalIsOpen && !isOpen) {
-                    setKebabIsOpen(false);
-                  }
-                  setKebabModalIsOpen(isOpen);
-                },
+                onKebabModalChange: setIsKebabModalOpen,
               }}
             >
               <Dropdown
                 toggle={
                   <KebabToggle
                     onToggle={isOpen => {
-                      if (!kebabModalIsOpen) {
-                        setKebabIsOpen(isOpen);
+                      if (!isKebabModalOpen) {
+                        setIsKebabOpen(isOpen);
                       }
                     }}
                   />
                 }
-                isOpen={kebabIsOpen || kebabModalIsOpen}
+                isOpen={isKebabOpen}
                 isPlain
                 dropdownItems={additionalControls}
               />
             </KebabifiedProvider>
           </ToolbarItem>
         )}
-        {!advancedSearchShown && (
+        {!isAdvancedSearchShown && (
           <ToolbarGroup>
             {additionalControls.map(control => (
               <ToolbarItem key={control.key}>{control}</ToolbarItem>
             ))}
           </ToolbarGroup>
         )}
-        {!advancedSearchShown && pagination && itemCount > 0 && (
+        {!isAdvancedSearchShown && pagination && itemCount > 0 && (
           <ToolbarItem variant="pagination">{pagination}</ToolbarItem>
         )}
       </ToolbarContent>
diff --git a/awx/ui_next/src/components/JobList/JobListCancelButton.jsx b/awx/ui_next/src/components/JobList/JobListCancelButton.jsx
index 165b245638..684d088c96 100644
--- a/awx/ui_next/src/components/JobList/JobListCancelButton.jsx
+++ b/awx/ui_next/src/components/JobList/JobListCancelButton.jsx
@@ -1,4 +1,4 @@
-import React, { useContext, useState } from 'react';
+import React, { useContext, useEffect, useState } from 'react';
 import { withI18n } from '@lingui/react';
 import { t } from '@lingui/macro';
 import { arrayOf, func } from 'prop-types';
@@ -23,12 +23,15 @@ function JobListCancelButton({ i18n, jobsToCancel, onCancel }) {
   };
 
   const toggleModal = () => {
-    if (isKebabified) {
-      onKebabModalChange(!isModalOpen);
-    }
     setIsModalOpen(!isModalOpen);
   };
 
+  useEffect(() => {
+    if (isKebabified) {
+      onKebabModalChange(isModalOpen);
+    }
+  }, [isKebabified, isModalOpen, onKebabModalChange]);
+
   const renderTooltip = () => {
     const jobsUnableToCancel = jobsToCancel
       .filter(cannotCancel)
diff --git a/awx/ui_next/src/components/PaginatedDataList/ToolbarDeleteButton.jsx b/awx/ui_next/src/components/PaginatedDataList/ToolbarDeleteButton.jsx
index db77aa6174..25a280d549 100644
--- a/awx/ui_next/src/components/PaginatedDataList/ToolbarDeleteButton.jsx
+++ b/awx/ui_next/src/components/PaginatedDataList/ToolbarDeleteButton.jsx
@@ -1,4 +1,4 @@
-import React, { useContext, useState } from 'react';
+import React, { useContext, useEffect, useState } from 'react';
 import {
   func,
   bool,
@@ -75,12 +75,15 @@ function ToolbarDeleteButton({
   };
 
   const toggleModal = () => {
-    if (isKebabified) {
-      onKebabModalChange(!isModalOpen);
-    }
     setIsModalOpen(!isModalOpen);
   };
 
+  useEffect(() => {
+    if (isKebabified) {
+      onKebabModalChange(isModalOpen);
+    }
+  }, [isKebabified, isModalOpen, onKebabModalChange]);
+
   const renderTooltip = () => {
     const itemsUnableToDelete = itemsToDelete
       .filter(cannotDelete)
diff --git a/awx/ui_next/src/components/PaginatedDataList/ToolbarDeleteButton.test.jsx b/awx/ui_next/src/components/PaginatedDataList/ToolbarDeleteButton.test.jsx
index 843ffa5b4a..c85730226c 100644
--- a/awx/ui_next/src/components/PaginatedDataList/ToolbarDeleteButton.test.jsx
+++ b/awx/ui_next/src/components/PaginatedDataList/ToolbarDeleteButton.test.jsx
@@ -26,8 +26,8 @@ describe('<ToolbarDeleteButton />', () => {
     const wrapper = mountWithContexts(
       <ToolbarDeleteButton onDelete={() => {}} itemsToDelete={[itemA]} />
     );
+    expect(wrapper.find('Modal')).toHaveLength(0);
     wrapper.find('button').simulate('click');
-    expect(wrapper.find('ToolbarDeleteButton').state('isModalOpen')).toBe(true);
     wrapper.update();
     expect(wrapper.find('Modal')).toHaveLength(1);
   });
@@ -37,15 +37,14 @@ describe('<ToolbarDeleteButton />', () => {
     const wrapper = mountWithContexts(
       <ToolbarDeleteButton onDelete={onDelete} itemsToDelete={[itemA]} />
     );
-    wrapper.find('ToolbarDeleteButton').setState({ isModalOpen: true });
+    wrapper.find('button').simulate('click');
     wrapper.update();
     wrapper
       .find('ModalBoxFooter button[aria-label="confirm delete"]')
       .simulate('click');
+    wrapper.update();
     expect(onDelete).toHaveBeenCalled();
-    expect(wrapper.find('ToolbarDeleteButton').state('isModalOpen')).toBe(
-      false
-    );
+    expect(wrapper.find('Modal')).toHaveLength(0);
   });
 
   test('should disable button when no delete permissions', () => {
diff --git a/awx/ui_next/src/screens/Template/TemplateList/TemplateList.test.jsx b/awx/ui_next/src/screens/Template/TemplateList/TemplateList.test.jsx
index 377e87583e..2486ea5579 100644
--- a/awx/ui_next/src/screens/Template/TemplateList/TemplateList.test.jsx
+++ b/awx/ui_next/src/screens/Template/TemplateList/TemplateList.test.jsx
@@ -261,7 +261,9 @@ describe('<TemplateList />', () => {
       },
     });
 
-    wrapper.find('button[aria-label="Delete"]').prop('onClick')();
+    await act(async () => {
+      wrapper.find('button[aria-label="Delete"]').prop('onClick')();
+    });
     wrapper.update();
     await act(async () => {
       await wrapper
@@ -302,7 +304,9 @@ describe('<TemplateList />', () => {
         summary_fields: { user_capabilities: { delete: true } },
       },
     });
-    wrapper.find('button[aria-label="Delete"]').prop('onClick')();
+    await act(async () => {
+      wrapper.find('button[aria-label="Delete"]').prop('onClick')();
+    });
     wrapper.update();
     await act(async () => {
       await wrapper