Scheduler: refactor appointment_popup module (TS)

[bit-byte0]

Scheduler: refactor appointment_popup module (TS) #4124

What does the PR change?

• Removed m_ prefix from files in appointment_popup/ folder.
• Also added types and replaced old Deferred with native Promise.

How did you achieve this?

Removing from files _m prefix turned strict mode on, and strict mode complained about missing types and the Deferred usage (it needed @ts-expect-error to even compile). Fixed all that: added interfaces, switched to async/await + native Promise.

One more thing - some tests broke after switching to Promise because they expected the save to finish synchronously (which Deferred kinda allowed). Native Promise always resolves async, so added await Promise.resolve() after save calls in those tests to let the chain settle before asserting.

How can we verify these changes?

• Run Jest: pnpm run test-jest
• Run QUnit scheduler tests
• Open demo app, try creating/editing/deleting regular and recurring appointments - should work same as before

[Copilot]

Pull request overview

Refactors the internal Scheduler appointment popup module by splitting/renaming legacy m_* modules into clearer TS modules (form, popup), tightening types around scheduler/form/popup interactions, and extracting form-item customization logic into a dedicated utility with Jest coverage.

Changes:

• Renamed/refactored appointment popup modules (m_form → form, m_popup → popup) and updated Scheduler + test mocks imports accordingly.
• Strengthened TypeScript typings across appointment popup/form/recurrence utilities (e.g., DayOfWeek, scheduler “proxy” interfaces).
• Added customize_form_items.ts helper + updated unit tests to use the new module.

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
packages/devextreme/js/__internal/scheduler/m_scheduler.ts	Updates appointment popup imports and includes minor formatting cleanups.
packages/devextreme/js/__internal/scheduler/appointment_popup/utils.ts	Tightens firstDayOfWeek typing to DayOfWeek.
packages/devextreme/js/__internal/scheduler/appointment_popup/recurrence_form.ts	Repoints scheduler typing to the new form scheduler interface and improves typing in component creation.
packages/devextreme/js/__internal/scheduler/appointment_popup/popup.ts	Major refactor: introduces explicit state/types and replaces Deferred/when flow with Promises for saving + popup show/hide.
packages/devextreme/js/__internal/scheduler/appointment_popup/form.ts	Major refactor: introduces AppointmentFormScheduler interface, updates customization pipeline, and improves typing.
packages/devextreme/js/__internal/scheduler/appointment_popup/customize_form_items.ts	New extracted helper for merging default form items with user config.
packages/devextreme/js/__internal/scheduler/appointment_popup/customize_form_items.test.ts	Updates tests to import the new helper module.
packages/devextreme/js/__internal/scheduler/tests/mock/model/scheduler.ts	Updates appointment popup constant import to the renamed module.
packages/devextreme/js/__internal/scheduler/tests/mock/create_appointment_popup.ts	Updates appointment popup/form imports to renamed modules and keeps mocks aligned.

Comments suppressed due to low confidence (2)

packages/devextreme/js/__internal/scheduler/appointment_popup/popup.ts:139

• show()/hide() throw errors.Error('E1033') when popupInstance is not set. E1033 is an existing UI widgets error code for an unrelated case (date navigator step) and expects a parameter, so this will produce a misleading error message here. Prefer not throwing at all (e.g., no-op/return) or throw a dedicated error with a correct code/message; alternatively, capture the instance from createComponent synchronously so popupInstance is always defined before calling show()/hide().
  packages/devextreme/js/__internal/scheduler/appointment_popup/popup.ts:343
• saveChangesAsync() calls hideLoading() only in the success .then() branch. If this.config.onSave() (or the validation promise) rejects/throws, the chain goes to .catch(noop) and the load panel is never hidden, leaving the UI stuck. Use a finally to always call hideLoading() (or call it in both success and error paths) and avoid swallowing the error before the load panel is cleaned up.

[Copilot]

Pull request overview

Copilot reviewed 13 out of 14 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

packages/devextreme/js/__internal/scheduler/appointment_popup/popup.ts:341

