Skip to content

Commit 1cb2454

Browse files
Lukas742Lukas742
authored
fix(AnalyticalTable): don't include empty rows in internal functions (#4196)
This PR excludes empty rows from all table calculations, like filtering, sorting, selection, etc.
1 parent ea21252 commit 1cb2454

File tree

11 files changed

+146
-77
lines changed

11 files changed

+146
-77
lines changed

‎packages/main/src/components/AnalyticalTable/AnalyticalTable.cy.tsx

Lines changed: 59 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1066,16 +1066,24 @@ describe('AnalyticalTable', () => {
10661066
it('Alternate Row Color', () => {
10671067
const standardRowColor = cssVarToRgb(ThemingParameters.sapList_Background);
10681068
const alternatingRowColor = cssVarToRgb(ThemingParameters.sapList_AlternatingBackground);
1069-
cy.mount(<AnalyticalTable data={data} columns={columns} alternateRowColor />);
1069+
cy.mount(<AnalyticalTable data={data} columns={columns} alternateRowColor minRows={7} />);
10701070
cy.get('[data-component-name="AnalyticalTableContainer"]').should('have.css', 'background-color', standardRowColor);
1071-
for (let i = 1; i <= 5; i++) {
1071+
for (let i = 1; i <= 4; i++) {
10721072
if (i % 2) {
10731073
// no color set
10741074
cy.get(`[aria-rowindex="${i}"]`).should('have.css', 'background-color', 'rgba(0, 0, 0, 0)');
10751075
} else {
10761076
cy.get(`[aria-rowindex="${i}"]`).should('have.css', 'background-color', alternatingRowColor);
10771077
}
10781078
}
1079+
cy.get('[data-empty-row="true"]').each(($emptyRow, i) => {
1080+
if ((i + 1) % 2) {
1081+
// no color set
1082+
cy.wrap($emptyRow).should('have.css', 'background-color', 'rgba(0, 0, 0, 0)');
1083+
} else {
1084+
cy.wrap($emptyRow).should('have.css', 'background-color', alternatingRowColor);
1085+
}
1086+
});
10791087
});
10801088

10811089
it('initial column order', () => {
@@ -1529,6 +1537,55 @@ describe('AnalyticalTable', () => {
15291537
});
15301538
});
15311539

1540+
it('empty rows', () => {
1541+
const ShowSelectedComp = () => {
1542+
const instance = useRef(null);
1543+
const [selected, setSelected] = useState({});
1544+
return (
1545+
<>
1546+
<Button
1547+
onClick={() => {
1548+
setSelected(instance.current?.state.selectedRowIds);
1549+
}}
1550+
>
1551+
Show Selected
1552+
</Button>
1553+
<AnalyticalTable
1554+
selectionMode={AnalyticalTableSelectionMode.MultiSelect}
1555+
data={data}
1556+
columns={columns}
1557+
tableInstance={instance}
1558+
/>
1559+
Selected: {JSON.stringify(selected)}
1560+
</>
1561+
);
1562+
};
1563+
cy.mount(<AnalyticalTable data={[]} columns={columns} />);
1564+
cy.get('[data-empty-row="true"]').should('not.exist');
1565+
cy.mount(<AnalyticalTable data={[...data, ...data]} columns={columns} />);
1566+
cy.get('[data-empty-row="true"]').should('not.exist');
1567+
cy.mount(
1568+
<AnalyticalTable
1569+
data={dataTree}
1570+
columns={columns}
1571+
isTreeTable
1572+
reactTableOptions={{ initialState: { expanded: { 1: true } } }}
1573+
/>
1574+
);
1575+
cy.get('[data-empty-row="true"]').should('not.exist');
1576+
cy.mount(<AnalyticalTable data={data} columns={columns} minRows={15} />);
1577+
cy.get('[data-empty-row="true"]').should('have.length', 11);
1578+
cy.mount(<AnalyticalTable data={dataTree} columns={columns} isTreeTable />);
1579+
cy.get('[data-empty-row="true"]').should('have.length', 3);
1580+
cy.mount(<ShowSelectedComp />);
1581+
cy.get('#__ui5wcr__internal_selection_column').click();
1582+
cy.findByText('Show Selected').click();
1583+
cy.findByText('Selected: {"0":true,"1":true,"2":true,"3":true}').should('be.visible');
1584+
cy.get('[data-empty-row="true"]').click();
1585+
cy.findByText('Show Selected').click();
1586+
cy.findByText('Selected: {"0":true,"1":true,"2":true,"3":true}').should('be.visible');
1587+
});
1588+
15321589
cypressPassThroughTestsFactory(AnalyticalTable, { data, columns });
15331590
});
15341591

‎packages/main/src/components/AnalyticalTable/AnayticalTable.jss.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ const styles = {
125125
marginBottom: '-1px',
126126
boxSizing: 'border-box',
127127
display: 'flex',
128-
'&:hover': {
128+
'&:hover:not([data-empty-row])': {
129129
backgroundColor: ThemingParameters.sapList_Hover_Background
130130
},
131131
'&[data-is-selected]': {
@@ -187,7 +187,10 @@ const styles = {
187187
position: 'relative',
188188
'&:focus': {
189189
outlineOffset: `calc(-1 * ${ThemingParameters.sapContent_FocusWidth})`,
190-
outline: `${ThemingParameters.sapContent_FocusWidth} ${ThemingParameters.sapContent_FocusStyle} ${ThemingParameters.sapContent_FocusColor}`
190+
outline: `${ThemingParameters.sapContent_FocusWidth} ${ThemingParameters.sapContent_FocusStyle} ${ThemingParameters.sapContent_FocusColor}`,
191+
'&[data-empty-row-cell]': {
192+
outline: 'none'
193+
}
191194
}
192195
},
193196
noDataContainer: {

‎packages/main/src/components/AnalyticalTable/TableBody/EmptyRow.tsx

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
import { VirtualItem } from '@tanstack/react-virtual';
2+
import React, { ReactNode } from 'react';
3+
4+
export const EmptyRow = ({
5+
virtualRow,
6+
className,
7+
children
8+
}: {
9+
virtualRow: VirtualItem<any>;
10+
className: string;
11+
children?: ReactNode;
12+
}) => {
13+
return (
14+
<div
15+
data-empty-row="true"
16+
key={`empty_row_${virtualRow.index}`}
17+
className={className}
18+
style={{
19+
height: `${virtualRow.size}px`,
20+
transform: `translateY(${virtualRow.start}px)`,
21+
boxSizing: 'border-box'
22+
}}
23+
>
24+
{children}
25+
</div>
26+
);
27+
};

‎packages/main/src/components/AnalyticalTable/TableBody/VirtualTableBody.tsx

Lines changed: 45 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import { useVirtualizer } from '@tanstack/react-virtual';
2-
import React, { MutableRefObject, ReactNode, useCallback, useMemo } from 'react';
2+
import { clsx } from 'clsx';
3+
import React, { MutableRefObject, ReactNode, useCallback, useMemo, useRef } from 'react';
34
import { ScrollToRefType } from '../interfaces';
5+
import { EmptyRow } from './EmptyRow';
46
import { RowSubComponent as SubComponent } from './RowSubComponent';
57

68
interface VirtualTableBodyProps {
@@ -30,6 +32,7 @@ const measureElement = (el) => el.offsetHeight;
3032

3133
export const VirtualTableBody = (props: VirtualTableBodyProps) => {
3234
const {
35+
alternateRowColor,
3336
classes,
3437
prepareRow,
3538
rows,
@@ -54,6 +57,7 @@ export const VirtualTableBody = (props: VirtualTableBodyProps) => {
5457
const itemCount = Math.max(minRows, rows.length);
5558
const overscan = overscanCount ? overscanCount : Math.floor(visibleRows / 2);
5659
const rowHeight = popInRowHeight !== internalRowHeight ? popInRowHeight : internalRowHeight;
60+
const lastNonEmptyRow = useRef(null);
5761

5862
const rowVirtualizer = useVirtualizer({
5963
count: itemCount,
@@ -103,17 +107,48 @@ export const VirtualTableBody = (props: VirtualTableBodyProps) => {
103107
const row = rows[virtualRow.index];
104108
const rowIndexWithHeader = virtualRow.index + 1;
105109
if (!row || row.groupByVal === 'undefined') {
110+
const alternate = alternateRowColor && virtualRow.index % 2 !== 0;
111+
if (!lastNonEmptyRow.current?.cells) {
112+
return (
113+
<EmptyRow
114+
key={`empty_row_${virtualRow.index}`}
115+
virtualRow={virtualRow}
116+
className={clsx(classes.tr, alternate && classes.alternateRowColor)}
117+
/>
118+
);
119+
}
120+
const cells = lastNonEmptyRow.current.cells;
121+
106122
return (
107-
<div
123+
<EmptyRow
108124
key={`empty_row_${virtualRow.index}`}
109-
className={classes.tr}
110-
style={{
111-
height: `${virtualRow.size}px`,
112-
transform: `translateY(${virtualRow.start}px)`,
113-
boxSizing: 'border-box'
114-
}}
115-
/>
125+
virtualRow={virtualRow}
126+
className={clsx(classes.tr, alternate && classes.alternateRowColor)}
127+
>
128+
{cells.map((item) => {
129+
const cellProps = item.getCellProps();
130+
const {
131+
'aria-colindex': _0,
132+
'aria-selected': _1,
133+
'aria-label': _2,
134+
tabIndex: _3,
135+
...emptyRowCellProps
136+
} = cellProps;
137+
return (
138+
<div
139+
{...emptyRowCellProps}
140+
key={`${visibleRowIndex}-${emptyRowCellProps.key}`}
141+
data-empty-row-cell="true"
142+
tabIndex={-1}
143+
aria-hidden
144+
style={{ ...emptyRowCellProps.style, cursor: 'unset' }}
145+
/>
146+
);
147+
})}
148+
</EmptyRow>
116149
);
150+
} else {
151+
lastNonEmptyRow.current = row;
117152
}
118153
prepareRow(row);
119154
const rowProps = row.getRowProps();
@@ -153,7 +188,7 @@ export const VirtualTableBody = (props: VirtualTableBodyProps) => {
153188
}}
154189
aria-rowindex={virtualRow.index + 1}
155190
>
156-
{!row.original?.emptyRow && RowSubComponent && (row.isExpanded || alwaysShowSubComponent) && (
191+
{RowSubComponent && (row.isExpanded || alwaysShowSubComponent) && (
157192
<SubComponent
158193
subComponentsHeight={subComponentsHeight}
159194
virtualRow={virtualRow}
@@ -193,10 +228,6 @@ export const VirtualTableBody = (props: VirtualTableBodyProps) => {
193228
...directionStyles
194229
}
195230
};
196-
if (row.original?.emptyRow) {
197-
// eslint-disable-next-line react/jsx-key
198-
return <div {...allCellProps} />;
199-
}
200231
let contentToRender;
201232
if (
202233
cell.column.id === '__ui5wcr__internal_highlight_column' ||

‎packages/main/src/components/AnalyticalTable/hooks/useA11y.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,6 @@ const getCellProps = (cellProps, { cell: { column, row, value }, instance }) =>
1414
instance.webComponentsReactProperties;
1515
const updatedCellProps: UpdatedCellProptypes = { 'aria-colindex': columnIndex + 1 }; // aria index is 1 based, not 0
1616

17-
if (row.original?.emptyRow) {
18-
return [cellProps, updatedCellProps];
19-
}
20-
2117
const RowSubComponent = typeof renderRowSubComponent === 'function' ? renderRowSubComponent(row) : undefined;
2218
const rowIsExpandable = row.canExpand || (RowSubComponent && !alwaysShowSubComponent);
2319

‎packages/main/src/components/AnalyticalTable/hooks/useKeyboardNavigation.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,9 @@ const getTableProps = (tableProps, { instance: { webComponentsReactProperties, d
8080

8181
const onTableFocus = useCallback(
8282
(e) => {
83+
if (e.target.dataset?.emptyRowCell === 'true') {
84+
return;
85+
}
8386
const isFirstCellAvailable = e.target.querySelector('div[data-column-index="0"][data-row-index="1"]');
8487
if (e.target.dataset.componentName === 'AnalyticalTableContainer') {
8588
e.target.tabIndex = -1;

‎packages/main/src/components/AnalyticalTable/hooks/useSingleRowStateSelection.ts

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,6 @@ const getRowProps = (rowProps, { row, instance }) => {
1313
return;
1414
}
1515

16-
// don't select empty rows
17-
const isEmptyRow = row.original?.emptyRow;
18-
if (isEmptyRow) {
19-
return;
20-
}
21-
2216
// don't select grouped rows
2317
if (row.isGrouped) {
2418
return;

‎packages/main/src/components/AnalyticalTable/hooks/useStyling.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,11 @@ const ROW_SELECTION_ATTRIBUTE = 'data-is-selected';
4646
const getRowProps = (rowProps, { instance, row }) => {
4747
const { webComponentsReactProperties } = instance;
4848
const { classes, selectionBehavior, selectionMode, alternateRowColor } = webComponentsReactProperties;
49-
const isEmptyRow = row.original?.emptyRow;
5049
let className = classes.tr;
51-
const rowCanBeSelected =
52-
[AnalyticalTableSelectionMode.SingleSelect, AnalyticalTableSelectionMode.MultiSelect].includes(selectionMode) &&
53-
!isEmptyRow;
50+
const rowCanBeSelected = [
51+
AnalyticalTableSelectionMode.SingleSelect,
52+
AnalyticalTableSelectionMode.MultiSelect
53+
].includes(selectionMode);
5454

5555
if (row.isGrouped) {
5656
className += ` ${classes.tableGroupHeader}`;

‎packages/main/src/components/AnalyticalTable/index.tsx

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ import { VirtualTableBody } from './TableBody/VirtualTableBody';
8888
import { VirtualTableBodyContainer } from './TableBody/VirtualTableBodyContainer';
8989
import { stateReducer } from './tableReducer/stateReducer';
9090
import { TitleBar } from './TitleBar';
91-
import { orderByFn, tagNamesWhichShouldNotSelectARow } from './util';
91+
import { tagNamesWhichShouldNotSelectARow } from './util';
9292
import { VerticalResizer } from './VerticalResizer';
9393

9494
export interface AnalyticalTableColumnDefinition {
@@ -650,27 +650,13 @@ const AnalyticalTable = forwardRef<AnalyticalTableDomRef, AnalyticalTablePropTyp
650650

651651
const getSubRows = useCallback((row) => row.subRows || row[subRowsKey] || [], [subRowsKey]);
652652

653-
const data = useMemo(() => {
654-
if (rawData.length === 0) {
655-
return rawData;
656-
}
657-
if (minRows > rawData.length) {
658-
const missingRows: number = minRows - rawData.length;
659-
const emptyRows = Array.from({ length: missingRows }, (v, i) => i).map(() => ({ emptyRow: true }));
660-
661-
return [...rawData, ...emptyRows];
662-
}
663-
return rawData;
664-
}, [rawData, minRows]);
665-
666653
const invalidTableA11yText = i18nBundle.getText(INVALID_TABLE);
667654
const tableInstanceRef = useRef<Record<string, any>>(null);
668655
tableInstanceRef.current = useTable(
669656
{
670657
columns,
671-
data,
658+
data: rawData,
672659
defaultColumn: DefaultColumn,
673-
orderByFn,
674660
getSubRows,
675661
stateReducer,
676662
disableFilters: !filterable,

‎packages/main/src/components/AnalyticalTable/pluginHooks/useIndeterminateRowSelection.tsx

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -98,11 +98,7 @@ export const useIndeterminateRowSelection = (onIndeterminateChange?: onIndetermi
9898
};
9999
}
100100

101-
const indeterminateRowsById = getIndeterminate(
102-
rows.filter((item) => !item.original.emptyRow),
103-
rowsById,
104-
state
105-
);
101+
const indeterminateRowsById = getIndeterminate(rows, rowsById, state);
106102

107103
return {
108104
...newState,

‎packages/main/src/components/AnalyticalTable/util/index.ts

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,6 @@
11
import { CSSProperties } from 'react';
2-
import { defaultOrderByFn } from 'react-table';
32
import { TextAlign, VerticalAlign } from '../../../enums';
43

5-
export const orderByFn = (rows, functions, directions) => {
6-
const wrapSortFn = (sortFn, index) => {
7-
const desc = directions[index] === false || directions[index] === 'desc';
8-
9-
return (rowA, rowB) => {
10-
if (rowA.original?.emptyRow && !rowB.original?.emptyRow) {
11-
return desc ? -1 : 1;
12-
}
13-
if (!rowA.original?.emptyRow && rowB.original?.emptyRow) {
14-
return desc ? 1 : -1;
15-
}
16-
if (rowA.original?.emptyRow && rowB.original?.emptyRow) {
17-
return 0;
18-
}
19-
return sortFn(rowA, rowB);
20-
};
21-
};
22-
23-
const wrappedSortfunctions = functions.map(wrapSortFn);
24-
25-
return defaultOrderByFn(rows, wrappedSortfunctions, directions);
26-
};
27-
284
// copied from https://github.com/tannerlinsley/react-table/blob/f97fb98509d0b27cc0bebcf3137872afe4f2809e/src/utils.js#L320-L347 (13. Jan 2021)
295
const reOpenBracket = /\[/g;
306
const reCloseBracket = /]/g;

0 commit comments

Comments
 (0)