-
Notifications
You must be signed in to change notification settings - Fork 664
T1311329 - DataGrid - Column chooser hides a banded column on using search and recursive selection #32186
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 26_1
Are you sure you want to change the base?
T1311329 - DataGrid - Column chooser hides a banded column on using search and recursive selection #32186
Changes from all commits
561a831
0883820
fad874a
c42163c
fd3f78c
de65c34
659c3a1
9037432
67e0801
772895b
9779083
24d5b5c
779d885
26523fe
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| import { TreeViewModel } from '@ts/ui/__tests__/__mock__/model/tree_view'; | ||
|
|
||
| const CLASSES = { | ||
| columnChooser: 'dx-datagrid-column-chooser', | ||
| columnChooserList: 'dx-datagrid-column-chooser-list', | ||
| popupWrapper: 'dx-popup-wrapper', | ||
| }; | ||
|
|
||
| export class ColumnChooserModel { | ||
| constructor(protected readonly root: HTMLElement) {} | ||
|
|
||
| private getPopupWrapper(): HTMLElement | null { | ||
| return document.body.querySelector(`.${CLASSES.popupWrapper}.${CLASSES.columnChooser}`); | ||
| } | ||
|
|
||
| private getOverlay(): HTMLElement | null { | ||
| const wrapper = this.getPopupWrapper(); | ||
| return wrapper?.querySelector('.dx-overlay-content') ?? null; | ||
| } | ||
|
|
||
| private getTreeView(): TreeViewModel | null { | ||
| const overlay = this.getOverlay(); | ||
| if (!overlay) return null; | ||
|
|
||
| const treeViewElement = overlay.querySelector(`.${CLASSES.columnChooserList}`) as HTMLElement; | ||
| return treeViewElement ? new TreeViewModel(treeViewElement) : null; | ||
Raushen marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| public isVisible(): boolean { | ||
| return this.getOverlay() !== null; | ||
| } | ||
|
|
||
| public searchColumn(text: string): void { | ||
| const treeView = this.getTreeView(); | ||
| treeView?.setSearchValue(text); | ||
| } | ||
|
|
||
| public toggleColumn(columnText: string): void { | ||
| const treeView = this.getTreeView(); | ||
| const checkBox = treeView?.getCheckboxByText(columnText); | ||
| checkBox?.toggle(); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,307 @@ | ||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||
| afterEach, beforeEach, describe, expect, it, jest, | ||||||||||||||||||||||||||||||||||||
| } from '@jest/globals'; | ||||||||||||||||||||||||||||||||||||
| import type { dxElementWrapper } from '@js/core/renderer'; | ||||||||||||||||||||||||||||||||||||
| import $ from '@js/core/renderer'; | ||||||||||||||||||||||||||||||||||||
| import type { Properties as DataGridProperties } from '@js/ui/data_grid'; | ||||||||||||||||||||||||||||||||||||
| import DataGrid from '@js/ui/data_grid'; | ||||||||||||||||||||||||||||||||||||
| import errors from '@js/ui/widget/ui.errors'; | ||||||||||||||||||||||||||||||||||||
| import { DataGridModel } from '@ts/grids/data_grid/__tests__/__mock__/model/data_grid'; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| const SELECTORS = { | ||||||||||||||||||||||||||||||||||||
| gridContainer: '#gridContainer', | ||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| const GRID_CONTAINER_ID = 'gridContainer'; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| const createDataGrid = async ( | ||||||||||||||||||||||||||||||||||||
| options: DataGridProperties = {}, | ||||||||||||||||||||||||||||||||||||
| ): Promise<{ | ||||||||||||||||||||||||||||||||||||
| $container: dxElementWrapper; | ||||||||||||||||||||||||||||||||||||
| component: DataGridModel; | ||||||||||||||||||||||||||||||||||||
| instance: DataGrid; | ||||||||||||||||||||||||||||||||||||
| }> => new Promise((resolve) => { | ||||||||||||||||||||||||||||||||||||
| const $container = $('<div>') | ||||||||||||||||||||||||||||||||||||
| .attr('id', GRID_CONTAINER_ID) | ||||||||||||||||||||||||||||||||||||
| .appendTo(document.body); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| const dataGridOptions: DataGridProperties = { | ||||||||||||||||||||||||||||||||||||
| keyExpr: 'id', | ||||||||||||||||||||||||||||||||||||
| ...options, | ||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| const instance = new DataGrid($container.get(0) as HTMLDivElement, dataGridOptions); | ||||||||||||||||||||||||||||||||||||
| const component = new DataGridModel($container.get(0) as HTMLElement); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| jest.runAllTimers(); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| resolve({ | ||||||||||||||||||||||||||||||||||||
| $container, | ||||||||||||||||||||||||||||||||||||
| component, | ||||||||||||||||||||||||||||||||||||
| instance, | ||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| const beforeTest = (): void => { | ||||||||||||||||||||||||||||||||||||
| jest.useFakeTimers(); | ||||||||||||||||||||||||||||||||||||
| jest.spyOn(errors, 'log').mockImplementation(jest.fn()); | ||||||||||||||||||||||||||||||||||||
| jest.spyOn(errors, 'Error').mockImplementation(() => ({})); | ||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| const afterTest = (): void => { | ||||||||||||||||||||||||||||||||||||
| const $container = $(SELECTORS.gridContainer); | ||||||||||||||||||||||||||||||||||||
| const dataGrid = ($container as any).dxDataGrid('instance') as DataGrid; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| dataGrid.dispose(); | ||||||||||||||||||||||||||||||||||||
| $container.remove(); | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+53
to
+56
|
||||||||||||||||||||||||||||||||||||
| const dataGrid = ($container as any).dxDataGrid('instance') as DataGrid; | |
| dataGrid.dispose(); | |
| $container.remove(); | |
| if ($container.length) { | |
| const hasDxDataGrid = typeof ($container as any).dxDataGrid === 'function'; | |
| const dataGrid = hasDxDataGrid | |
| ? ($container as any).dxDataGrid('instance') as DataGrid | undefined | |
| : undefined; | |
| if (dataGrid) { | |
| dataGrid.dispose(); | |
| } | |
| $container.remove(); | |
| } |
anna-shakhova marked this conversation as resolved.
Show resolved
Hide resolved
Raushen marked this conversation as resolved.
Show resolved
Hide resolved
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test relies on DataGrid's auto-generated caption for the column with dataField: 'data1_level1'. While DataGrid does auto-generate captions from dataFields (converting underscores to spaces and capitalizing), explicitly setting a caption would make the test more readable and less fragile to potential changes in the caption generation logic. Consider adding an explicit caption property to make the test's intent clearer.
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test uses .toBeDefined() which passes even when find() returns undefined. This assertion will always pass because find() always returns a value (either an object or undefined), and undefined.toBeDefined() is true. Use .toBeTruthy() or check the result with .not.toBeUndefined() for the intended behavior.
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to the previous comment, .toBeUndefined() on a find() result will always be false since find() returns undefined (not an object with .toBeUndefined() method). The correct assertion should be .toBe(undefined) or .toBeFalsy() to properly check that the column was not found.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The method queries document.body directly rather than using this.root as the search context. This could cause issues if multiple column choosers exist in tests. Consider scoping the query to this.root or documenting why global document.body search is necessary for popup wrappers (which are typically rendered in a portal outside the grid container).