Scheduler: add new scroll api method

[aleksei-semikozov]

No description provided.

[Copilot]

Pull request overview

This pull request adds a new API method overload to the Scheduler component's scrollTo method, allowing users to specify scroll alignment ('start' or 'center') when scrolling to a specific date.

Changes:

• Adds new scrollTo method signature that accepts an options object with group, allDay, and align properties
• Implements alignment logic in the workspace scroll calculations (defaults to 'center' for backward compatibility)
• Adds comprehensive Jest tests for the workspace-level scrollTo functionality

Reviewed changes

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

Show a summary per file

File	Description
packages/devextreme/ts/dx.all.d.ts	Adds new scrollTo method overload with options object parameter for dx.all bundle
packages/devextreme/js/ui/scheduler.d.ts	Adds new scrollTo method overload with JSDoc annotations and references new SchedulerScrollToAlign enum
packages/devextreme/js/__internal/scheduler/workspaces/m_work_space.ts	Implements align parameter in workspace scrollTo method to calculate scroll offsets based on alignment
packages/devextreme/js/__internal/scheduler/m_scheduler.ts	Adds type guard to distinguish between positional and options-object parameters, forwards align to workspace
packages/devextreme/js/__internal/scheduler/tests/workspace.base.test.ts	Adds comprehensive tests for workspace scrollTo with different alignment options

[Copilot]

The type guard _isScrollOptionsObject has a potential issue. Since GroupValues and RawGroupValues are both Record<string, ...> types, they could legitimately have properties named 'align', 'allDay', or 'group' as resource field names. This could cause the function to incorrectly identify a group values object as a ScrollToOptions object.

Consider adding a more robust check, such as checking if the value has exactly the expected properties and/or checking the types of those properties. For example, you could check if align exists and is a string with value 'start' or 'center'.

Suggested change

	return Boolean(options) && typeof options === 'object'
	&& ('align' in options || 'allDay' in options || 'group' in options);
	if (!options || typeof options !== 'object') {
	return false;
	}
	const { align, allDay, group } = options as {
	align?: unknown;
	allDay?: unknown;
	group?: unknown;
	};
	if (align === 'start' || align === 'center') {
	return true;
	}
	if (typeof allDay === 'boolean') {
	return true;
	}
	if (group !== null && typeof group === 'object') {
	return true;
	}
	return false;

[Copilot]

There are no tests for the new Scheduler-level scrollTo method signature that accepts an options object as the second parameter. The tests in workspace.base.test.ts only cover the workspace-level scrollTo method. Consider adding tests that verify the Scheduler class properly handles calls like:

• scheduler.scrollTo(date, { align: 'start' })
• scheduler.scrollTo(date, { align: 'center', allDay: true })
• scheduler.scrollTo(date, { group: groupValues, align: 'start' })

This would ensure the type guard in _isScrollOptionsObject works correctly and the align parameter is properly passed through to the workspace.

[Tucchhaa]

I have a question. Will we deprecate this old scrollTo API in the future? Should show a warning if the user uses old API?

[Tucchhaa]

In the neighbor PR, we also implemented tests for scrollTo, but in testcafe. Instead of checking magic numbers for scroll we check the cell data in scrollable's viewport to see if the scroll worked correctly.

I think that maybe in this test we could use the same approach (I mean to check the cell data instead of scroll position, because it appears more reliable).

What do you think?

[aleksei-semikozov]

I think for unit tests and e2e tests we need different strategies.

E2E vs Unit Tests

For e2e tests (like in your testcafe PR #32160) - yes, checking cell data is better approach! We test like user, high-level:

scheduler.scrollTo(date);
expect(getSchedulerCenterViewport(scheduler)).toEqual(date) // Good for e2e

But for unit tests - I think direct checks is better. Let me explain why.

Problem with Test Helpers in Unit Tests

When test fails with helper function like getWorkspaceCenterViewport(), it's not clear what broken:

• Is problem in workspace code?
• Or problem in helper logic?
• What exactly changed?

Helper function adds one more abstraction layer. In unit test we already testing small piece, so we don't need extra abstraction. It makes harder to understand what really happens.

Also helpers can grow complex over time and check more things than we actually need for this specific test.

Why "Magic Numbers" Can Be OK

About magic numbers - I can make them more clear with calculation:

const cellWidth = 60;
const cellCount = 6; 
const padding = 18;

expect(scrollLeft).toEqual(cellWidth * cellCount - padding * 2); // = 324

This way it's not really "magic" - you see what we checking and why this number.

When test fails, I immediately see: "oh, scroll is 334 instead of 324, so +10px difference" - very quick to understand what broken.

With helper I need dig into helper code to understand what it calculates and why test fails.

Main Point

For unit tests:

• Test breaks fast when something change ✓
• Easy to fix (just update number) ✓
• Clear what exactly broken ✓

For e2e tests:

• Your approach with checking cell data is correct ✓
• More stable when refactoring ✓

So I think both approaches good, just for different test types.

What you think?

[Tucchhaa]

Maybe types.ts file is a more appopriate place for this types?

[sjbur]

I agree that we should move this type somewhere, however, types.ts looks messy to me, but it is better than have this type here