Skip to content

Scheduler - Replace action enum with onDone/onCancel callbacks in AppointmentPopup #33047

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
aleksei-semikozov merged 7 commits into from
Apr 3, 2026
Merged

Scheduler - Replace action enum with onDone/onCancel callbacks in AppointmentPopup #33047

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
77cb409
Scheduler - replace action enum with onDone/onCancel callbacks in App…
aleksei-semikozov Mar 26, 2026
31cd08f
Scheduler - add default config and exclude-from-series test
aleksei-semikozov Mar 26, 2026
9c52ce8
Scheduler - preserve isToolbarVisible in popup config for backward co…
aleksei-semikozov Mar 26, 2026
ab421e7
Scheduler - fix review findings: remove dead isToolbarVisible, fix re…
aleksei-semikozov Apr 2, 2026
ab94f41
Scheduler - fix QUnit test: replace isToolbarVisible assertion with r…
aleksei-semikozov Apr 2, 2026
ce7006b
Merge branch '26_1' into scheduler-popup-callbacks
aleksei-semikozov Apr 2, 2026
c6b9fee
Scheduler - apply review: rename onDone to onSave, inline isReadOnly
aleksei-semikozov Apr 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions packages/devextreme/js/__internal/scheduler/__tests__/__mock__/create_appointment_popup.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import { Deferred } from '@js/core/utils/deferred';
import { mockTimeZoneCalculator } from '../../__mock__/timezone_calculator.mock';
import { AppointmentForm } from '../../appointment_popup/m_form';
import {
ACTION_TO_APPOINTMENT,
APPOINTMENT_POPUP_CLASS,
AppointmentPopup,
} from '../../appointment_popup/m_popup';
Expand Down Expand Up @@ -59,11 +58,13 @@ const resolvedDeferred = (): any => {

interface CreateAppointmentPopupOptions {
appointmentData?: Record<string, unknown>;
action?: number;
editing?: Record<string, unknown>;
firstDayOfWeek?: number;
startDayHour?: number;
onAppointmentFormOpening?: (...args: unknown[]) => void;
onDone?: jest.Mock;
title?: string;
readOnly?: boolean;
Comment thread
aleksei-semikozov marked this conversation as resolved.
addAppointment?: jest.Mock;
updateAppointment?: jest.Mock;
}
Expand All @@ -78,6 +79,7 @@ interface CreateAppointmentPopupResult {
updateAppointment: jest.Mock;
focus: jest.Mock;
updateScrollPosition: jest.Mock;
onDone: jest.Mock;
};
dispose: () => void;
}
Expand Down Expand Up @@ -112,6 +114,7 @@ export const createAppointmentPopup = async (
?? jest.fn(resolvedDeferred);
const focus = jest.fn();
const updateScrollPosition = jest.fn();
const onDone = options.onDone ?? jest.fn(resolvedDeferred);

const formSchedulerProxy = {
getResourceById: (): Record<string, unknown> => resourceManager.resourceById,
Expand Down Expand Up @@ -161,9 +164,10 @@ export const createAppointmentPopup = async (

const appointmentData = options.appointmentData
?? { ...DEFAULT_APPOINTMENT };
const action = options.action ?? ACTION_TO_APPOINTMENT.CREATE;
const title = options.title ?? 'New Appointment';
const readOnly = options.readOnly ?? false;

popup.show(appointmentData, { action, allowSaving: true });
popup.show(appointmentData, { onDone, title, readOnly });
await new Promise(process.nextTick);

const selector = `.dx-overlay-wrapper.${APPOINTMENT_POPUP_CLASS}`;
Expand Down Expand Up @@ -196,6 +200,7 @@ export const createAppointmentPopup = async (
updateAppointment,
focus,
updateScrollPosition,
onDone,
},
dispose,
};
Expand Down
75 changes: 65 additions & 10 deletions packages/devextreme/js/__internal/scheduler/appointment_popup/appointment_popup.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
import {
afterEach, beforeEach, describe, expect, it,
afterEach, beforeEach, describe, expect, it, jest,
} from '@jest/globals';

import fx from '../../../common/core/animation/fx';
import {
createAppointmentPopup,
disposeAppointmentPopups,
} from '../__tests__/__mock__/create_appointment_popup';
import { ACTION_TO_APPOINTMENT } from './m_popup';

describe('Isolated AppointmentPopup environment', () => {
beforeEach(() => {
Expand Down Expand Up @@ -45,20 +44,19 @@ describe('Isolated AppointmentPopup environment', () => {
expect(POM.cancelButton).toBeTruthy();
});

it('should call addAppointment on Save click for CREATE action', async () => {
it('should call onDone callback on Save click', async () => {
const { POM, callbacks } = await createAppointmentPopup({
appointmentData: {
text: 'New Appointment',
startDate: new Date(2021, 3, 26, 9, 30),
endDate: new Date(2021, 3, 26, 11, 0),
},
action: ACTION_TO_APPOINTMENT.CREATE,
});

POM.saveButton.click();

expect(callbacks.addAppointment).toHaveBeenCalledTimes(1);
expect(callbacks.addAppointment).toHaveBeenCalledWith(
expect(callbacks.onDone).toHaveBeenCalledTimes(1);
expect(callbacks.onDone).toHaveBeenCalledWith(
expect.objectContaining({
text: 'New Appointment',
startDate: new Date(2021, 3, 26, 9, 30),
Expand All @@ -67,19 +65,45 @@ describe('Isolated AppointmentPopup environment', () => {
);
});

it('should call updateAppointment on Save click for UPDATE action', async () => {
it('should not call addAppointment or updateAppointment directly', async () => {
const { POM, callbacks } = await createAppointmentPopup({
appointmentData: {
text: 'Existing Appointment',
text: 'Test',
startDate: new Date(2021, 3, 26, 9, 30),
endDate: new Date(2021, 3, 26, 11, 0),
},
action: ACTION_TO_APPOINTMENT.UPDATE,
});

POM.saveButton.click();

expect(callbacks.updateAppointment).toHaveBeenCalledTimes(1);
expect(callbacks.addAppointment).not.toHaveBeenCalled();
expect(callbacks.updateAppointment).not.toHaveBeenCalled();
});

it('should display title from config', async () => {
const { POM } = await createAppointmentPopup({
title: 'Edit Appointment',
});

const titleElement = POM.element.querySelector('.dx-toolbar-label');
expect(titleElement?.textContent).toBe('Edit Appointment');
});

it('should hide Save button when readOnly is true', async () => {
const { POM } = await createAppointmentPopup({
readOnly: true,
});

const saveButtons = POM.element.querySelectorAll('.dx-popup-done');
expect(saveButtons.length).toBe(0);
});

it('should show Save button when readOnly is false', async () => {
const { POM } = await createAppointmentPopup({
readOnly: false,
});

expect(POM.saveButton).toBeTruthy();
});

it('should hide popup on Cancel click', async () => {
Expand All @@ -89,4 +113,35 @@ describe('Isolated AppointmentPopup environment', () => {
POM.cancelButton.click();
expect(popup.visible).toBe(false);
});

it('should support composite onDone for exclude-from-series scenario', async () => {
const updateAppointment = jest.fn();
const addAppointment = jest.fn(() => Promise.resolve());

const sourceAppointment = { text: 'Series', recurrenceRule: 'FREQ=DAILY' };
const updatedAppointment = { text: 'Series', recurrenceException: '20210426' };

const onDone = jest.fn((newAppointment) => {
updateAppointment(sourceAppointment, updatedAppointment);
return addAppointment(newAppointment);
});

const { POM } = await createAppointmentPopup({
appointmentData: {
text: 'Single occurrence',
startDate: new Date(2021, 3, 26, 9, 30),
endDate: new Date(2021, 3, 26, 11, 0),
},
title: 'Edit Appointment',
onDone,
});

POM.saveButton.click();

expect(onDone).toHaveBeenCalledTimes(1);
expect(updateAppointment).toHaveBeenCalledWith(sourceAppointment, updatedAppointment);
expect(addAppointment).toHaveBeenCalledWith(
expect.objectContaining({ text: 'Single occurrence' }),
);
});
});
57 changes: 18 additions & 39 deletions packages/devextreme/js/__internal/scheduler/appointment_popup/m_popup.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@ const POPUP_FULL_SCREEN_MODE_WINDOW_WIDTH_THRESHOLD = 485;

const DAY_IN_MS = dateUtils.dateToMilliseconds('day');

export const ACTION_TO_APPOINTMENT = {
CREATE: 0,
UPDATE: 1,
EXCLUDE_FROM_SERIES: 2,
};
export interface AppointmentPopupConfig {
onDone: (appointment: Record<string, unknown>) => PromiseLike<unknown>;
Comment thread
aleksei-semikozov marked this conversation as resolved.
Outdated
Copy link
Copy Markdown
Contributor

@Tucchhaa Tucchhaa Apr 3, 2026

Choose a reason for hiding this comment

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

small sugestion: let's rename onDone to onSave, so it will be more obvious when this callback is called

Copy link
Copy Markdown
Contributor Author

@aleksei-semikozov aleksei-semikozov Apr 3, 2026

Choose a reason for hiding this comment

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

Done

title: string;
readOnly: boolean;
}
Comment thread
aleksei-semikozov marked this conversation as resolved.

export class AppointmentPopup {
scheduler: any;
Expand All @@ -41,6 +41,12 @@ export class AppointmentPopup {

state: any;

private config: AppointmentPopupConfig = {
onDone: () => Promise.resolve(),
title: '',
readOnly: false,
};

get popup(): dxPopup {
return this._popup as dxPopup;
}
Expand All @@ -54,7 +60,6 @@ export class AppointmentPopup {
this.form = form;

this.state = {
action: null,
lastEditData: null,
saveChangesLocker: false,
appointment: {
Expand All @@ -63,11 +68,9 @@ export class AppointmentPopup {
};
}

show(appointment, config) {
show(appointment, config: AppointmentPopupConfig) {
this.state.appointment.data = appointment;
this.state.action = config.action;
this.state.allowSaving = config.allowSaving;
this.state.excludeInfo = config.excludeInfo;
this.config = config;

this.disposePopup();

Expand Down Expand Up @@ -173,16 +176,8 @@ export class AppointmentPopup {
});
}

private isReadOnly(appointmentAdapter: AppointmentAdapter): boolean {
if (Boolean(appointmentAdapter.source) && appointmentAdapter.disabled) {
return true;
}

if (this.state.action === ACTION_TO_APPOINTMENT.CREATE) {
return false;
}

return !this.scheduler.getEditingConfig().allowUpdating;
private isReadOnly(): boolean {
return this.config.readOnly;
}

private createAppointmentAdapter(rawAppointment): AppointmentAdapter {
Expand All @@ -200,7 +195,7 @@ export class AppointmentPopup {

const formData = this.createFormData(appointmentAdapter);

this.form.readOnly = this.isReadOnly(appointmentAdapter);
this.form.readOnly = this.isReadOnly();
Copy link
Copy Markdown
Contributor

@Tucchhaa Tucchhaa Apr 3, 2026

Choose a reason for hiding this comment

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

minor: let's remove isReadOnly method and directly use the variable:

this.form.readOnly = this.config.readOnly;

Copy link
Copy Markdown
Contributor Author

@aleksei-semikozov aleksei-semikozov Apr 3, 2026

Choose a reason for hiding this comment

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

Done

this.form.formData = formData;

this.form.showMainGroup();
Expand Down Expand Up @@ -286,20 +281,7 @@ export class AppointmentPopup {

const appointment = clonedAdapter.source;

switch (this.state.action) {
case ACTION_TO_APPOINTMENT.CREATE:
this.scheduler.addAppointment(appointment).done(deferred.resolve);
break;
case ACTION_TO_APPOINTMENT.UPDATE:
this.scheduler.updateAppointment(this.state.appointment.data, appointment).done(deferred.resolve);
break;
case ACTION_TO_APPOINTMENT.EXCLUDE_FROM_SERIES:
this.scheduler.updateAppointment(this.state.excludeInfo.sourceAppointment, this.state.excludeInfo.updatedAppointment);
this.scheduler.addAppointment(appointment).done(deferred.resolve);
break;
default:
break;
}
when(this.config.onDone(appointment)).done(deferred.resolve);

deferred.done(() => {
hideLoading();
Expand Down Expand Up @@ -413,13 +395,10 @@ export class AppointmentPopup {
return;
}

const isCreating = this.state.action === ACTION_TO_APPOINTMENT.CREATE;
const formTitleKey = isCreating ? 'dxScheduler-newPopupTitle' : 'dxScheduler-editPopupTitle';

const toolbarItems: ToolbarItem[] = [{
toolbar: 'top',
location: 'before',
text: messageLocalization.format(formTitleKey),
text: this.config.title,
cssClass: 'dx-toolbar-label',
}];

Expand Down
56 changes: 40 additions & 16 deletions packages/devextreme/js/__internal/scheduler/m_scheduler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1623,14 +1623,24 @@ class Scheduler extends SchedulerOptionsBaseWidget {
}

if (isPopupEditing) {
this.appointmentPopup.show(singleRawAppointment, {
isToolbarVisible: true, // TODO: remove when legacyForm is deleted
action: ACTION_TO_APPOINTMENT.EXCLUDE_FROM_SERIES,
excludeInfo: {
sourceAppointment: rawAppointment,
updatedAppointment: appointment.source,
},
});
const popupConfig = this.editing.legacyForm
? {
isToolbarVisible: true,
action: ACTION_TO_APPOINTMENT.EXCLUDE_FROM_SERIES,
excludeInfo: {
sourceAppointment: rawAppointment,
updatedAppointment: appointment.source,
},
}
: {
onDone: (newAppointment) => {
this.updateAppointment(rawAppointment, appointment.source);
return this.addAppointment(newAppointment);
},
title: messageLocalization.format('dxScheduler-editPopupTitle'),
readOnly: Boolean(appointment.source) && appointment.disabled,
};
this.appointmentPopup.show(singleRawAppointment, popupConfig);
this.editAppointmentData = rawAppointment;
} else {
this.updateAppointmentCore(rawAppointment, appointment.source, () => {
Expand Down Expand Up @@ -2005,20 +2015,34 @@ class Scheduler extends SchedulerOptionsBaseWidget {

if (isCreateAppointment) {
delete this.editAppointmentData; // TODO
this.editing.allowAdding && this.appointmentPopup.show(rawAppointment, {
isToolbarVisible: true, // TODO: remove when legacyForm is deleted
action: ACTION_TO_APPOINTMENT.CREATE,
});
if (this.editing.allowAdding) {
const popupConfig = this.editing.legacyForm
? { isToolbarVisible: true, action: ACTION_TO_APPOINTMENT.CREATE }
: {
onDone: (appointment) => this.addAppointment(appointment),
title: messageLocalization.format('dxScheduler-newPopupTitle'),
readOnly: false,
};
this.appointmentPopup.show(rawAppointment, popupConfig);
}
} else {
const startDate = this._dataAccessors.get('startDate', newRawTargetedAppointment || rawAppointment);

this.checkRecurringAppointment(rawAppointment, newTargetedAppointment, startDate, () => {
this.editAppointmentData = rawAppointment; // TODO

this.appointmentPopup.show(rawAppointment, {
isToolbarVisible: this.editing.allowUpdating, // TODO: remove when legacyForm is deleted
action: ACTION_TO_APPOINTMENT.UPDATE,
});
const adapter = new AppointmentAdapter(rawAppointment, this._dataAccessors);
const isDisabled = Boolean(adapter.source) && adapter.disabled;
const readOnly = isDisabled || !this.editing.allowUpdating;

const popupConfig = this.editing.legacyForm
? { isToolbarVisible: this.editing.allowUpdating, action: ACTION_TO_APPOINTMENT.UPDATE }
: {
onDone: (appointment) => this.updateAppointment(rawAppointment, appointment),
title: messageLocalization.format('dxScheduler-editPopupTitle'),
readOnly,
};
this.appointmentPopup.show(rawAppointment, popupConfig);
}, false, true);
}
}
Expand Down
2 changes: 1 addition & 1 deletion ...eme/testing/tests/DevExpress.ui.widgets.scheduler/integration.appointmentTooltip.tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ module('Integration: Appointment tooltip', moduleConfig, () => {
},
'show has a right appointment data arg');

assert.equal(args[1].isToolbarVisible, true, 'show has a right createNewAppointment arg');
assert.equal(args[1].readOnly, false, 'show has a right readOnly arg');

assert.notOk(scheduler.tooltip.isVisible(), 'tooltip was hidden');
});
Expand Down
Loading