• This makes saveChangesAsync defer the save work to a native Promise microtask even when form validation is synchronous. hideAppointmentPopup(true) still calls saveChangesAsync() without awaiting it and then returns immediately, so callers that currently rely on the appointment being saved when hideAppointmentPopup(true) returns will now observe stale data until the microtask runs (the existing integration tests around hideAppointmentPopup(true) assert synchronously). Consider preserving the synchronous path for already-complete validation or updating the caller/API contract to await the returned promise.
  packages/devextreme/js/__internal/scheduler/appointment_popup/popup.ts:138
• E1033 is the date-navigator error ("Unknown step in the date navigator: '{0}'"), so throwing it here would produce a misleading message unrelated to appointment popup creation/hiding. Use an error that matches this failure mode, or keep the previous no-op behavior for hide() when no popup instance exists.

[sjbur]

E1033: 'Unknown step in the date navigator: '{0}'',

I am not sure that we need to display this error here. And not sure if we need to return any errors here or in hide step

[aleksei-semikozov]

https://js.devexpress.com/jQuery/Documentation/ApiReference/UI_Components/Errors_and_Warnings/#E1033

A [Scheduler](https://js.devexpress.com/jQuery/Documentation/ApiReference/UI_Components/dxScheduler/) internal error.

To solve the issue, please submit a ticket to our [Support Center](https://www.devexpress.com/ask). Include your UI component configuration, fake data, and the steps needed to reproduce the issue in the ticket.

[sjbur]

OK, looks like a mistake of mine

[sjbur]

@aleksei-semikozov I think you had a bug related to this string when you were also passing a function instead of option.

[aleksei-semikozov]

In this function, we use the firstDayOfWeek option in this way, but if this otpion is not specified, the localization settings are applied. This is the same way we use it in the header

code link

[sjbur]

Why do we need this in tests?

[bit-byte0]

because Deferred that was used before refactoring were acting synchronously even that it is Promise analogue and all tests also were set up to check synchronous behavior. After refactoring we used Promise that acts asynchronous and tests were not ready for it - they didn't have any option to wait a bit (save button clicks before Promise resolves) and that's why I needed to add async behavior to tests

[sjbur]

OK, thank you

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (5)

packages/devextreme/js/__internal/scheduler/appointment_popup/popup.ts:342

• If validation or onSave rejects, this catch(noop) resolves the promise without running the preceding hideLoading() handler. A failed add/update can therefore leave the scheduler load panel visible indefinitely after the Save button is clicked; use cleanup that runs on both fulfillment and rejection instead of swallowing before hiding.
  packages/devextreme/js/__internal/scheduler/appointment_popup/popup.ts:130
• E1033 is the scheduler date navigator error (“Unknown step in the date navigator”), so using it for a missing popup instance would surface a misleading and incorrectly parameterized error if popup creation fails.

This issue also appears on line 138 of the same file.
packages/devextreme/js/__internal/scheduler/appointment_popup/popup.ts:138

• E1033 refers to an unknown date navigator step, not to popup state. Throwing it here would produce a misleading error message if hide() is called without an initialized popup instance.
  packages/devextreme/js/__internal/scheduler/appointment_popup/form.ts:50
• The scheduler firstDayOfWeek option is optional, and the appointment popup has tests that expect undefined to be passed through to DateBox calendar options when the option is unset. Narrowing this config to DayOfWeek forces callers to replace undefined with a concrete value, changing that behavior.
  packages/devextreme/js/__internal/scheduler/appointment_popup/recurrence_form.ts:24
• firstDayOfWeek can be unset on the scheduler, and recurrence calendar options should preserve that undefined value so the DateBox/Calendar defaulting logic remains unchanged. Typing the recurrence form config as a required DayOfWeek prevents that path and encourages forcing a localized fallback before the calendar sees the option.

[Copilot]

Pull request overview

Copilot reviewed 17 out of 18 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

packages/devextreme/js/__internal/scheduler/appointment_popup/popup.ts:342

• When config.onSave (or the validation promise) rejects, this chain jumps directly to catch(noop), so hideLoading() is never called after showLoadPanel() was used. That can leave the scheduler blocked by a stale load panel on failed saves; the cleanup should run in the rejection path as well (for example via finally) before deciding whether to swallow or propagate the error.