Skip to content

Commit

Permalink
Revert "fix: cover select all edge cases (#10187)"
Browse files Browse the repository at this point in the history
This reverts commit 11a2581.
  • Loading branch information
hkang1 committed Nov 13, 2024
1 parent e56dd5a commit e8cca5e
Show file tree
Hide file tree
Showing 8 changed files with 70 additions and 88 deletions.
3 changes: 0 additions & 3 deletions webui/react/src/components/LoadableCount.test.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { render, screen } from '@testing-library/react';
import { Loaded, NotLoaded } from 'hew/utils/loadable';
import { noop } from 'lodash';

import LoadableCount from './LoadableCount';

Expand All @@ -25,11 +24,9 @@ const setup = (totalCount: number, selectedCount?: number, loaded?: boolean) =>
const total = loaded ? Loaded(totalCount) : NotLoaded;
render(
<LoadableCount
handleSelectionChange={noop}
labelPlural={LABEL_PLURAL}
labelSingular={LABEL_SINGULAR}
selectedCount={selectedCount ?? 0}
selectionAction="NONE"
total={total}
/>,
);
Expand Down
50 changes: 22 additions & 28 deletions webui/react/src/components/LoadableCount.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import Button from 'hew/Button';
import { HandleSelectionChangeType } from 'hew/DataGrid/DataGrid';
import { Loadable } from 'hew/utils/loadable';
import { useMemo } from 'react';

Expand All @@ -8,23 +7,24 @@ import { pluralizer } from 'utils/string';

import css from './LoadableCount.module.scss';

export type SelectionAction = 'SELECT_ALL' | 'CLEAR_SELECTION' | 'NONE';
interface Props {
total: Loadable<number>;
labelSingular: string;
labelPlural: string;
onActualSelectAll?: () => void;
onClearSelect?: () => void;
pageSize?: number;
selectedCount: number;
selectionAction: SelectionAction;
handleSelectionChange: HandleSelectionChangeType;
}

