From 0eb81072b5bee59ce672c120834cf10b317e9f20 Mon Sep 17 00:00:00 2001
From: Ron Lavi <1ronlavi@gmail.com>
Date: Wed, 27 Dec 2023 19:55:25 +0000
Subject: [PATCH] Inventory page UX enhancements

- Moved the page description into a modal which is reachable from the dropdown kebab.
- Moved also the settings card into a modal.
- Small margin/padding fixes.
- replace PF3 Buttons with PF4 Buttons.
---
 .../Components/AccountList/accountList.scss   |  5 ++++
 .../InventorySettings/InventorySettings.js    |  3 ---
 .../InventorySettings.test.js.snap            |  3 ---
 .../Components/PageHeader/PageHeader.js       |  6 -----
 .../Components/PageHeader/PageHeader.scss     |  1 -
 .../Components/PageHeader/PageTitle.js        | 26 +++++++++++++++++++
 .../__snapshots__/PageHeader.test.js.snap     |  6 -----
 .../__snapshots__/PageTitle.test.js.snap      | 20 ++++++++++++++
 .../PageHeader/components/AboutModal/index.js | 26 +++++++++++++++++++
 .../components/SettingsModal/index.js         | 26 +++++++++++++++++++
 .../Components/TabHeader/TabHeader.js         | 11 ++++----
 .../__snapshots__/TabHeader.test.js.snap      |  6 +----
 .../ForemanInventoryConstants.js              |  4 +++
 .../__snapshots__/InsightsTable.test.js.snap  |  3 +++
 webpack/common/Switcher/HelpLabel.js          |  2 +-
 webpack/common/Switcher/SwitcherPF4.scss      |  4 +++
 16 files changed, 122 insertions(+), 30 deletions(-)
 create mode 100644 webpack/ForemanInventoryUpload/Components/PageHeader/components/AboutModal/index.js
 create mode 100644 webpack/ForemanInventoryUpload/Components/PageHeader/components/SettingsModal/index.js

