Scheduler: T1310524 — shadeUntilCurrentTime property is inconsistently applied to resources outside the viewport in virtual scrolling mode #32125
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug (T1310524) where the shadeUntilCurrentTime property was inconsistently applied to resource groups outside the viewport when virtual scrolling mode is enabled. The issue occurred because the getRoundedCellWidth method returned 0 for groups that weren't rendered in the DOM due to virtual scrolling, causing the shader width calculation to fail.
Key Changes
- Added fallback logic in
getRoundedCellWidthto use group 0's cell width when a requested group has no rendered elements - Tracks the number of found DOM elements to detect when virtualized groups are being queried
- Added comprehensive test coverage for the virtual scrolling scenario with multiple resource groups
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| packages/devextreme/js/__internal/scheduler/workspaces/m_work_space.ts | Implements fallback mechanism in getRoundedCellWidth to return group 0's width when virtualized groups have no rendered DOM elements |
| packages/devextreme/js/__internal/scheduler/tests/workspace.test.ts | Adds new test file with coverage for the virtual scrolling bug scenario, verifying that non-rendered groups return valid cell widths |
| return width / (totalCellCount + cellCount - startIndex); | ||
| const divisor = totalCellCount + cellCount - startIndex; | ||
| let result = width / divisor; | ||
|
|
There was a problem hiding this comment.
The fallback logic for handling virtualized groups that are not rendered in the DOM is not documented. Consider adding a comment explaining that when virtual scrolling is enabled and a group is not rendered, this fallback uses group 0's cell width as a reference, which is needed for proper shader rendering via the shadeUntilCurrentTime feature.
| // When virtual scrolling is enabled, cells for some groups may not be rendered in the DOM. | |
| // In that case, the computed width can be 0 with no elements found for the target group. | |
| // To keep features such as shadeUntilCurrentTime working correctly, fall back to using | |
| // group 0's cell width as a reference for shader rendering. |
| let foundElements = 0; | ||
| for (let i = startIndex; i < totalCellCount + cellCount; i++) { | ||
| const element = $($cells).eq(i).get(0); | ||
| const elementWidth = element ? getBoundingRect(element).width : 0; | ||
| width += elementWidth; | ||
| if (element) { | ||
| foundElements++; | ||
| } |
There was a problem hiding this comment.
The variable name 'foundElements' could be more descriptive to clarify its purpose. Consider renaming it to 'renderedElementCount' to better convey that it tracks the number of DOM elements that were actually found and rendered.
| const CLASSES = { | ||
| scheduler: 'dx-scheduler', | ||
| workSpace: 'dx-scheduler-work-space', | ||
| shaderTop: 'dx-scheduler-date-time-shader-top', | ||
| }; |
There was a problem hiding this comment.
The CLASSES object defines 'workSpace' and 'shaderTop' properties that are not used in this test. Consider removing these unused properties or adding additional test assertions that use them to verify the shader rendering behavior.
| cellCount, | ||
| ); | ||
|
|
||
| expect(lastGroupWidth).toBeGreaterThan(0); |
There was a problem hiding this comment.
The test assertion only verifies that lastGroupWidth is greater than 0, but doesn't verify that it matches group 0's width. Consider adding an assertion that compares the last group's width to the first group's width to ensure the fallback mechanism is working correctly: expect(lastGroupWidth).toBe(workSpace.getRoundedCellWidth(0, 0, cellCount)).
1 participant
No description provided.