const LoadableCount: React.FC<Props> = ({
total,
labelPlural,
labelSingular,
onActualSelectAll,
onClearSelect,
pageSize = 20,
selectedCount,
selectionAction,
handleSelectionChange,
}: Props) => {
const isMobile = useMobile();

Expand All @@ -51,31 +51,25 @@ const LoadableCount: React.FC<Props> = ({
const actualSelectAll = useMemo(() => {
return Loadable.match(total, {
_: () => null,
Loaded: () => {
switch (selectionAction) {
case 'SELECT_ALL': {
const onClick = () => handleSelectionChange('add-all');
return (
<Button data-test="select-all" type="text" onClick={onClick}>
Select all {labelPlural} in table
</Button>
);
}
case 'CLEAR_SELECTION': {
const onClick = () => handleSelectionChange('remove-all');
return (
<Button data-test="clear-selection" type="text" onClick={onClick}>
Clear Selection
</Button>
);
}
case 'NONE': {
return null;
}
Loaded: (loadedTotal) => {
if (onActualSelectAll && selectedCount >= pageSize && selectedCount < loadedTotal) {
return (
<Button data-test="select-all" type="text" onClick={onActualSelectAll}>
Select all {labelPlural} in table
</Button>
);
} else if (onClearSelect && (selectedCount >= pageSize || selectedCount === loadedTotal)) {
return (
<Button data-test="clear-selection" type="text" onClick={onClearSelect}>
Clear Selection
</Button>
);
}

return null;
},
});
}, [labelPlural, handleSelectionChange, selectionAction, total]);
}, [labelPlural, onActualSelectAll, onClearSelect, pageSize, selectedCount, total]);

if (!isMobile) {
return (
Expand Down
12 changes: 10 additions & 2 deletions webui/react/src/components/Searches/Searches.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -637,6 +637,14 @@ const Searches: React.FC<Props> = ({ project }) => {
users,
]);

const handleActualSelectAll = useCallback(() => {
handleSelectionChange?.('add-all');
}, [handleSelectionChange]);

const handleClearSelect = useCallback(() => {
handleSelectionChange?.('remove-all');
}, [handleSelectionChange]);

const handleHeaderClick = useCallback(
(columnId: string): void => {
if (columnId === MULTISELECT) {
Expand Down Expand Up @@ -779,10 +787,8 @@ const Searches: React.FC<Props> = ({ project }) => {
columnGroups={[V1LocationType.EXPERIMENT]}
entityCopy="Show searches…"
formStore={formStore}
handleSelectionChange={handleSelectionChange}
initialVisibleColumns={columnsIfLoaded}
isOpenFilter={isOpenFilter}
isRangeSelected={isRangeSelected}
labelPlural="searches"
labelSingular="search"
pageSize={settings.pageLimit}
Expand All @@ -797,6 +803,8 @@ const Searches: React.FC<Props> = ({ project }) => {
total={total}
onActionComplete={handleActionComplete}
onActionSuccess={handleActionSuccess}
onActualSelectAll={handleActualSelectAll}
onClearSelect={handleClearSelect}
onIsOpenFilterChange={handleIsOpenFilterChange}
onRowHeightChange={handleRowHeightChange}
onSortChange={handleSortChange}
Expand Down
33 changes: 10 additions & 23 deletions webui/react/src/components/TableActionBar.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import Button from 'hew/Button';
import Column from 'hew/Column';
import { HandleSelectionChangeType, Sort } from 'hew/DataGrid/DataGrid';
import { Sort } from 'hew/DataGrid/DataGrid';
import Dropdown, { MenuItem } from 'hew/Dropdown';
import Icon, { IconName } from 'hew/Icon';
import { useModal } from 'hew/Modal';
Expand Down Expand Up @@ -57,7 +57,7 @@ import { capitalizeWord } from 'utils/string';
import { openCommandResponse } from 'utils/wait';

import { FilterFormSet } from './FilterForm/components/type';
import LoadableCount, { SelectionAction } from './LoadableCount';
import LoadableCount from './LoadableCount';
import css from './TableActionBar.module.scss';

const batchActions = [
Expand Down Expand Up @@ -91,22 +91,22 @@ const actionIcons: Record<BatchAction, IconName> = {
interface Props {
compareViewOn?: boolean;
formStore: FilterFormStore;
handleSelectionChange: HandleSelectionChangeType;
heatmapBtnVisible?: boolean;
heatmapOn?: boolean;
initialVisibleColumns: string[];
isOpenFilter: boolean;
isRangeSelected: (range: [number, number]) => boolean;
onActionComplete?: () => Promise<void>;
onActionSuccess?: (action: BatchAction, successfulIds: number[]) => void;
onActualSelectAll?: () => void;
onClearSelect?: () => void;
onComparisonViewToggle?: () => void;
onHeatmapToggle?: (heatmapOn: boolean) => void;
onIsOpenFilterChange?: (value: boolean) => void;
onRowHeightChange?: (rowHeight: RowHeight) => void;
onSortChange?: (sorts: Sort[]) => void;
onVisibleColumnChange?: (newColumns: string[], pinnedCount?: number) => void;
onHeatmapSelectionRemove?: (id: string) => void;
pageSize: number;
pageSize?: number;
project: Project;
projectColumns: Loadable<ProjectColumn[]>;
rowHeight: RowHeight;
Expand All @@ -129,14 +129,14 @@ const TableActionBar: React.FC<Props> = ({
compareViewOn,
formStore,
tableFilterString,
handleSelectionChange,
heatmapBtnVisible,
heatmapOn,
initialVisibleColumns,
isOpenFilter,
isRangeSelected,
onActionComplete,
onActionSuccess,
onActualSelectAll,
onClearSelect,
onComparisonViewToggle,
onHeatmapToggle,
onIsOpenFilterChange,
Expand Down Expand Up @@ -412,20 +412,6 @@ const TableActionBar: React.FC<Props> = ({
}, [] as MenuItem[]);
}, [availableBatchActions]);

const selectionAction: SelectionAction = useMemo(() => {
return total.match({
_: () => 'NONE' as const,
Loaded: (loadedTotal) => {
if (isRangeSelected([0, pageSize]) && selectionSize < loadedTotal) {
return 'SELECT_ALL';
} else if (selectionSize > pageSize) {
return 'CLEAR_SELECTION';
}
return 'NONE';
},
});
}, [isRangeSelected, selectionSize, pageSize, total]);

return (
<div className={css.base} data-test-component="tableActionBar">
<Row>
Expand Down Expand Up @@ -469,12 +455,13 @@ const TableActionBar: React.FC<Props> = ({
</Dropdown>
)}
<LoadableCount
handleSelectionChange={handleSelectionChange}
labelPlural={labelPlural}
labelSingular={labelSingular}
pageSize={pageSize}
selectedCount={selectionSize}
selectionAction={selectionAction}
total={total}
onActualSelectAll={onActualSelectAll}
onClearSelect={onClearSelect}
/>
</Row>
</Column>
Expand Down
1 change: 0 additions & 1 deletion webui/react/src/e2e/models/components/TableActionBar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ export class TableActionBar extends NamedComponent {
heatmapToggle = new BaseComponent({ parent: this, selector: '[data-test="heatmapToggle"]' });
compare = new BaseComponent({ parent: this, selector: '[data-test="compare"]' });
clearSelection = new BaseComponent({ parent: this, selector: '[data-test="clear-selection"]' });
selectAll = new BaseComponent({ parent: this, selector: '[data-test="select-all"]' });
// TODO a bunch of modals
}

Expand Down
13 changes: 4 additions & 9 deletions webui/react/src/e2e/tests/experimentList.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,10 @@ test.describe('Experiment List', () => {
const count = await getCount();
if (count !== 0) {
await grid.headRow.clickSelectHeader();
const selectAllButton = projectDetailsPage.f_experimentList.tableActionBar.selectAll;
const clearAllButton = projectDetailsPage.f_experimentList.tableActionBar.clearSelection;
if (await selectAllButton.pwLocator.isVisible()) {
await selectAllButton.pwLocator.click();
await clearAllButton.pwLocator.click();
} else if (await clearAllButton.pwLocator.isVisible()) {
await clearAllButton.pwLocator.click();
} else {
await grid.headRow.clickSelectHeader();
const isClearSelectionVisible =
await projectDetailsPage.f_experimentList.tableActionBar.clearSelection.pwLocator.isVisible();
if (isClearSelectionVisible) {
await projectDetailsPage.f_experimentList.tableActionBar.clearSelection.pwLocator.click();
}
}
});
Expand Down
13 changes: 10 additions & 3 deletions webui/react/src/pages/F_ExpList/F_ExperimentList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,14 @@ const F_ExperimentList: React.FC<Props> = ({ project }) => {
[handleSelectionChange, openToast],
);

const handleActualSelectAll = useCallback(() => {
handleSelectionChange?.('add-all');
}, [handleSelectionChange]);

const handleClearSelect = useCallback(() => {
handleSelectionChange?.('remove-all');
}, [handleSelectionChange]);

const handleContextMenuComplete = useCallback(
(action: ExperimentAction, id: number, data?: Partial<BulkExperimentItem>) =>
handleActionSuccess(action, [id], data),
Expand Down Expand Up @@ -979,15 +987,12 @@ const F_ExperimentList: React.FC<Props> = ({ project }) => {
compareViewOn={settings.compare}
entityCopy="Show experiments…"
formStore={formStore}
handleSelectionChange={handleSelectionChange}
heatmapBtnVisible={heatmapBtnVisible}
heatmapOn={settings.heatmapOn}
initialVisibleColumns={columnsIfLoaded}
isOpenFilter={isOpenFilter}
isRangeSelected={isRangeSelected}
labelPlural="experiments"
labelSingular="experiment"
pageSize={settings.pageLimit}
pinnedColumnsCount={settings.pinnedColumnsCount}
project={project}
projectColumns={projectColumns}
Expand All @@ -1000,6 +1005,8 @@ const F_ExperimentList: React.FC<Props> = ({ project }) => {
total={total}
onActionComplete={handleActionComplete}
onActionSuccess={handleActionSuccess}
onActualSelectAll={handleActualSelectAll}
onClearSelect={handleClearSelect}
onComparisonViewToggle={handleToggleComparisonView}
onHeatmapSelectionRemove={(id) => {
const newSelection = settings.heatmapSkipped.filter((s) => s !== id);
Expand Down
33 changes: 14 additions & 19 deletions webui/react/src/pages/FlatRuns/FlatRuns.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ import {
SpecialColumnNames,
} from 'components/FilterForm/components/type';
import TableFilter from 'components/FilterForm/TableFilter';
import LoadableCount, { SelectionAction } from 'components/LoadableCount';
import LoadableCount from 'components/LoadableCount';
import MultiSortMenu, { EMPTY_SORT, sortMenuItemsForColumn } from 'components/MultiSortMenu';
import { OptionsMenu, RowHeight } from 'components/OptionsMenu';
import {
Expand Down Expand Up @@ -779,13 +779,21 @@ const FlatRuns: React.FC<Props> = ({ projectId, workspaceId, searchId }) => {
[updateSettings],
);

const handleActualSelectAll = useCallback(() => {
handleSelectionChange?.('add-all');
}, [handleSelectionChange]);

const handleClearSelect = useCallback(() => {
handleSelectionChange?.('remove-all');
}, [handleSelectionChange]);

const handleHeaderClick = useCallback(
(columnId: string): void => {
if (columnId === MULTISELECT) {
if (isRangeSelected([0, settings.pageLimit])) {
handleSelectionChange('remove', [0, settings.pageLimit]);
handleSelectionChange?.('remove', [0, settings.pageLimit]);
} else {
handleSelectionChange('add', [0, settings.pageLimit]);
handleSelectionChange?.('add', [0, settings.pageLimit]);
}
}
},
Expand Down Expand Up @@ -967,20 +975,6 @@ const FlatRuns: React.FC<Props> = ({ projectId, workspaceId, searchId }) => {
};
}, [canceler, stopPolling]);

const selectionAction: SelectionAction = useMemo(() => {
return total.match({
_: () => 'NONE' as const,
Loaded: (loadedTotal) => {
if (isRangeSelected([0, settings.pageLimit]) && selectionSize < loadedTotal) {
return 'SELECT_ALL';
} else if (selectionSize > settings.pageLimit) {
return 'CLEAR_SELECTION';
}
return 'NONE';
},
});
}, [isRangeSelected, selectionSize, settings.pageLimit, total]);

return (
<div className={css.content} ref={contentRef}>
<Row>
Expand Down Expand Up @@ -1035,12 +1029,13 @@ const FlatRuns: React.FC<Props> = ({ projectId, workspaceId, searchId }) => {
onActionComplete={onActionComplete}
/>
<LoadableCount
handleSelectionChange={handleSelectionChange}
labelPlural="runs"
labelSingular="run"
pageSize={settings.pageLimit}
selectedCount={selectionSize}
selectionAction={selectionAction}
total={total}
onActualSelectAll={handleActualSelectAll}
onClearSelect={handleClearSelect}
/>
</Row>
</Column>
Expand Down

0 comments on commit e8cca5e

Please sign in to comment.