Scheduler - Simplify Form scheduler proxy to config-based initialization#33422
Scheduler - Simplify Form scheduler proxy to config-based initialization#33422aleksei-semikozov wants to merge 5 commits intoDevExpress:26_1from
Conversation
aleksei-semikozov
changed the title
There was a problem hiding this comment.
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/RecurrenceFormConfigand 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. |
| 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; | ||
| } |
There was a problem hiding this comment.
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.
| editing: this.editing, | ||
| resourceManager: this.resourceManager, | ||
| firstDayOfWeek: this.getFirstDayOfWeek(), | ||
| startDayHour: this.option('startDayHour') ?? 0, |
There was a problem hiding this comment.
startDayHour in AppointmentFormConfig is taken from this.option('startDayHour'), while other view-dependent values (e.g. firstDayOfWeek) are resolved via view options. If startDayHour is overridden per-view (views[].startDayHour), the appointment form will use the global value instead of the effective current-view value, which can lead to incorrect start/end times when toggling All Day. Consider using this.getViewOption('startDayHour') (with the same nullish fallback) to match current-view behavior (see scheduler_options_base_widget.ts:getViewOption).
| startDayHour: this.option('startDayHour') ?? 0, | |
| startDayHour: this.getViewOption('startDayHour') ?? 0, |
| const element = $('<div>'); | ||
| const editingConfig = this.scheduler.getEditingConfig(); | ||
| const { | ||
| items: formItems, onContentReady, onInitialized, ...customFormOptions |
There was a problem hiding this comment.
formItems is destructured from this.getEditingForm() but never used. If the intent is just to omit items from customFormOptions, consider renaming it to an intentionally-unused name (e.g. _formItems) or adjusting the destructuring to avoid introducing an unused local (helps keep eslint/TS no-unused-vars clean).
| items: formItems, onContentReady, onInitialized, ...customFormOptions | |
| items: _formItems, onContentReady, onInitialized, ...customFormOptions |
| this.setRemoteFilterIfNeeded(); | ||
|
|
||
| this.postponeDataSourceLoading(); | ||
| this.createAppointmentPopupForm(); |
There was a problem hiding this comment.
In the startDayHour/endDayHour branch, createAppointmentPopupForm() is called for both options. Since the form config snapshots startDayHour but does not directly depend on endDayHour (end-day changes are already reflected via getCalculatedEndDate calling into the workspace), consider calling createAppointmentPopupForm() only when name === 'startDayHour' to avoid unnecessary popup/form re-creation on endDayHour changes.
| this.createAppointmentPopupForm(); | |
| if (name === 'startDayHour') { | |
| this.createAppointmentPopupForm(); | |
| } |
No description provided.