Skip to content

Commit 0fd1c99

Browse files
Lukas742Lukas742
authored
fix(AnalyticalTable): clear sorting correctly in tree table (#7669)
The actual fix adds `updateOnSortClear` to the dependency array to enforce row rerendering. This commit also merges all `columnDeps` hooks into one for easier maintenance. Fixes #7649
1 parent 0cd95eb commit 0fd1c99

File tree

6 files changed

+110
-39
lines changed

6 files changed

+110
-39
lines changed

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

Lines changed: 78 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -112,24 +112,93 @@ describe('AnalyticalTable', () => {
112112
cy.findByText('Name').click();
113113
cy.get('[ui5-popover]').should('be.visible');
114114
cy.get('[ui5-list]').clickUi5ListItemByText('Sort Ascending');
115-
cy.get('@onSortSpy').should('have.been.calledWithMatch', {
116-
detail: { column: { id: 'name' }, sortDirection: 'asc' },
117-
});
115+
cy.get('@onSortSpy')
116+
.its('lastCall')
117+
.should('have.been.calledWithMatch', {
118+
detail: { column: { id: 'name' }, sortDirection: 'asc' },
119+
});
118120
cy.get('[aria-rowindex="3"] > [aria-colindex="1"]').should('text', 'C');
119121

120122
cy.findByText('Name').click();
121123
cy.get('[ui5-list]').clickUi5ListItemByText('Clear Sorting');
122-
cy.get('@onSortSpy').should('have.been.calledWithMatch', {
123-
detail: { column: { id: 'name' }, sortDirection: 'clear' },
124-
});
124+
cy.get('@onSortSpy')
125+
.its('lastCall')
126+
.should('have.been.calledWithMatch', {
127+
detail: { column: { id: 'name' }, sortDirection: 'clear' },
128+
});
125129
cy.get('[aria-rowindex="3"] > [aria-colindex="1"]').should('text', 'X');
126130

127131
cy.findByText('Name').click();
128132
cy.get('[ui5-list]').clickUi5ListItemByText('Sort Descending');
129-
cy.get('@onSortSpy').should('have.been.calledWithMatch', {
130-
detail: { column: { id: 'name' }, sortDirection: 'desc' },
131-
});
133+
cy.get('@onSortSpy')
134+
.its('lastCall')
135+
.should('have.been.calledWithMatch', {
136+
detail: { column: { id: 'name' }, sortDirection: 'desc' },
137+
});
138+
cy.get('[aria-rowindex="3"] > [aria-colindex="1"]').should('text', 'B');
139+
140+
cy.log('subRows sorting');
141+
const treeData = [
142+
{
143+
category: 'Number',
144+
subRows: [{ category: '2' }, { category: '1' }, { category: '3' }],
145+
},
146+
{
147+
category: 'Alphabet',
148+
subRows: [{ category: 'B' }, { category: 'A' }, { category: 'C' }],
149+
},
150+
];
151+
const treeColumns: AnalyticalTableColumnDefinition[] = [{ Header: 'Category', accessor: 'category' }];
152+
cy.mount(<AnalyticalTable data={treeData} columns={treeColumns} sortable isTreeTable onSort={sort} />);
153+
// expand rows
154+
cy.get('[ui5-button]').click({ multiple: true });
155+
cy.findByText('Category').click();
156+
cy.get('[ui5-list]').clickUi5ListItemByText('Sort Ascending');
157+
cy.get('@onSortSpy')
158+
.its('lastCall')
159+
.should('have.been.calledWithMatch', {
160+
detail: { column: { id: 'category' }, sortDirection: 'asc' },
161+
});
162+
cy.get('[aria-rowindex="1"] > [aria-colindex="1"]').should('text', 'Alphabet');
163+
cy.get('[aria-rowindex="2"] > [aria-colindex="1"]').should('text', 'A');
132164
cy.get('[aria-rowindex="3"] > [aria-colindex="1"]').should('text', 'B');
165+
cy.get('[aria-rowindex="4"] > [aria-colindex="1"]').should('text', 'C');
166+
cy.get('[aria-rowindex="5"] > [aria-colindex="1"]').should('text', 'Number');
167+
cy.get('[aria-rowindex="6"] > [aria-colindex="1"]').should('text', '1');
168+
cy.get('[aria-rowindex="7"] > [aria-colindex="1"]').should('text', '2');
169+
cy.get('[aria-rowindex="8"] > [aria-colindex="1"]').should('text', '3');
170+
171+
cy.findByText('Category').click();
172+
cy.get('[ui5-list]').clickUi5ListItemByText('Sort Descending');
173+
cy.get('@onSortSpy')
174+
.its('lastCall')
175+
.should('have.been.calledWithMatch', {
176+
detail: { column: { id: 'category' }, sortDirection: 'desc' },
177+
});
178+
cy.get('[aria-rowindex="1"] > [aria-colindex="1"]').should('text', 'Number');
179+
cy.get('[aria-rowindex="2"] > [aria-colindex="1"]').should('text', '3');
180+
cy.get('[aria-rowindex="3"] > [aria-colindex="1"]').should('text', '2');
181+
cy.get('[aria-rowindex="4"] > [aria-colindex="1"]').should('text', '1');
182+
cy.get('[aria-rowindex="5"] > [aria-colindex="1"]').should('text', 'Alphabet');
183+
cy.get('[aria-rowindex="6"] > [aria-colindex="1"]').should('text', 'C');
184+
cy.get('[aria-rowindex="7"] > [aria-colindex="1"]').should('text', 'B');
185+
cy.get('[aria-rowindex="8"] > [aria-colindex="1"]').should('text', 'A');
186+
187+
cy.findByText('Category').click();
188+
cy.get('[ui5-list]').clickUi5ListItemByText('Clear Sorting');
189+
cy.get('@onSortSpy')
190+
.its('lastCall')
191+
.should('have.been.calledWithMatch', {
192+
detail: { column: { id: 'category' }, sortDirection: 'clear' },
193+
});
194+
cy.get('[aria-rowindex="1"] > [aria-colindex="1"]').should('text', 'Number');
195+
cy.get('[aria-rowindex="2"] > [aria-colindex="1"]').should('text', '2');
196+
cy.get('[aria-rowindex="3"] > [aria-colindex="1"]').should('text', '1');
197+
cy.get('[aria-rowindex="4"] > [aria-colindex="1"]').should('text', '3');
198+
cy.get('[aria-rowindex="5"] > [aria-colindex="1"]').should('text', 'Alphabet');
199+
cy.get('[aria-rowindex="6"] > [aria-colindex="1"]').should('text', 'B');
200+
cy.get('[aria-rowindex="7"] > [aria-colindex="1"]').should('text', 'A');
201+
cy.get('[aria-rowindex="8"] > [aria-colindex="1"]').should('text', 'C');
133202
});
134203

135204
it('row count modes', () => {

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
import type { ReactTableHooks, TableInstance } from '../types/index.js';
2+
3+
const columnsDeps = (deps, { instance }: { instance: TableInstance }) => {
4+
const { webComponentsReactProperties, state } = instance;
5+
const { selectionMode, selectionBehavior, withRowHighlight, highlightField, withNavigationHighlight, isTreeTable } =
6+
webComponentsReactProperties;
7+
8+
// required as subRows aren't updated when sorting is cleared
9+
const updateOnSortClear = isTreeTable ? state.sortBy.length === 0 : false;
10+
return [
11+
...deps,
12+
selectionMode,
13+
selectionBehavior,
14+
withRowHighlight,
15+
highlightField,
16+
withNavigationHighlight,
17+
updateOnSortClear,
18+
];
19+
};
20+
21+
const visibleColumnsDeps = (deps, { instance }: { instance: TableInstance }) => {
22+
const { webComponentsReactProperties } = instance;
23+
const { selectionMode, selectionBehavior, withRowHighlight, withNavigationHighlight } = webComponentsReactProperties;
24+
return [...deps, selectionMode, selectionBehavior, withRowHighlight, withNavigationHighlight];
25+
};
26+
27+
export const useColumnsDeps = (hooks: ReactTableHooks) => {
28+
hooks.columnsDeps.push(columnsDeps);
29+
hooks.visibleColumnsDeps.push(visibleColumnsDeps);
30+
};

‎packages/main/src/components/AnalyticalTable/hooks/useRowHighlight.tsx‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,6 @@ const Cell = (instance: TableInstance) => {
3131
/*
3232
* TABLE HOOKS
3333
*/
34-
const columnsDeps = (deps, { instance: { webComponentsReactProperties } }: { instance: TableInstance }) => {
35-
return [...deps, webComponentsReactProperties.withRowHighlight, webComponentsReactProperties.highlightField];
36-
};
37-
const visibleColumnsDeps = (deps, { instance }: { instance: TableInstance }) => [
38-
...deps,
39-
instance.webComponentsReactProperties.withRowHighlight,
40-
];
4134
const visibleColumns = (
4235
currentVisibleColumns,
4336
{ instance: { webComponentsReactProperties } }: { instance: TableInstance },
@@ -77,8 +70,6 @@ const columns = (currentColumns, { instance }: { instance: TableInstance }) => {
7770

7871
export const useRowHighlight = (hooks: ReactTableHooks) => {
7972
hooks.columns.push(columns);
80-
hooks.columnsDeps.push(columnsDeps);
81-
hooks.visibleColumnsDeps.push(visibleColumnsDeps);
8273
hooks.visibleColumns.push(visibleColumns);
8374
};
8475

‎packages/main/src/components/AnalyticalTable/hooks/useRowNavigationIndicator.tsx‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,6 @@ const Cell = (instance) => {
2727
/*
2828
* TABLE HOOKS
2929
*/
30-
const columnsDeps = (deps, { instance: { webComponentsReactProperties } }: { instance: TableInstance }) => {
31-
return [...deps, webComponentsReactProperties.withNavigationHighlight];
32-
};
33-
const visibleColumnsDeps = (deps, { instance }: { instance: TableInstance }) => [
34-
...deps,
35-
instance.webComponentsReactProperties.withNavigationHighlight,
36-
];
3730
const visibleColumns = (
3831
currentVisibleColumns,
3932
{ instance: { webComponentsReactProperties } }: { instance: TableInstance },
@@ -72,7 +65,5 @@ const columns = (currentColumns, { instance }: { instance: TableInstance }) => {
7265

7366
export const useRowNavigationIndicators = (hooks: ReactTableHooks) => {
7467
hooks.columns.push(columns);
75-
hooks.columnsDeps.push(columnsDeps);
76-
hooks.visibleColumnsDeps.push(visibleColumnsDeps);
7768
hooks.visibleColumns.push(visibleColumns);
7869
};

‎packages/main/src/components/AnalyticalTable/hooks/useRowSelectionColumn.tsx‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -131,16 +131,6 @@ const headerProps = (props, { instance }: { instance: TableInstance }) => {
131131
return props;
132132
};
133133

134-
const columnDeps = (deps, { instance: { webComponentsReactProperties } }: { instance: TableInstance }) => {
135-
return [...deps, webComponentsReactProperties.selectionMode, webComponentsReactProperties.selectionBehavior];
136-
};
137-
138-
const visibleColumnsDeps = (deps, { instance }: { instance: TableInstance }) => [
139-
...deps,
140-
instance.webComponentsReactProperties.selectionMode,
141-
instance.webComponentsReactProperties.selectionBehavior,
142-
];
143-
144134
const visibleColumns = (
145135
currentVisibleColumns,
146136
{ instance: { webComponentsReactProperties } }: { instance: TableInstance },
@@ -219,8 +209,6 @@ export const useRowSelectionColumn = (hooks: ReactTableHooks) => {
219209
hooks.getToggleRowSelectedProps.push(setToggleRowSelectedProps);
220210
hooks.getToggleAllRowsSelectedProps.push(setToggleAllRowsSelectedProps);
221211
hooks.columns.push(columns);
222-
hooks.columnsDeps.push(columnDeps);
223-
hooks.visibleColumnsDeps.push(visibleColumnsDeps);
224212
hooks.visibleColumns.push(visibleColumns);
225213
};
226214
useRowSelectionColumn.pluginName = 'useRowSelectionColumn';

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ import { TablePlaceholder } from './defaults/LoadingComponent/TablePlaceholder.j
6262
import { DefaultNoDataComponent } from './defaults/NoDataComponent/index.js';
6363
import { useA11y } from './hooks/useA11y.js';
6464
import { useAutoResize } from './hooks/useAutoResize.js';
65+
import { useColumnsDeps } from './hooks/useColumnsDeps.js';
6566
import { useColumnDragAndDrop } from './hooks/useDragAndDrop.js';
6667
import { useDynamicColumnWidths } from './hooks/useDynamicColumnWidths.js';
6768
import { useKeyboardNavigation } from './hooks/useKeyboardNavigation.js';
@@ -267,6 +268,7 @@ const AnalyticalTable = forwardRef<AnalyticalTableDomRef, AnalyticalTablePropTyp
267268
useExpanded,
268269
useRowSelect,
269270
useResizeColumns,
271+
useColumnsDeps,
270272
useResizeColumnsConfig,
271273
useRowSelectionColumn,
272274
useAutoResize,

0 commit comments

Comments
 (0)