Scheduler: T1310544 — scrollTo does not take offset into account#32073
Scheduler: T1310544 — scrollTo does not take offset into account#32073sjbur wants to merge 10 commits intoDevExpress:26_1from
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where the Scheduler's scrollTo method did not properly account for the viewOffset option, which shifts the displayed time range. The fix ensures that when an offset is applied, hours are not incorrectly clamped to the startDayHour/endDayHour range, and the valid scroll date range is extended appropriately.
Key changes:
- Modified
_getScrollCoordinatesto skip hour clamping whenviewOffsetis non-zero, allowing natural hour values to be used with the offset - Enhanced
_isValidScrollDateto extend the valid date range by theviewOffseton both ends - Consolidated test files by merging
workspace.base.test.tsandworkspace.recalculation.test.tsinto a single organizedworkspace.test.tswith a new test case for the offset scrolling behavior
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
packages/devextreme/js/__internal/scheduler/workspaces/m_work_space.ts |
Fixed _getScrollCoordinates to conditionally apply hour clamping only when viewOffset === 0, and updated _isValidScrollDate to extend the valid date range by the offset value |
packages/devextreme/js/__internal/scheduler/__tests__/workspace.test.ts |
New consolidated test file containing all workspace tests including base functionality, async template recalculation, and a new test for scrollTo with offset (T1310544) |
packages/devextreme/js/__internal/scheduler/__tests__/workspace.base.test.ts |
Deleted - tests moved to workspace.test.ts |
packages/devextreme/js/__internal/scheduler/__tests__/workspace.recalculation.test.ts |
Deleted - tests moved to workspace.test.ts |
aleksei-semikozov
left a comment
aleksei-semikozov
left a comment
There was a problem hiding this comment.
I see two remote test files.
How I see them:
workspace.base.test.ts - unit test.
workspace.recalculation.test.ts - integration test where we create the scheduler
I don't think this test should be in one file
I would rename workspace.recalculation.test.ts -> workspace.test.ts and write your test case in this file
|
@aleksei-semikozov applied changes you asked |
| const extendedMin = new Date(min.getTime() - viewOffset); | ||
| const extendedMax = new Date(max.getTime() + viewOffset); |
There was a problem hiding this comment.
Question: shouldn't min and max dates be offseted in the same direction?
| if (hours < startDayHour) { | ||
| hours = startDayHour; | ||
| } | ||
|
|
||
| if (hours >= endDayHour) { | ||
| hours = endDayHour - 1; | ||
| if (hours >= endDayHour) { | ||
| hours = endDayHour - 1; | ||
| } |
There was a problem hiding this comment.
I notice that we validate scrollTo date here before calculating scrollTo coordinates.
I suspect that this normalization may be redundant now, since we will throw a warning if scrollTo date is out of range. But I am not fully sure.
Can you investigate it please?
There was a problem hiding this comment.
I got it. We need this normalization for views larger than day, e.g. timelineWeek, timelineMonth, etc.
However we should not just ignore this normalization if viewOffset is set. We need to adjust this normalization to include viewOffset into it.
For example this case (I have changed currentView and viewOffset). The scheduler is scrolled to 5 PM, however it should scroll to the last cell of the day (7 PM).
Please adjust the normalization and write a test for it.
No description provided.