diff --git a/webpack/ForemanInventoryUpload/Components/AccountList/accountList.scss b/webpack/ForemanInventoryUpload/Components/AccountList/accountList.scss
index 82f25b7f..e42918d5 100644
--- a/webpack/ForemanInventoryUpload/Components/AccountList/accountList.scss
+++ b/webpack/ForemanInventoryUpload/Components/AccountList/accountList.scss
@@ -1,4 +1,9 @@
 .rh-cloud-inventory-page {
+
+  section {
+    padding-bottom: 0;
+  }
+
   #main {
     padding: 0;
 
diff --git a/webpack/ForemanInventoryUpload/Components/InventorySettings/InventorySettings.js b/webpack/ForemanInventoryUpload/Components/InventorySettings/InventorySettings.js
index 9e0d598f..463bf0fd 100644
--- a/webpack/ForemanInventoryUpload/Components/InventorySettings/InventorySettings.js
+++ b/webpack/ForemanInventoryUpload/Components/InventorySettings/InventorySettings.js
@@ -1,6 +1,4 @@
 import React from 'react';
-import { translate as __ } from 'foremanReact/common/I18n';
-
 import AdvancedSetting from './AdvancedSetting';
 import { settingsDict } from './AdvancedSetting/AdvancedSettingsConstants';
 
@@ -8,7 +6,6 @@ import './InventorySettings.scss';
 
 const InventorySettings = () => (
   <div className="inventory-settings">
-    <h3>{__('Settings')}</h3>
     {Object.keys(settingsDict).map(key => (
       <AdvancedSetting setting={key} key={key} />
     ))}
diff --git a/webpack/ForemanInventoryUpload/Components/InventorySettings/__tests__/__snapshots__/InventorySettings.test.js.snap b/webpack/ForemanInventoryUpload/Components/InventorySettings/__tests__/__snapshots__/InventorySettings.test.js.snap
index 6f5229a2..9af41cc5 100644
--- a/webpack/ForemanInventoryUpload/Components/InventorySettings/__tests__/__snapshots__/InventorySettings.test.js.snap
+++ b/webpack/ForemanInventoryUpload/Components/InventorySettings/__tests__/__snapshots__/InventorySettings.test.js.snap
@@ -4,9 +4,6 @@ exports[`InventorySettings rendering render without Props 1`] = `
 <div
   className="inventory-settings"
 >
-  <h3>
-    Settings
-  </h3>
   <AdvancedSetting
     key="autoUploadEnabled"
     setting="autoUploadEnabled"
diff --git a/webpack/ForemanInventoryUpload/Components/PageHeader/PageHeader.js b/webpack/ForemanInventoryUpload/Components/PageHeader/PageHeader.js
index d7327852..06633bb0 100644
--- a/webpack/ForemanInventoryUpload/Components/PageHeader/PageHeader.js
+++ b/webpack/ForemanInventoryUpload/Components/PageHeader/PageHeader.js
@@ -1,7 +1,5 @@
 import React from 'react';
 import { Grid } from 'patternfly-react';
-import InventorySettings from '../InventorySettings';
-import PageDescription from './components/PageDescription';
 import InventoryFilter from '../InventoryFilter';
 import ToolbarButtons from './components/ToolbarButtons';
 import SettingsWarning from './components/SettingsWarning';
@@ -12,10 +10,6 @@ const PageHeader = () => (
   <div className="inventory-upload-header">
     <SettingsWarning />
     <PageTitle />
-    <div className="inventory-upload-header-description">
-      <InventorySettings />
-      <PageDescription />
-    </div>
     <Grid.Row>
       <Grid.Col xs={4}>
         <InventoryFilter />
diff --git a/webpack/ForemanInventoryUpload/Components/PageHeader/PageHeader.scss b/webpack/ForemanInventoryUpload/Components/PageHeader/PageHeader.scss
index cd2a3cfe..72884cec 100644
--- a/webpack/ForemanInventoryUpload/Components/PageHeader/PageHeader.scss
+++ b/webpack/ForemanInventoryUpload/Components/PageHeader/PageHeader.scss
@@ -1,6 +1,5 @@
 .rh-cloud-inventory-page {
   .inventory-upload-header {
-    margin-top: 35px;
 
     h1 {
       font-family: 'RedHatDisplay';
diff --git a/webpack/ForemanInventoryUpload/Components/PageHeader/PageTitle.js b/webpack/ForemanInventoryUpload/Components/PageHeader/PageTitle.js
index c77d9d58..d5570762 100644
--- a/webpack/ForemanInventoryUpload/Components/PageHeader/PageTitle.js
+++ b/webpack/ForemanInventoryUpload/Components/PageHeader/PageTitle.js
@@ -13,18 +13,31 @@ import {
   ACTIONS_HISTORY_BUTTON_TEXT,
   DOCS_BUTTON_TEXT,
   CLOUD_PING_TITLE,
+  ABOUT_TITLE,
+  SETTINGS_TITLE,
 } from '../../ForemanInventoryConstants';
 import {
   getActionsHistoryUrl,
   getInventoryDocsUrl,
 } from '../../ForemanInventoryHelpers';
 import CloudPingModal from './components/CloudPingModal';
+import AboutModal from './components/AboutModal';
+import SettingsModal from './components/SettingsModal';
 
 const PageTitle = () => {
   const [isDropdownOpen, setIsDropdownOpen] = useState(false);
   const [showPingModal, setPingModal] = useState(false);
+  const [showAboutModal, setAboutModal] = useState(false);
+  const [showSettingsModal, setSettingsModal] = useState(false);
+
   const togglePingModal = () => setPingModal(v => !v);
+  const toggleAboutModal = () => setAboutModal(v => !v);
+  const toggleSettingsModal = () => setSettingsModal(v => !v);
+
   const dropdownItems = [
+    <DropdownItem key="settings" onClick={toggleSettingsModal}>
+      {SETTINGS_TITLE}
+    </DropdownItem>,
     <DropdownItem
       key="tasks-history-button"
       href={getActionsHistoryUrl()}
@@ -44,6 +57,9 @@ const PageTitle = () => {
     <DropdownItem key="cloud-ping" onClick={togglePingModal}>
       {CLOUD_PING_TITLE}
     </DropdownItem>,
+    <DropdownItem key="about" onClick={toggleAboutModal}>
+      {ABOUT_TITLE}
+    </DropdownItem>,
   ];
   return (
     <Grid className="inventory-upload-header-title">
@@ -70,6 +86,16 @@ const PageTitle = () => {
           toggle={togglePingModal}
           title={CLOUD_PING_TITLE}
         />
+        <AboutModal
+          isOpen={showAboutModal}
+          toggle={toggleAboutModal}
+          title={ABOUT_TITLE}
+        />
+        <SettingsModal
+          isOpen={showSettingsModal}
+          toggle={toggleSettingsModal}
+          title={SETTINGS_TITLE}
+        />
       </GridItem>
     </Grid>
   );
diff --git a/webpack/ForemanInventoryUpload/Components/PageHeader/__tests__/__snapshots__/PageHeader.test.js.snap b/webpack/ForemanInventoryUpload/Components/PageHeader/__tests__/__snapshots__/PageHeader.test.js.snap
index ebde573e..4a447d59 100644
--- a/webpack/ForemanInventoryUpload/Components/PageHeader/__tests__/__snapshots__/PageHeader.test.js.snap
+++ b/webpack/ForemanInventoryUpload/Components/PageHeader/__tests__/__snapshots__/PageHeader.test.js.snap
@@ -6,12 +6,6 @@ exports[`PageHeader rendering render without Props 1`] = `
 >
   <ConnectedSettingsWarning />
   <PageTitle />
-  <div
-    className="inventory-upload-header-description"
-  >
-    <ConnectedInventorySettings />
-    <PageDescription />
-  </div>
   <Row
     bsClass="row"
     componentClass="div"
diff --git a/webpack/ForemanInventoryUpload/Components/PageHeader/__tests__/__snapshots__/PageTitle.test.js.snap b/webpack/ForemanInventoryUpload/Components/PageHeader/__tests__/__snapshots__/PageTitle.test.js.snap
index 345dbce1..a5cf8b48 100644
--- a/webpack/ForemanInventoryUpload/Components/PageHeader/__tests__/__snapshots__/PageTitle.test.js.snap
+++ b/webpack/ForemanInventoryUpload/Components/PageHeader/__tests__/__snapshots__/PageTitle.test.js.snap
@@ -23,6 +23,11 @@ exports[`PageTitle rendering render without Props 1`] = `
       className="title-dropdown"
       dropdownItems={
         Array [
+          <DropdownItem
+            onClick={[Function]}
+          >
+            Settings
+          </DropdownItem>,
           <DropdownItem
             href="/foreman_tasks/tasks?search=action++%3D++ForemanInventoryUpload%3A%3AAsync%3A%3AGenerateReportJob+or+action++%3D++ForemanInventoryUpload%3A%3AAsync%3A%3AGenerateAllReportsJob&page=1"
             rel="noopener noreferrer"
@@ -42,6 +47,11 @@ exports[`PageTitle rendering render without Props 1`] = `
           >
             Connectivity test
           </DropdownItem>,
+          <DropdownItem
+            onClick={[Function]}
+          >
+            About
+          </DropdownItem>,
         ]
       }
       isOpen={false}
