Conversation
There was a problem hiding this comment.
Pull request overview
Refactors the DataGrid sticky columns + grouping screenshot coverage (TestCafe) by splitting the previous “horizontal scroll” scenario into two separate screenshot tests and adjusting the widget configuration used to reproduce the overlap/intersection cases.
Changes:
- Removed the
unstablemeta flag and the runtimeapiOption('summary.totalItems', ...)mutation + repeated scroll/waits from the existing test. - Introduced a second dedicated test case for the “total summary” scenario and adjusted
customizeColumns/summaryconfiguration for both cases. - Renamed the original test title to explicitly call out the “intersection” scenario.
| @@ -291,31 +291,15 @@ test('DataGrid - Group row content is scrolled if repaintChangesOnly is enabled | |||
| // T1284612 | |||
| test.meta({ | |||
| browserSize: [900, 800], | |||
There was a problem hiding this comment.
Check, please, whether we need custom browserSize for these test.
We set width and height for DataGrid in its configuration, then make screenshot of the grid only, looks like custom window size not used
| customizeColumns(columns) { | ||
| columns[2].groupIndex = 0; | ||
| columns[1].visible = false; | ||
| columns[4].width = 150; |
There was a problem hiding this comment.
Suggestion: return back previous logic with changing grid option in runtime to change summary.totalItelm and column[index].width. This will save us time on initial page render
There was a problem hiding this comment.
I do not agree. It is difficult to create one config for 4 cases. Also I do not think initial render takes a lot of time.
| totalItems: [{ | ||
| column: 'SaleAmount', | ||
| summaryType: 'max', | ||
| valueFormat: 'currency', |
There was a problem hiding this comment.
The total summary displayFormat string MAXMAXMAXMAX: {0} is hard to interpret and looks like a placeholder. If the intent is to force a long label for layout coverage, consider using a clearer long label (or add a brief inline comment explaining why this specific text is needed) to keep the test configuration self-explanatory.
| valueFormat: 'currency', | |
| valueFormat: 'currency', | |
| // Use a deliberately long caption to verify layout/width handling of total summary text. |
No description provided.