Skip to content

Scheduler - Simplify Form scheduler proxy to config-based initialization#33422

Open
aleksei-semikozov wants to merge 4 commits intoDevExpress:26_1from
aleksei-semikozov:scheduler-form-config-3759
Open

Scheduler - Simplify Form scheduler proxy to config-based initialization#33422
aleksei-semikozov wants to merge 4 commits intoDevExpress:26_1from
aleksei-semikozov:scheduler-form-config-3759

Conversation

@aleksei-semikozov
Copy link
Copy Markdown
Contributor

No description provided.

@aleksei-semikozov aleksei-semikozov requested a review from a team as a code owner April 27, 2026 13:07
Copilot AI review requested due to automatic review settings April 27, 2026 13:07
@aleksei-semikozov aleksei-semikozov self-assigned this Apr 27, 2026
@aleksei-semikozov aleksei-semikozov changed the title Scheduler - Replace AppointmentForm scheduler proxy with plain config Scheduler - Simplify Form scheduler proxy to config-based initialization Apr 27, 2026
@aleksei-semikozov aleksei-semikozov marked this pull request as draft April 27, 2026 13:11
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors Scheduler appointment form integration by replacing the AppointmentForm “scheduler proxy” object with an explicit, typed configuration object.

Changes:

  • Introduced AppointmentFormConfig / RecurrenceFormConfig and switched AppointmentForm/RecurrenceForm to consume plain config instead of a Scheduler proxy.
  • Updated Scheduler option-change handling to recreate the appointment form/popup via createAppointmentPopupForm() when relevant options change.
  • Updated internal Scheduler appointment popup test mocks to construct AppointmentForm using the new config shape.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
packages/devextreme/js/__internal/scheduler/m_scheduler.ts Passes a typed AppointmentFormConfig into AppointmentForm and recreates popup/form on key option changes.
packages/devextreme/js/__internal/scheduler/appointment_popup/m_recurrence_form.ts Replaces Scheduler dependency with a minimal RecurrenceFormConfig for first-day-of-week and component creation.
packages/devextreme/js/__internal/scheduler/appointment_popup/m_form.ts Replaces Scheduler proxy usage with AppointmentFormConfig and threads config through recurrence form creation.
packages/devextreme/js/__internal/scheduler/tests/mock/create_appointment_popup.ts Updates mocks to build AppointmentForm using the new config object.

Comment thread packages/devextreme/js/__internal/scheduler/appointment_popup/m_form.ts Outdated
@aleksei-semikozov aleksei-semikozov marked this pull request as ready for review April 27, 2026 14:34
Copilot AI review requested due to automatic review settings April 27, 2026 14:34
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

dataAccessors: this._dataAccessors,
editing: this.editing,
resourceManager: this.resourceManager,
firstDayOfWeek: this.option('firstDayOfWeek') ?? 0,
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

firstDayOfWeek is being defaulted to 0 when the option is not specified. This bypasses the scheduler’s locale-aware default (Scheduler.getFirstDayOfWeek() uses dateLocalization.firstDayOfWeekIndex() when the option is undefined) and can make the appointment form calendar start on Sunday even when the workspace/header start on a different day. Consider sourcing this value via this.getFirstDayOfWeek() (or the same logic) instead of this.option('firstDayOfWeek') ?? 0 so the popup form stays consistent with the rest of the scheduler.

Suggested change
firstDayOfWeek: this.option('firstDayOfWeek') ?? 0,
firstDayOfWeek: this.getFirstDayOfWeek(),

Copilot uses AI. Check for mistakes.
Comment on lines +43 to +51
export interface AppointmentFormConfig {
dataAccessors: AppointmentDataAccessor;
editing: SchedulerProperties['editing'];
resourceManager: ResourceManager;
firstDayOfWeek: number;
startDayHour: number;
createComponent: (element: dxElementWrapper, Component: any, options: any) => any;
getCalculatedEndDate: (startDate: Date) => Date;
}
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AppointmentFormConfig.createComponent is currently typed as (element: dxElementWrapper, Component: any, options: any) => any, but the scheduler’s _createComponent helper can be called with non-wrapper element types (e.g., selector/HTML strings) and this mismatch is already forcing @ts-expect-error at the call site. Consider widening the element parameter type (or introducing a shared CreateComponent type used across scheduler internals) to avoid TS suppressions and keep this config-based API consistently type-safe.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants