Scheduler: refactor appointment_popup module (TS) by bit-byte0 · Pull Request #33575 · DevExpress/DevExtreme

bit-byte0 · 2026-05-13T15:43:09Z

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

…refix

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.

…ek')

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 · 2026-05-15T09:15:40Z

+    if (this.popupInstance) {
+      this.popupInstance.show().catch(noop);
+    } else {
+      throw errors.Error('E1033');


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

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.

OK, looks like a mistake of mine

sjbur · 2026-05-15T09:27:28Z

      getResourceManager: () => this.resourceManager,

-      getFirstDayOfWeek: () => this.option('firstDayOfWeek'),
+      getFirstDayOfWeek: () => this.getFirstDayOfWeek(),


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

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 · 2026-05-15T09:28:15Z


                scheduler.instance.showAppointmentPopup(newItem, true);
                $('.dx-scheduler-appointment-popup .dx-popup-done').trigger('dxclick');
+                await Promise.resolve();


Why do we need this in tests?

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

OK, thank you

refactor(scheduler): appointment popup and recurrence form, drop m_ p…

021cf05

…refix

bit-byte0 self-assigned this May 13, 2026

Copilot AI review requested due to automatic review settings May 13, 2026 15:43

bit-byte0 requested a review from a team as a code owner May 13, 2026 15:43

bit-byte0 added the 26_1 label May 13, 2026

Copilot started reviewing on behalf of bit-byte0 May 13, 2026 15:44 View session

Copilot AI reviewed May 13, 2026

View reviewed changes

Maksim Zakharov added 2 commits May 13, 2026 20:05

fix(scheduler): set getFirstDayOfWeek instead of option('firstDayOfWe…

17bdcd8

…ek')

test(scheduler): await save before asserting in popup tests

c92bbab

Copilot AI review requested due to automatic review settings May 14, 2026 13:35

Copilot started reviewing on behalf of bit-byte0 May 14, 2026 13:36 View session

Copilot AI reviewed May 14, 2026

View reviewed changes

test(scheduler): yet another await Promise.resolve() after popup save

531f0e1

sjbur approved these changes May 15, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Scheduler: refactor appointment_popup module (TS)#33575

Scheduler: refactor appointment_popup module (TS)#33575
bit-byte0 wants to merge 4 commits into
DevExpress:26_1from
bit-byte0:refactor/type-appointment-popup

bit-byte0 commented May 13, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

sjbur May 15, 2026

Uh oh!

aleksei-semikozov May 15, 2026

Uh oh!

sjbur May 15, 2026

Uh oh!

sjbur May 15, 2026

Uh oh!

aleksei-semikozov May 15, 2026

Uh oh!

sjbur May 15, 2026

Uh oh!

bit-byte0 May 15, 2026

Uh oh!

sjbur May 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

bit-byte0 commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Scheduler: refactor appointment_popup module (TS) #4124

What does the PR change?

How did you achieve this?

How can we verify these changes?

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

bit-byte0 commented May 13, 2026 •

edited

Loading

Scheduler: refactor `appointment_popup` module (TS) #4124