DataGrid - Pager - Show correct pageIndex when virtualScrolling is enabled on pageSize change#33412
DataGrid - Pager - Show correct pageIndex when virtualScrolling is enabled on pageSize change#33412Tucchhaa wants to merge 8 commits intoDevExpress:26_1from
Conversation
Tucchhaa
force-pushed
the
fix_grid_pager_26_1
branch
from
0b6a17d to
664a129
Compare
There was a problem hiding this comment.
Pull request overview
Fixes DataGrid pager’s displayed page index when switching the page size to “all” (internally pageSize = 0) while virtual scrolling is enabled, ensuring the pager info shows a valid “Page 1 of 1 …” state.
Changes:
- Adjust pager page index calculation to handle
pageSize === 0(the “all items” mode). - Add a TestCafe e2e regression test covering the virtual-scrolling + pageSize “all” scenario.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/devextreme/js/__internal/grids/grid_core/pager/m_pager.ts | Ensures pager’s 1-based pageIndex is correct when pageSize is set to 0 (“all”). |
| e2e/testcafe-devextreme/tests/dataGrid/common/pager.ts | Adds regression test verifying pager info becomes “Page 1 of 1 …” after switching to “all” with virtual scrolling. |
| export const MAX_PAGES_COUNT = 10; | ||
|
|
||
| const getPageIndex = function (dataController) { | ||
| if (dataController.pageSize() === 0) { |
There was a problem hiding this comment.
This lines may confuse later, I think it should be fixed on pageIndex level instead
like changePaging in data controller resets pageIndex to 0 in case of pageSize = 0, same should be here, also you may notice that after changing to pageSize = All grid shows 1st page (rows from 1), not keeps the prev one
Tucchhaa
force-pushed
the
fix_grid_pager_26_1
branch
from
f4a744f to
09c0b62
Compare
| @@ -671,7 +671,7 @@ QUnit.module('Real DataController and ColumnsController', { | |||
| this.cellValue(0, 2, '5'); | |||
| this.saveEditData(); | |||
| // assert | |||
| assert.equal(focusedRowChangedFiresCount, 2, 'onFocusedRowChanged fires count'); | |||
There was a problem hiding this comment.
Name of the test suggest that focusedRowChangedFiresCount should increase after refresh, but current test doesn't check it.
Initial test did it, though: link
Tucchhaa
force-pushed
the
fix_grid_pager_26_1
branch
from
c3d5804 to
eb5a17d
Compare
| if (optionName === 'pageSize' && value === 0) { | ||
| that.option('paging.pageIndex', 0); | ||
| } |
There was a problem hiding this comment.
the main fix, synchronizes pageIndex between dataSource and dataController when pageSize is changed to 'all' .
| @@ -1004,18 +1004,19 @@ export const data = (Base: ModuleType<DataController>) => class VirtualScrolling | |||
| }; | |||
| } | |||
|
|
|||
| private _updateVisiblePageIndex(currentPageIndex?) { | |||
| private _updateVisiblePageIndex(value?: number): void { | |||
There was a problem hiding this comment.
just code-style fixes
| this._currentOperationTypes = this._dataSource.operationTypes(); | ||
| this._handleDataChanged(e); | ||
| this._currentOperationTypes = null; | ||
| }; |
There was a problem hiding this comment.
_currentOperationTypes was working incorrectly. It could be set to null before its value is applied to the next change, because of:
- There could be several changes that were merged into one. (In this case _fireChanged wouldn't be called because of this if-statement)
- Race condition here
P.S. this is related to the bug only indirectly. I noticed it while looking for the fix and fixed the bug in several cases
| @@ -1231,36 +1233,32 @@ export class DataController extends DataHelperMixin(modules.Controller) { | |||
| } | |||
| } | |||
|
|
|||
| public updateItems(change?, isDataChanged?) { | |||
There was a problem hiding this comment.
code-style changes only
No description provided.