Scheduler: T1310544 — scrollTo does not take offset into account #32073

sjbur · 2025-12-30T09:38:05Z

No description provided.

Copilot

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 _getScrollCoordinates to skip hour clamping when viewOffset is non-zero, allowing natural hour values to be used with the offset
Enhanced _isValidScrollDate to extend the valid date range by the viewOffset on both ends
Consolidated test files by merging workspace.base.test.ts and workspace.recalculation.test.ts into a single organized workspace.test.ts with 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

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

sjbur · 2026-01-05T07:45:45Z

@aleksei-semikozov applied changes you asked

Tucchhaa · 2026-01-07T08:56:01Z

packages/devextreme/js/__internal/scheduler/workspaces/m_work_space.ts

+    const extendedMin = new Date(min.getTime() - viewOffset);
+    const extendedMax = new Date(max.getTime() + viewOffset);


Question: shouldn't min and max dates be offseted in the same direction?

Tucchhaa · 2026-01-07T09:02:18Z

packages/devextreme/js/__internal/scheduler/workspaces/m_work_space.ts

+      if (hours < startDayHour) {
+        hours = startDayHour;
+      }

-    if (hours >= endDayHour) {
-      hours = endDayHour - 1;
+      if (hours >= endDayHour) {
+        hours = endDayHour - 1;
+      }


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?

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.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

packages/devextreme/js/__internal/scheduler/__tests__/workspace.test.ts

packages/devextreme/js/__internal/scheduler/workspaces/m_work_space.ts

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

packages/devextreme/js/__internal/scheduler/workspaces/m_work_space.ts

packages/devextreme/js/__internal/scheduler/__tests__/workspace.test.ts

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

Copilot · 2026-01-08T17:57:43Z

packages/devextreme/js/__internal/scheduler/workspaces/m_work_space.ts

+      // If adjustedEndDayHour is 0, it means the range is [adjustedStartDayHour, 24), not crossing midnight
+      // Only consider it crossing midnight if adjustedEndDayHour > 0 and adjustedStartDayHour >= adjustedEndDayHour
+      const crossesMidnight = adjustedEndDayHour > 0 && adjustedStartDayHour >= adjustedEndDayHour;
+
+      // If adjustedEndDayHour is 0, treat it as 24 for range comparison


The comment "If adjustedEndDayHour is 0, it means the range is [adjustedStartDayHour, 24), not crossing midnight" may be misleading. When adjustedEndDayHour equals 0 after modulo 24, it actually means the end hour wraps to midnight (hour 24 becomes hour 0). This is not the same as saying "not crossing midnight" - it still represents a range that extends to exactly midnight. The condition for crossing midnight on line 1431 correctly excludes the case where adjustedEndDayHour is 0, but the comment explanation could be clearer about why this edge case needs special handling.

Suggested change

// If adjustedEndDayHour is 0, it means the range is [adjustedStartDayHour, 24), not crossing midnight

// Only consider it crossing midnight if adjustedEndDayHour > 0 and adjustedStartDayHour >= adjustedEndDayHour

const crossesMidnight = adjustedEndDayHour > 0 && adjustedStartDayHour >= adjustedEndDayHour;

// If adjustedEndDayHour is 0, treat it as 24 for range comparison

// adjustedStartDayHour/adjustedEndDayHour are modulo 24. When endDayHour + viewOffset reaches 24,

// adjustedEndDayHour becomes 0, which represents an end exactly at midnight (hour 24 → 0).

// We treat the range as "crossing midnight" only when adjustedEndDayHour > 0 and

// adjustedStartDayHour >= adjustedEndDayHour; the wrapped-to-0 case is handled separately below.

const crossesMidnight = adjustedEndDayHour > 0 && adjustedStartDayHour >= adjustedEndDayHour;

// If adjustedEndDayHour is 0 (wrapped midnight), treat it as 24 for range comparison

Copilot · 2026-01-08T17:57:44Z

packages/devextreme/js/__internal/scheduler/workspaces/m_work_space.ts

    const currentDate = date || new Date(this.option('currentDate'));
    const startDayHour = this.option('startDayHour');
    const endDayHour = this.option('endDayHour');
+    const viewOffset = this.option('viewOffset') as number;


Type assertion assumes viewOffset is always a number, but if the option could be undefined or null in some edge cases, this could cause runtime errors. Consider adding a null check or using a fallback value like: const viewOffset = (this.option('viewOffset') as number) ?? 0;

Suggested change

const viewOffset = this.option('viewOffset') as number;

const rawViewOffset = this.option('viewOffset');

const viewOffset = typeof rawViewOffset === 'number' ? rawViewOffset : 0;

Copilot · 2026-01-08T17:57:44Z

packages/devextreme/js/__internal/scheduler/workspaces/m_work_space.ts

  }

  _isValidScrollDate(date, throwWarning = true) {
+    const viewOffset = this.option('viewOffset') as number;


Type assertion assumes viewOffset is always a number, but if the option could be undefined or null in some edge cases, this could cause runtime errors. Consider adding a null check or using a fallback value like: const viewOffset = (this.option('viewOffset') as number) ?? 0;

Suggested change

const viewOffset = this.option('viewOffset') as number;

const viewOffset = (this.option('viewOffset') as number | undefined | null) ?? 0;

fix: fix bug

7f828cf

sjbur self-assigned this Dec 30, 2025

sjbur added the 26_1 label Dec 30, 2025

sjbur marked this pull request as ready for review December 31, 2025 07:49

sjbur requested a review from a team as a code owner December 31, 2025 07:49

Copilot AI review requested due to automatic review settings December 31, 2025 07:49

Copilot started reviewing on behalf of sjbur December 31, 2025 07:49 View session

Copilot AI reviewed Dec 31, 2025

View reviewed changes

aleksei-semikozov reviewed Dec 31, 2025

View reviewed changes

test: reerrange tests

37a82c0

sjbur requested review from a team and aleksei-semikozov January 5, 2026 07:44

aleksei-semikozov previously approved these changes Jan 5, 2026

View reviewed changes

Tucchhaa requested changes Jan 7, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings January 8, 2026 13:52

sjbur dismissed aleksei-semikozov’s stale review via c202731 January 8, 2026 13:52

Copilot started reviewing on behalf of sjbur January 8, 2026 13:52 View session

Copilot AI reviewed Jan 8, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings January 8, 2026 16:19

sjbur force-pushed the issue-2570_26_1 branch from ab7164e to 37a82c0 Compare January 8, 2026 16:19

Copilot started reviewing on behalf of sjbur January 8, 2026 16:20 View session

Copilot AI reviewed Jan 8, 2026

View reviewed changes

sjbur added 2 commits January 8, 2026 18:21

feat: try to fix bug with new strategy

d67c7df

fix: move comment

44445c4

Copilot AI review requested due to automatic review settings January 8, 2026 17:47

Copilot started reviewing on behalf of sjbur January 8, 2026 17:48 View session

Copilot AI reviewed Jan 8, 2026

View reviewed changes

		const extendedMin = new Date(min.getTime() - viewOffset);
		const extendedMax = new Date(max.getTime() + viewOffset);

	const viewOffset = this.option('viewOffset') as number;
	const rawViewOffset = this.option('viewOffset');
	const viewOffset = typeof rawViewOffset === 'number' ? rawViewOffset : 0;

	const viewOffset = this.option('viewOffset') as number;
	const viewOffset = (this.option('viewOffset') as number \| undefined \| null) ?? 0;

Scheduler: T1310544 — scrollTo does not take offset into account #32073

Are you sure you want to change the base?

Scheduler: T1310544 — scrollTo does not take offset into account #32073

Conversation

sjbur commented Dec 30, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

aleksei-semikozov left a comment

Choose a reason for hiding this comment

Uh oh!

sjbur commented Jan 5, 2026

Uh oh!

Tucchhaa Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Tucchhaa Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Tucchhaa Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Tucchhaa Jan 7, 2026 •

edited

Loading

Tucchhaa Jan 7, 2026 •

edited

Loading

Tucchhaa Jan 7, 2026 •

edited

Loading