Skip to content

Commit 94c1fa9

Browse files
hkang1hkang1
committed
Revert "fix: cover select all edge cases (#10187)"
This reverts commit 11a2581.
1 parent 258455d commit 94c1fa9

File tree

8 files changed

+70
-88
lines changed

8 files changed

+70
-88
lines changed

‎webui/react/src/components/LoadableCount.test.tsx‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import { render, screen } from '@testing-library/react';
22
import { Loaded, NotLoaded } from 'hew/utils/loadable';
3-
import { noop } from 'lodash';
43

54
import LoadableCount from './LoadableCount';
65

@@ -25,11 +24,9 @@ const setup = (totalCount: number, selectedCount?: number, loaded?: boolean) =>
2524
const total = loaded ? Loaded(totalCount) : NotLoaded;
2625
render(
2726
<LoadableCount
28-
handleSelectionChange={noop}
2927
labelPlural={LABEL_PLURAL}
3028
labelSingular={LABEL_SINGULAR}
3129
selectedCount={selectedCount ?? 0}
32-
selectionAction="NONE"
3330
total={total}
3431
/>,
3532
);

‎webui/react/src/components/LoadableCount.tsx‎

Lines changed: 22 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import Button from 'hew/Button';
2-
import { HandleSelectionChangeType } from 'hew/DataGrid/DataGrid';
32
import { Loadable } from 'hew/utils/loadable';
43
import { useMemo } from 'react';
54

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

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

11-
export type SelectionAction = 'SELECT_ALL' | 'CLEAR_SELECTION' | 'NONE';
1210
interface Props {
1311
total: Loadable<number>;
1412
labelSingular: string;
1513
labelPlural: string;
14+
onActualSelectAll?: () => void;
15+
onClearSelect?: () => void;
16+
pageSize?: number;
1617
selectedCount: number;
17-
selectionAction: SelectionAction;
18-
handleSelectionChange: HandleSelectionChangeType;
1918
}
2019

2120
const LoadableCount: React.FC<Props> = ({
2221
total,
2322
labelPlural,
2423
labelSingular,
24+
onActualSelectAll,
25+
onClearSelect,
26+
pageSize = 20,
2527
selectedCount,
26-
selectionAction,
27-
handleSelectionChange,
2828
}: Props) => {
2929
const isMobile = useMobile();
3030

@@ -51,31 +51,25 @@ const LoadableCount: React.FC<Props> = ({
5151
const actualSelectAll = useMemo(() => {
5252
return Loadable.match(total, {
5353
_: () => null,
54-
Loaded: () => {
55-
switch (selectionAction) {
56-
case 'SELECT_ALL': {
57-
const onClick = () => handleSelectionChange('add-all');
58-
return (
59-
<Button data-test="select-all" type="text" onClick={onClick}>
60-
Select all {labelPlural} in table
61-
</Button>
62-
);
63-
}
64-
case 'CLEAR_SELECTION': {
65-
const onClick = () => handleSelectionChange('remove-all');
66-
return (
67-
<Button data-test="clear-selection" type="text" onClick={onClick}>
68-
Clear Selection
69-
</Button>
70-
);
71-
}
72-
case 'NONE': {
73-
return null;
74-
}
54+
Loaded: (loadedTotal) => {
55+
if (onActualSelectAll && selectedCount >= pageSize && selectedCount < loadedTotal) {
56+
return (
57+
<Button data-test="select-all" type="text" onClick={onActualSelectAll}>
58+
Select all {labelPlural} in table
59+
</Button>
60+
);
61+
} else if (onClearSelect && (selectedCount >= pageSize || selectedCount === loadedTotal)) {
62+
return (
63+
<Button data-test="clear-selection" type="text" onClick={onClearSelect}>
64+
Clear Selection
65+
</Button>
66+
);
7567
}
68+
69+
return null;
7670
},
7771
});
78-
}, [labelPlural, handleSelectionChange, selectionAction, total]);
72+
}, [labelPlural, onActualSelectAll, onClearSelect, pageSize, selectedCount, total]);
7973

8074
if (!isMobile) {
8175
return (

‎webui/react/src/components/Searches/Searches.tsx‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -637,6 +637,14 @@ const Searches: React.FC<Props> = ({ project }) => {
637637
users,
638638
]);
639639

640+
const handleActualSelectAll = useCallback(() => {
641+
handleSelectionChange?.('add-all');
642+
}, [handleSelectionChange]);
643+
644+
const handleClearSelect = useCallback(() => {
645+
handleSelectionChange?.('remove-all');
646+
}, [handleSelectionChange]);
647+
640648
const handleHeaderClick = useCallback(
641649
(columnId: string): void => {
642650
if (columnId === MULTISELECT) {
@@ -779,10 +787,8 @@ const Searches: React.FC<Props> = ({ project }) => {
779787
columnGroups={[V1LocationType.EXPERIMENT]}
780788
entityCopy="Show searches…"
781789
formStore={formStore}
782-
handleSelectionChange={handleSelectionChange}
783790
initialVisibleColumns={columnsIfLoaded}
784791
isOpenFilter={isOpenFilter}
785-
isRangeSelected={isRangeSelected}
786792
labelPlural="searches"
787793
labelSingular="search"
788794
pageSize={settings.pageLimit}
@@ -797,6 +803,8 @@ const Searches: React.FC<Props> = ({ project }) => {
797803
total={total}
798804
onActionComplete={handleActionComplete}
799805
onActionSuccess={handleActionSuccess}
806+
onActualSelectAll={handleActualSelectAll}
807+
onClearSelect={handleClearSelect}
800808
onIsOpenFilterChange={handleIsOpenFilterChange}
801809
onRowHeightChange={handleRowHeightChange}
802810
onSortChange={handleSortChange}

‎webui/react/src/components/TableActionBar.tsx‎

Lines changed: 10 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import Button from 'hew/Button';
22
import Column from 'hew/Column';
3-
import { HandleSelectionChangeType, Sort } from 'hew/DataGrid/DataGrid';
3+
import { Sort } from 'hew/DataGrid/DataGrid';
44
import Dropdown, { MenuItem } from 'hew/Dropdown';
55
import Icon, { IconName } from 'hew/Icon';
66
import { useModal } from 'hew/Modal';
@@ -57,7 +57,7 @@ import { capitalizeWord } from 'utils/string';
5757
import { openCommandResponse } from 'utils/wait';
5858

5959
import { FilterFormSet } from './FilterForm/components/type';
60-
import LoadableCount, { SelectionAction } from './LoadableCount';
60+
import LoadableCount from './LoadableCount';
6161
import css from './TableActionBar.module.scss';
6262

6363
const batchActions = [
@@ -91,22 +91,22 @@ const actionIcons: Record<BatchAction, IconName> = {
9191
interface Props {
9292
compareViewOn?: boolean;
9393
formStore: FilterFormStore;
94-
handleSelectionChange: HandleSelectionChangeType;
9594
heatmapBtnVisible?: boolean;
9695
heatmapOn?: boolean;
9796
initialVisibleColumns: string[];
9897
isOpenFilter: boolean;
99-
isRangeSelected: (range: [number, number]) => boolean;
10098
onActionComplete?: () => Promise<void>;
10199
onActionSuccess?: (action: BatchAction, successfulIds: number[]) => void;
100+
onActualSelectAll?: () => void;
101+
onClearSelect?: () => void;
102102
onComparisonViewToggle?: () => void;
103103
onHeatmapToggle?: (heatmapOn: boolean) => void;
104104
onIsOpenFilterChange?: (value: boolean) => void;
105105
onRowHeightChange?: (rowHeight: RowHeight) => void;
106106
onSortChange?: (sorts: Sort[]) => void;
107107
onVisibleColumnChange?: (newColumns: string[], pinnedCount?: number) => void;
108108
onHeatmapSelectionRemove?: (id: string) => void;
109-
pageSize: number;
109+
pageSize?: number;
110110
project: Project;
111111
projectColumns: Loadable<ProjectColumn[]>;
112112
rowHeight: RowHeight;
@@ -129,14 +129,14 @@ const TableActionBar: React.FC<Props> = ({
129129
compareViewOn,
130130
formStore,
131131
tableFilterString,
132-
handleSelectionChange,
133132
heatmapBtnVisible,
134133
heatmapOn,
135134
initialVisibleColumns,
136135
isOpenFilter,
137-
isRangeSelected,
138136
onActionComplete,
139137
onActionSuccess,
138+
onActualSelectAll,
139+
onClearSelect,
140140
onComparisonViewToggle,
141141
onHeatmapToggle,
142142
onIsOpenFilterChange,
@@ -412,20 +412,6 @@ const TableActionBar: React.FC<Props> = ({
412412
}, [] as MenuItem[]);
413413
}, [availableBatchActions]);
414414

415-
const selectionAction: SelectionAction = useMemo(() => {
416-
return total.match({
417-
_: () => 'NONE' as const,
418-
Loaded: (loadedTotal) => {
419-
if (isRangeSelected([0, pageSize]) && selectionSize < loadedTotal) {
420-
return 'SELECT_ALL';
421-
} else if (selectionSize > pageSize) {
422-
return 'CLEAR_SELECTION';
423-
}
424-
return 'NONE';
425-
},
426-
});
427-
}, [isRangeSelected, selectionSize, pageSize, total]);
428-
429415
return (
430416
<div className={css.base} data-test-component="tableActionBar">
431417
<Row>
@@ -469,12 +455,13 @@ const TableActionBar: React.FC<Props> = ({
469455
</Dropdown>
470456
)}
471457
<LoadableCount
472-
handleSelectionChange={handleSelectionChange}
473458
labelPlural={labelPlural}
474459
labelSingular={labelSingular}
460+
pageSize={pageSize}
475461
selectedCount={selectionSize}
476-
selectionAction={selectionAction}
477462
total={total}
463+
onActualSelectAll={onActualSelectAll}
464+
onClearSelect={onClearSelect}
478465
/>
479466
</Row>
480467
</Column>

‎webui/react/src/e2e/models/components/TableActionBar.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ export class TableActionBar extends NamedComponent {
2626
heatmapToggle = new BaseComponent({ parent: this, selector: '[data-test="heatmapToggle"]' });
2727
compare = new BaseComponent({ parent: this, selector: '[data-test="compare"]' });
2828
clearSelection = new BaseComponent({ parent: this, selector: '[data-test="clear-selection"]' });
29-
selectAll = new BaseComponent({ parent: this, selector: '[data-test="select-all"]' });
3029
// TODO a bunch of modals
3130
}
3231

‎webui/react/src/e2e/tests/experimentList.spec.ts‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -65,15 +65,10 @@ test.describe('Experiment List', () => {
6565
const count = await getCount();
6666
if (count !== 0) {
6767
await grid.headRow.clickSelectHeader();
68-
const selectAllButton = projectDetailsPage.f_experimentList.tableActionBar.selectAll;
69-
const clearAllButton = projectDetailsPage.f_experimentList.tableActionBar.clearSelection;
70-
if (await selectAllButton.pwLocator.isVisible()) {
71-
await selectAllButton.pwLocator.click();
72-
await clearAllButton.pwLocator.click();
73-
} else if (await clearAllButton.pwLocator.isVisible()) {
74-
await clearAllButton.pwLocator.click();
75-
} else {
76-
await grid.headRow.clickSelectHeader();
68+
const isClearSelectionVisible =
69+
await projectDetailsPage.f_experimentList.tableActionBar.clearSelection.pwLocator.isVisible();
70+
if (isClearSelectionVisible) {
71+
await projectDetailsPage.f_experimentList.tableActionBar.clearSelection.pwLocator.click();
7772
}
7873
}
7974
});

‎webui/react/src/pages/F_ExpList/F_ExperimentList.tsx‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -487,6 +487,14 @@ const F_ExperimentList: React.FC<Props> = ({ project }) => {
487487
[handleSelectionChange, openToast],
488488
);
489489

490+
const handleActualSelectAll = useCallback(() => {
491+
handleSelectionChange?.('add-all');
492+
}, [handleSelectionChange]);
493+
494+
const handleClearSelect = useCallback(() => {
495+
handleSelectionChange?.('remove-all');
496+
}, [handleSelectionChange]);
497+
490498
const handleContextMenuComplete = useCallback(
491499
(action: ExperimentAction, id: number, data?: Partial<BulkExperimentItem>) =>
492500
handleActionSuccess(action, [id], data),
@@ -979,15 +987,12 @@ const F_ExperimentList: React.FC<Props> = ({ project }) => {
979987
compareViewOn={settings.compare}
980988
entityCopy="Show experiments…"
981989
formStore={formStore}
982-
handleSelectionChange={handleSelectionChange}
983990
heatmapBtnVisible={heatmapBtnVisible}
984991
heatmapOn={settings.heatmapOn}
985992
initialVisibleColumns={columnsIfLoaded}
986993
isOpenFilter={isOpenFilter}
987-
isRangeSelected={isRangeSelected}
988994
labelPlural="experiments"
989995
labelSingular="experiment"
990-
pageSize={settings.pageLimit}
991996
pinnedColumnsCount={settings.pinnedColumnsCount}
992997
project={project}
993998
projectColumns={projectColumns}
@@ -1000,6 +1005,8 @@ const F_ExperimentList: React.FC<Props> = ({ project }) => {
10001005
total={total}
10011006
onActionComplete={handleActionComplete}
10021007
onActionSuccess={handleActionSuccess}
1008+
onActualSelectAll={handleActualSelectAll}
1009+
onClearSelect={handleClearSelect}
10031010
onComparisonViewToggle={handleToggleComparisonView}
10041011
onHeatmapSelectionRemove={(id) => {
10051012
const newSelection = settings.heatmapSkipped.filter((s) => s !== id);

‎webui/react/src/pages/FlatRuns/FlatRuns.tsx‎

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ import {
4646
SpecialColumnNames,
4747
} from 'components/FilterForm/components/type';
4848
import TableFilter from 'components/FilterForm/TableFilter';
49-
import LoadableCount, { SelectionAction } from 'components/LoadableCount';
49+
import LoadableCount from 'components/LoadableCount';
5050
import MultiSortMenu, { EMPTY_SORT, sortMenuItemsForColumn } from 'components/MultiSortMenu';
5151
import { OptionsMenu, RowHeight } from 'components/OptionsMenu';
5252
import {
@@ -779,13 +779,21 @@ const FlatRuns: React.FC<Props> = ({ projectId, workspaceId, searchId }) => {
779779
[updateSettings],
780780
);
781781

782+
const handleActualSelectAll = useCallback(() => {
783+
handleSelectionChange?.('add-all');
784+
}, [handleSelectionChange]);
785+
786+
const handleClearSelect = useCallback(() => {
787+
handleSelectionChange?.('remove-all');
788+
}, [handleSelectionChange]);
789+
782790
const handleHeaderClick = useCallback(
783791
(columnId: string): void => {
784792
if (columnId === MULTISELECT) {
785793
if (isRangeSelected([0, settings.pageLimit])) {
786-
handleSelectionChange('remove', [0, settings.pageLimit]);
794+
handleSelectionChange?.('remove', [0, settings.pageLimit]);
787795
} else {
788-
handleSelectionChange('add', [0, settings.pageLimit]);
796+
handleSelectionChange?.('add', [0, settings.pageLimit]);
789797
}
790798
}
791799
},
@@ -967,20 +975,6 @@ const FlatRuns: React.FC<Props> = ({ projectId, workspaceId, searchId }) => {
967975
};
968976
}, [canceler, stopPolling]);
969977

970-
const selectionAction: SelectionAction = useMemo(() => {
971-
return total.match({
972-
_: () => 'NONE' as const,
973-
Loaded: (loadedTotal) => {
974-
if (isRangeSelected([0, settings.pageLimit]) && selectionSize < loadedTotal) {
975-
return 'SELECT_ALL';
976-
} else if (selectionSize > settings.pageLimit) {
977-
return 'CLEAR_SELECTION';
978-
}
979-
return 'NONE';
980-
},
981-
});
982-
}, [isRangeSelected, selectionSize, settings.pageLimit, total]);
983-
984978
return (
985979
<div className={css.content} ref={contentRef}>
986980
<Row>
@@ -1035,12 +1029,13 @@ const FlatRuns: React.FC<Props> = ({ projectId, workspaceId, searchId }) => {
10351029
onActionComplete={onActionComplete}
10361030
/>
10371031
<LoadableCount
1038-
handleSelectionChange={handleSelectionChange}
10391032
labelPlural="runs"
10401033
labelSingular="run"
1034+
pageSize={settings.pageLimit}
10411035
selectedCount={selectionSize}
1042-
selectionAction={selectionAction}
10431036
total={total}
1037+
onActualSelectAll={handleActualSelectAll}
1038+
onClearSelect={handleClearSelect}
10441039
/>
10451040
</Row>
10461041
</Column>

0 commit comments

Comments
 (0)