@@ -59,6 +69,16 @@ exports[`PageTitle rendering render without Props 1`] = `
       title="Connectivity test"
       toggle={[Function]}
     />
+    <AboutModal
+      isOpen={false}
+      title="About"
+      toggle={[Function]}
+    />
+    <SettingsModal
+      isOpen={false}
+      title="Settings"
+      toggle={[Function]}
+    />
   </GridItem>
 </Grid>
 `;
diff --git a/webpack/ForemanInventoryUpload/Components/PageHeader/components/AboutModal/index.js b/webpack/ForemanInventoryUpload/Components/PageHeader/components/AboutModal/index.js
new file mode 100644
index 00000000..2cd7bdde
--- /dev/null
+++ b/webpack/ForemanInventoryUpload/Components/PageHeader/components/AboutModal/index.js
@@ -0,0 +1,26 @@
+/* eslint-disable camelcase */
+import React from 'react';
+import PropTypes from 'prop-types';
+import { Modal, ModalVariant } from '@patternfly/react-core';
+import PageDescription from '../PageDescription';
+
+const AboutModal = ({ isOpen, toggle, title }) => (
+  <Modal
+    id="rh-cloud-about-modal"
+    appendTo={document.getElementsByClassName('react-container')[0]}
+    variant={ModalVariant.large}
+    title={title}
+    isOpen={isOpen}
+    onClose={toggle}
+  >
+    <PageDescription />
+  </Modal>
+);
+
+AboutModal.propTypes = {
+  isOpen: PropTypes.bool.isRequired,
+  toggle: PropTypes.func.isRequired,
+  title: PropTypes.string.isRequired,
+};
+
+export default AboutModal;
diff --git a/webpack/ForemanInventoryUpload/Components/PageHeader/components/SettingsModal/index.js b/webpack/ForemanInventoryUpload/Components/PageHeader/components/SettingsModal/index.js
new file mode 100644
index 00000000..dec90e25
--- /dev/null
+++ b/webpack/ForemanInventoryUpload/Components/PageHeader/components/SettingsModal/index.js
@@ -0,0 +1,26 @@
+/* eslint-disable camelcase */
+import React from 'react';
+import PropTypes from 'prop-types';
+import { Modal, ModalVariant } from '@patternfly/react-core';
+import InventorySettings from '../../../InventorySettings';
+
+const SettingsModal = ({ isOpen, toggle, title }) => (
+  <Modal
+    id="about-modal"
+    appendTo={document.getElementsByClassName('react-container')[0]}
+    variant={ModalVariant.small}
+    title={title}
+    isOpen={isOpen}
+    onClose={toggle}
+  >
+    <InventorySettings />
+  </Modal>
+);
+
+SettingsModal.propTypes = {
+  isOpen: PropTypes.bool.isRequired,
+  toggle: PropTypes.func.isRequired,
+  title: PropTypes.string.isRequired,
+};
+
+export default SettingsModal;
diff --git a/webpack/ForemanInventoryUpload/Components/TabHeader/TabHeader.js b/webpack/ForemanInventoryUpload/Components/TabHeader/TabHeader.js
index 31b6a305..855e1e0e 100644
--- a/webpack/ForemanInventoryUpload/Components/TabHeader/TabHeader.js
+++ b/webpack/ForemanInventoryUpload/Components/TabHeader/TabHeader.js
@@ -1,6 +1,7 @@
 import React from 'react';
 import PropTypes from 'prop-types';
