Refs #36793 - Refactored based on suggestions

Apply suggestions from code review Co-authored-by: Jeremy Lenz <[email protected]>
theforeman · Oct 18, 2023 · f7ac715 · f7ac715
1 parent 1b1c644
commit f7ac715
Show file tree

Hide file tree

Showing 12 changed files with 191 additions and 183 deletions.
diff --git a/app/registries/foreman/settings/general.rb b/app/registries/foreman/settings/general.rb
@@ -53,9 +53,9 @@
       full_name: N_('Show Experimental Labs'))
     setting('new_hosts_pages',
       type: :boolean,
-      description: N_("Whether or not to show the new hosts page for All Hosts (requires reload of page)"),
+      description: N_("Whether or not to show the new overview page for All Hosts (requires reload of page)"),
       default: false,
-      full_name: N_('Show New Hosts Pages'))
+      full_name: N_('Show New Host Overview Page'))
     setting('display_fqdn_for_hosts',
       type: :boolean,
       description: N_('Display names of hosts as FQDNs. If disabled, only display names of hosts as hostnames.'),

diff --git a/app/registries/menu/loader.rb b/app/registries/menu/loader.rb
@@ -53,10 +53,9 @@ def self.load
 
         menu.sub_menu :hosts_menu,      :caption => N_('Hosts'), :icon => 'fa fa-server' do
           menu.item :hosts, :caption => N_('All Hosts'),
-                :if => proc {!Setting[:new_hosts_pages]}
+                :if => proc { !Setting[:new_hosts_pages] }
           menu.item :newhosts, :caption => N_('All Hosts'),
-                :if => proc {Setting[:new_hosts_pages]},
-                :caption => N_('All Hosts'),
+                :if => proc { Setting[:new_hosts_pages] },
                 :url => '/new/hosts',
                 :url_hash => { :controller => 'api/v2/hosts', :action => 'index' }
 

diff --git a/webpack/assets/javascripts/react_app/components/HostsIndex/Selectors.js b/webpack/assets/javascripts/react_app/components/HostsIndex/Selectors.js
@@ -0,0 +1,4 @@
+import { selectComponentByWeight } from '../common/Slot/SlotSelectors';
+
+export const selectKebabItems = () =>
+  selectComponentByWeight('hosts-index-kebab');
diff --git a/webpack/assets/javascripts/react_app/components/HostsIndex/index.js b/webpack/assets/javascripts/react_app/components/HostsIndex/index.js
@@ -1,28 +1,107 @@
-import React from 'react';
-import { CubeIcon } from '@patternfly/react-icons';
+import React, { createContext } from 'react';
+import PropTypes from 'prop-types';
+import { useSelector, shallowEqual } from 'react-redux';
+import { Td } from '@patternfly/react-table';
+import { ToolbarItem } from '@patternfly/react-core';
 import { translate as __ } from '../../common/I18n';
 import TableIndexPage from '../PF4/TableIndexPage/TableIndexPage';
+import { ActionKebab } from '../PF4/TableIndexPage/ActionKebab';
 import { HOSTS_API_PATH, API_REQUEST_KEY } from '../../routes/Hosts/constants';
+import { selectKebabItems } from './Selectors';
+import { useAPI } from '../../common/hooks/API/APIHooks';
+import { useBulkSelect } from '../PF4/TableIndexPage/Table/TableHooks';
+import SelectAllCheckbox from '../PF4/TableIndexPage/Table/SelectAllCheckbox';
+import { getPageStats } from '../PF4/TableIndexPage/Table/helpers';
+
+export const ForemanHostsIndexActionsBarContext = createContext({});
 
 const HostsIndex = () => {
   const columns = {
     name: {
       title: __('Name'),
-      wrapper: ({ id, name }) => (<a href={`hosts/${id}`}>{name}</a>),
+      wrapper: ({ id, name }) => <a href={`hosts/${id}`}>{name}</a>,
       isSorted: true,
     },
   };
+  const defaultParams = { search: '' }; // search ||
 
-  const computeContentSource = search =>
-    `/change_host_content_source?search=${search}`;
+  const response = useAPI('get', `${HOSTS_API_PATH}?include_permissions=true`, {
+    key: API_REQUEST_KEY,
+    params: defaultParams,
+  });
 
-  const customActionKebabs = [
-    {
-      title: __('Change content source'),
-      icon: <CubeIcon />,
-      computeHref: computeContentSource,
+  const {
+    response: {
+      search: apiSearchQuery,
+      results,
+      total,
+      per_page: perPage,
+      page,
     },
-  ];
+  } = response;
+
+  const { pageRowCount } = getPageStats({ total, page, perPage });
+
+  const { fetchBulkParams, ...selectAllOptions } = useBulkSelect({
+    results,
+    metadata: {},
+    initialSearchQuery: apiSearchQuery || '',
+  });
+
+  const {
+    selectAll,
+    selectPage,
+    selectNone,
+    selectedCount,
+    selectOne,
+    areAllRowsOnPageSelected,
+    areAllRowsSelected,
+    isSelected,
+  } = selectAllOptions;
+
+  const selectionToolbar = (
+    <ToolbarItem key="selectAll">
+      <SelectAllCheckbox
+        {...{
+          selectAll,
+          selectPage,
+          selectNone,
+          selectedCount,
+          pageRowCount,
+        }}
+        totalCount={total}
+        areAllRowsOnPageSelected={areAllRowsOnPageSelected()}
+        areAllRowsSelected={areAllRowsSelected()}
+      />
+    </ToolbarItem>
+  );
+
+  const RowSelectTd = ({ rowData }) => (
+    <Td
+      select={{
+        rowIndex: rowData.id,
+        onSelect: (_event, isSelecting) => {
+          selectOne(isSelecting, rowData.id);
+        },
+        isSelected: isSelected(rowData.id),
+        disable: false,
+      }}
+    />
+  );
+
+  RowSelectTd.propTypes = {
+    rowData: PropTypes.object.isRequired,
+  };
+
+  const actionNode = [];
+  const registeredItems = useSelector(selectKebabItems, shallowEqual);
+  const customToolbarItems = (
+    <ForemanHostsIndexActionsBarContext.Provider
+      value={{ ...selectAllOptions, fetchBulkParams }}
+    >
+      <ActionKebab items={actionNode.concat(registeredItems)} />
+    </ForemanHostsIndexActionsBarContext.Provider>
+  );
 
   return (
     <TableIndexPage
@@ -32,9 +111,12 @@ const HostsIndex = () => {
       controller="hosts"
       isDeleteable
       columns={columns}
-      displaySelectAllCheckbox
-      customActionKebabs={customActionKebabs}
       creatable={false}
+      response={response}
+      customToolbarItems={customToolbarItems}
+      selectionToolbar={selectionToolbar}
+      showCheckboxes
+      rowSelectTd={RowSelectTd}
     />
   );
 };

diff --git a/webpack/assets/javascripts/react_app/components/PF4/TableIndexPage/ActionButtons.js b/webpack/assets/javascripts/react_app/components/PF4/TableIndexPage/ActionButtons.js
@@ -1,6 +1,11 @@
-import React from 'react';
+import React, { useState } from 'react';
 import PropTypes from 'prop-types';
-import { Button } from '@patternfly/react-core';
+import {
+  Button,
+  Dropdown,
+  KebabToggle,
+  DropdownItem,
+} from '@patternfly/react-core';
 
 /**
  * Generate a button or a dropdown of buttons
@@ -10,20 +15,43 @@ import { Button } from '@patternfly/react-core';
  */
 export const ActionButtons = ({ buttons: originalButtons }) => {
   const buttons = [...originalButtons];
+  const [isOpen, setIsOpen] = useState(false);
   if (!buttons.length) return null;
-
-  const pfButtons = buttons.map(button => (
-    <Button
-      key={button.title}
-      ouiaId="action-buttons-button"
-      component={button.action?.href ? 'a' : undefined}
-      {...button.action}
-    >
-      {button.title}
-    </Button>
-  ));
-
-  return <>{pfButtons}</>;
+  const firstButton = buttons.shift();
+  return (
+    <>
+      <Button
+        ouiaId="action-buttons-button"
+        component={firstButton.action?.href ? 'a' : undefined}
+        {...firstButton.action}
+      >
+        {firstButton.title}
+      </Button>
+      {buttons.length > 0 && (
+        <Dropdown
+          ouiaId="action-buttons-dropdown"
+          toggle={
+            <KebabToggle
+              aria-label="toggle action dropdown"
+              onToggle={setIsOpen}
+            />
+          }
+          isOpen={isOpen}
+          isPlain
+          dropdownItems={buttons.map(button => (
+            <DropdownItem
+              ouiaId={`${button.title}-dropdown-item`}
+              key={button.title}
+              title={button.title}
+              {...button.action}
+            >
+              {button.icon} {button.title}
+            </DropdownItem>
+          ))}
+        />
+      )}
+    </>
+  );
 };
 
 ActionButtons.propTypes = {
@@ -32,7 +60,6 @@ ActionButtons.propTypes = {
       action: PropTypes.object,
       title: PropTypes.string,
       icon: PropTypes.node,
-      isDisabled: PropTypes.bool,
     })
   ),
 };

diff --git a/webpack/assets/javascripts/react_app/components/PF4/TableIndexPage/ActionKebab.js b/webpack/assets/javascripts/react_app/components/PF4/TableIndexPage/ActionKebab.js
@@ -1,15 +1,14 @@
 import React, { useState } from 'react';
 import PropTypes from 'prop-types';
-import { Dropdown, KebabToggle, DropdownItem } from '@patternfly/react-core';
+import { Dropdown, KebabToggle } from '@patternfly/react-core';
 
 /**
  * Generate a button or a dropdown of buttons
  * @param  {String} title The title of the button for the title and text inside the button
  * @param  {Object} action action to preform when the button is click can be href with data-method or Onclick
  * @return {Function} button component or splitbutton component
  */
-export const ActionKebab = ({ items: originalItems }) => {
-  const items = [...originalItems];
+export const ActionKebab = ({ items }) => {
   const [isOpen, setIsOpen] = useState(false);
   if (!items.length) return null;
   return (
@@ -25,32 +24,15 @@ export const ActionKebab = ({ items: originalItems }) => {
           }
           isOpen={isOpen}
           isPlain
-          dropdownItems={items.map(item => (
-            <DropdownItem
-              ouiaId={`${item.title}-dropdown-item`}
-              key={item.title}
-              title={item.title}
-              {...item.action}
-              isDisabled={item.isDisabled}
-            >
-              {item.icon} {item.title}
-            </DropdownItem>
-          ))}
+          dropdownItems={items}
         />
       )}
     </>
   );
 };
 
 ActionKebab.propTypes = {
-  items: PropTypes.arrayOf(
-    PropTypes.shape({
-      action: PropTypes.object,
-      title: PropTypes.string,
-      icon: PropTypes.node,
-      isDisabled: PropTypes.bool,
-    })
-  ),
+  items: PropTypes.arrayOf(PropTypes.node),
 };
 
 ActionKebab.defaultProps = {

diff --git a/...ack/assets/javascripts/react_app/components/PF4/TableIndexPage/Table/SelectAllCheckbox.js b/...ack/assets/javascripts/react_app/components/PF4/TableIndexPage/Table/SelectAllCheckbox.js
@@ -115,8 +115,8 @@ const SelectAllCheckbox = ({
           ouiaId="select-all-checkbox-dropdown-toggle"
           splitButtonItems={[
             <DropdownToggleCheckbox
-              className="tablewrapper-select-all-checkbox"
-              key="tablewrapper-select-all-checkbox"
+              className="table-select-all-checkbox"
+              key="table-select-all-checkbox"
               ouiaId="select-all-checkbox-dropdown-toggle-checkbox"
               aria-label="Select all"
               onChange={checked => onSelectAllCheckboxChange(checked)}

diff --git a/...k/assets/javascripts/react_app/components/PF4/TableIndexPage/Table/SelectAllCheckbox.scss b/...k/assets/javascripts/react_app/components/PF4/TableIndexPage/Table/SelectAllCheckbox.scss
@@ -1,3 +1,3 @@
-.tablewrapper-select-all-checkbox {
+.table-select-all-checkbox {
   font-weight: normal;
 }
diff --git a/webpack/assets/javascripts/react_app/components/PF4/TableIndexPage/Table/Table.js b/webpack/assets/javascripts/react_app/components/PF4/TableIndexPage/Table/Table.js
@@ -29,9 +29,8 @@ export const Table = ({
   url,
   isPending,
   isEmbedded,
-  displaySelectAllCheckbox,
-  isSelected,
-  selectOne,
+  showCheckboxes,
+  rowSelectTd,
 }) => {
   const columnsToSortParams = {};
   Object.keys(columns).forEach(key => {
@@ -73,7 +72,7 @@ export const Table = ({
       getActions && getActions({ id, name, ...item }),
     ].filter(Boolean);
   const columnNamesKeys = Object.keys(columns);
-
+  const RowSelectTd = rowSelectTd;
   return (
     <>
       <DeleteModal
@@ -86,7 +85,7 @@ export const Table = ({
       <TableComposable variant="compact" ouiaId="table">
         <Thead>
           <Tr ouiaId="table-header">
-            {displaySelectAllCheckbox && <Th key="checkbox-th" />}
+            {showCheckboxes && <Th key="checkbox-th" />}
             {columnNamesKeys.map(k => (
               <Th
                 key={k}
@@ -129,18 +128,7 @@ export const Table = ({
           )}
           {results.map((result, rowIndex) => (
             <Tr key={rowIndex} ouiaId={`table-row-${rowIndex}`}>
-              {displaySelectAllCheckbox && (
-                <Td
-                  select={{
-                    rowIndex: result.id,
-                    onSelect: (_event, isSelecting) => {
-                      selectOne(isSelecting, result.id);
-                    },
-                    isSelected: isSelected(result.id),
-                    disable: false,
-                  }}
-                />
-              )}
+              {showCheckboxes && <RowSelectTd rowData={result} />}
               {columnNamesKeys.map(k => (
                 <Td key={k} dataLabel={columnNames[k]}>
                   {columns[k].wrapper ? columns[k].wrapper(result) : result[k]}
@@ -183,9 +171,8 @@ Table.propTypes = {
   url: PropTypes.string.isRequired,
   isPending: PropTypes.bool.isRequired,
   isEmbedded: PropTypes.bool,
-  displaySelectAllCheckbox: PropTypes.bool,
-  isSelected: PropTypes.func,
-  selectOne: PropTypes.func,
+  rowSelectTd: PropTypes.func,
+  showCheckboxes: PropTypes.bool,
 };
 
 Table.defaultProps = {
@@ -195,7 +182,6 @@ Table.defaultProps = {
   getActions: null,
   results: [],
   isEmbedded: false,
-  displaySelectAllCheckbox: false,
-  selectOne: noop,
-  isSelected: noop,
+  rowSelectTd: noop,
+  showCheckboxes: false,
 };