Skip to content

Commit

Permalink
Refs #36793 - Refactored based on suggestions
Browse files Browse the repository at this point in the history
Apply suggestions from code review

Co-authored-by: Jeremy Lenz <[email protected]>
  • Loading branch information
parthaa and jeremylenz committed Oct 18, 2023
1 parent 1b1c644 commit 0ee3760
Show file tree
Hide file tree
Showing 11 changed files with 189 additions and 180 deletions.
4 changes: 2 additions & 2 deletions app/registries/foreman/settings/general.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.'),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import { selectComponentByWeight } from '../common/Slot/SlotSelectors';

export const selectKebabItems = () =>
selectComponentByWeight('hosts-index-kebab');
108 changes: 95 additions & 13 deletions webpack/assets/javascripts/react_app/components/HostsIndex/index.js
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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}
/>
);
};
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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 = {
Expand All @@ -32,7 +60,6 @@ ActionButtons.propTypes = {
action: PropTypes.object,
title: PropTypes.string,
icon: PropTypes.node,
isDisabled: PropTypes.bool,
})
),
};
Expand Down
Original file line number Diff line number Diff line change
@@ -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 (
Expand All @@ -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 = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
.tablewrapper-select-all-checkbox {
.table-select-all-checkbox {
font-weight: normal;
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,8 @@ export const Table = ({
url,
isPending,
isEmbedded,
displaySelectAllCheckbox,
isSelected,
selectOne,
showCheckboxes,
rowSelectTd,
}) => {
const columnsToSortParams = {};
Object.keys(columns).forEach(key => {
Expand Down Expand Up @@ -73,7 +72,7 @@ export const Table = ({
getActions && getActions({ id, name, ...item }),
].filter(Boolean);
const columnNamesKeys = Object.keys(columns);

const RowSelectTd = rowSelectTd;
return (
<>
<DeleteModal
Expand All @@ -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}
Expand Down Expand Up @@ -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]}
Expand Down Expand Up @@ -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 = {
Expand All @@ -195,7 +182,6 @@ Table.defaultProps = {
getActions: null,
results: [],
isEmbedded: false,
displaySelectAllCheckbox: false,
selectOne: noop,
isSelected: noop,
rowSelectTd: noop,
showCheckboxes: false,
};
Loading

0 comments on commit 0ee3760

Please sign in to comment.