-import { Grid, Button, Icon } from 'patternfly-react';
+import { Grid, Icon } from 'patternfly-react';
+import { Button } from '@patternfly/react-core';
 import { noop } from 'foremanReact/common/helpers';
 import { sprintf, translate as __ } from 'foremanReact/common/I18n';
 import { isExitCodeLoading } from '../../ForemanInventoryHelpers';
@@ -15,19 +16,19 @@ const TabHeader = ({ exitCode, onRestart, onDownload, toggleFullScreen }) => (
       <div className="tab-action-buttons">
         {onRestart ? (
           <Button
-            bsStyle="primary"
+            variant="primary"
             onClick={onRestart}
-            disabled={isExitCodeLoading(exitCode)}
+            isDisabled={isExitCodeLoading(exitCode)}
           >
             {__('Restart')}
           </Button>
         ) : null}
         {onDownload ? (
-          <Button onClick={onDownload}>
+          <Button onClick={onDownload} variant="secondary">
             {__('Download Report')} <Icon name="download" />
           </Button>
         ) : null}
-        <Button onClick={toggleFullScreen}>
+        <Button onClick={toggleFullScreen} variant="secondary">
           {__('Full Screen')}
           <Icon name="arrows-alt" />
         </Button>
diff --git a/webpack/ForemanInventoryUpload/Components/TabHeader/__tests__/__snapshots__/TabHeader.test.js.snap b/webpack/ForemanInventoryUpload/Components/TabHeader/__tests__/__snapshots__/TabHeader.test.js.snap
index 61159cbd..5f59b29b 100644
--- a/webpack/ForemanInventoryUpload/Components/TabHeader/__tests__/__snapshots__/TabHeader.test.js.snap
+++ b/webpack/ForemanInventoryUpload/Components/TabHeader/__tests__/__snapshots__/TabHeader.test.js.snap
@@ -24,12 +24,8 @@ exports[`TabHeader rendering render without Props 1`] = `
       className="tab-action-buttons"
     >
       <Button
-        active={false}
-        block={false}
-        bsClass="btn"
-        bsStyle="default"
-        disabled={false}
         onClick={[Function]}
+        variant="secondary"
       >
         Full Screen
         <Icon
diff --git a/webpack/ForemanInventoryUpload/ForemanInventoryConstants.js b/webpack/ForemanInventoryUpload/ForemanInventoryConstants.js
index 89a3ce77..2f8ea405 100644
--- a/webpack/ForemanInventoryUpload/ForemanInventoryConstants.js
+++ b/webpack/ForemanInventoryUpload/ForemanInventoryConstants.js
@@ -9,3 +9,7 @@ export const ACTIONS_HISTORY_BUTTON_TEXT = __('Actions history');
 export const SYNC_BUTTON_TEXT = __(' Sync inventory status');
 
 export const CLOUD_PING_TITLE = __('Connectivity test');
+
+export const ABOUT_TITLE = __('About');
+
+export const SETTINGS_TITLE = __('Settings');
diff --git a/webpack/InsightsCloudSync/Components/InsightsTable/__tests__/__snapshots__/InsightsTable.test.js.snap b/webpack/InsightsCloudSync/Components/InsightsTable/__tests__/__snapshots__/InsightsTable.test.js.snap
index a4ab6305..d97ee2bc 100644
--- a/webpack/InsightsCloudSync/Components/InsightsTable/__tests__/__snapshots__/InsightsTable.test.js.snap
+++ b/webpack/InsightsCloudSync/Components/InsightsTable/__tests__/__snapshots__/InsightsTable.test.js.snap
@@ -10,8 +10,10 @@ exports[`InsightsTable rendering render with Props 1`] = `
     showSelectAllAlert={false}
   />
   <Table
+    actionsMenuAppendTo="inline"
     aria-label="Recommendations Table"
     borders={true}
+    canCollapseAll={false}
     canSelectAll={true}
     canSortFavorites={true}
     cells={
@@ -69,6 +71,7 @@ exports[`InsightsTable rendering render with Props 1`] = `
       ]
     }
     className="rh-cloud-recommendations-table"
+    collapseAllAriaLabel=""
     contentId="expanded-content"
     dropdownDirection="down"
     dropdownPosition="right"
diff --git a/webpack/common/Switcher/HelpLabel.js b/webpack/common/Switcher/HelpLabel.js
index 8830561d..e048ed87 100644
--- a/webpack/common/Switcher/HelpLabel.js
+++ b/webpack/common/Switcher/HelpLabel.js
@@ -19,7 +19,7 @@ export const HelpLabel = ({ text, id, className }) => {
 
 HelpLabel.propTypes = {
   id: PropTypes.string.isRequired,
-  text: PropTypes.string,
+  text: PropTypes.oneOfType([PropTypes.string, PropTypes.object]),
   className: PropTypes.string,
 };
 HelpLabel.defaultProps = {
diff --git a/webpack/common/Switcher/SwitcherPF4.scss b/webpack/common/Switcher/SwitcherPF4.scss
index f6b81926..c9d7abbc 100644
--- a/webpack/common/Switcher/SwitcherPF4.scss
+++ b/webpack/common/Switcher/SwitcherPF4.scss
@@ -6,4 +6,8 @@ label.foreman-rh-cloud-switcher {
   .switcher-help-label {
     padding-left: 5px;
   }
+
+  .pf-c-switch__toggle {
+    margin-right: 5px;
+  }
 }