Skip to content

Commit fb65e47

Browse files
bit-byte0bit-byte0
committed
refactor(scheduler): address KBN review feedback for appointments_new
1 parent 32de190 commit fb65e47

3 files changed

Lines changed: 164 additions & 28 deletions

File tree

‎packages/devextreme/js/__internal/scheduler/appointments_new/appointments.focus_controller.ts‎

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,10 @@ import { AppointmentCollector } from './appointment_collector';
1010
import type { Appointments } from './appointments';
1111
import type { ViewItem } from './view_item';
1212

13+
interface AppointmentsFocusControllerHandlers {
14+
onAppointmentEnterKeyDown: (appointmentView: BaseAppointmentView, event: DxEvent) => void;
15+
}
16+
1317
export class AppointmentsFocusController {
1418
private focusableSortedIndex = 0;
1519

@@ -27,7 +31,10 @@ export class AppointmentsFocusController {
2731
return this.appointments.option().tabIndex;
2832
}
2933

30-
constructor(private readonly appointments: Appointments) { }
34+
constructor(
35+
private readonly appointments: Appointments,
36+
private readonly handlers: AppointmentsFocusControllerHandlers,
37+
) { }
3138

3239
public onViewItemClick(viewItem: ViewItem): void {
3340
this.focusViewItem(viewItem);
@@ -119,12 +126,9 @@ export class AppointmentsFocusController {
119126
const { allowDelete, onDeleteKeyPress } = this.appointments.option();
120127
if (!allowDelete) { return; }
121128

122-
const sortedItem = this.sortedAppointments[viewItem.option().sortedIndex];
123-
if (!sortedItem) { return; }
124-
125129
const appointmentViewItem = viewItem as BaseAppointmentView;
126130
onDeleteKeyPress({
127-
appointmentData: sortedItem.itemData,
131+
appointmentData: appointmentViewItem.appointmentData,
128132
targetedAppointmentData: appointmentViewItem.targetedAppointmentData,
129133
});
130134
}
@@ -146,15 +150,16 @@ export class AppointmentsFocusController {
146150
}
147151

148152
private handleEnterKeyDown(viewItem: ViewItem, e: KeyboardKeyDownEvent): void {
149-
const { onItemActivate } = this.appointments.option();
150-
const sortedItem = this.sortedAppointments[viewItem.option().sortedIndex];
151-
if (!sortedItem) { return; }
152153
e.originalEvent.preventDefault();
153-
const appointmentViewItem = viewItem as BaseAppointmentView;
154-
onItemActivate({
155-
data: sortedItem.itemData,
156-
targetedAppointmentData: appointmentViewItem.targetedAppointmentData,
157-
});
154+
155+
if (viewItem instanceof AppointmentCollector) {
156+
return;
157+
}
158+
159+
this.handlers.onAppointmentEnterKeyDown(
160+
viewItem as BaseAppointmentView,
161+
e.originalEvent as DxEvent,
162+
);
158163
}
159164

160165
private focusBySortedItem(sortedItem: SortedEntity): void {

‎packages/devextreme/js/__internal/scheduler/appointments_new/appointments.test.ts‎

Lines changed: 143 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -934,6 +934,48 @@ describe('Appointments', () => {
934934
expect(wasDefaultPrevented).toBe(true);
935935
});
936936

937+
it('should prevent default browser behavior on Enter key', () => {
938+
const viewModel = [
939+
mockGridViewModel({ ...defaultAppointmentData }, { sortedIndex: 0 }),
940+
];
941+
942+
const instance = createAppointments({
943+
...getProperties(),
944+
});
945+
instance.option('viewModel', viewModel);
946+
947+
const viewItem = instance.getViewItemBySortedIndex(0);
948+
(viewItem?.$element().get(0) as HTMLElement).click();
949+
950+
const wasDefaultPrevented = !fireEvent.keyDown(
951+
viewItem?.$element().get(0) as HTMLElement,
952+
{ key: 'Enter' },
953+
);
954+
955+
expect(wasDefaultPrevented).toBe(true);
956+
});
957+
958+
it('should prevent default browser behavior on Space key', () => {
959+
const viewModel = [
960+
mockGridViewModel({ ...defaultAppointmentData }, { sortedIndex: 0 }),
961+
];
962+
963+
const instance = createAppointments({
964+
...getProperties(),
965+
});
966+
instance.option('viewModel', viewModel);
967+
968+
const viewItem = instance.getViewItemBySortedIndex(0);
969+
(viewItem?.$element().get(0) as HTMLElement).click();
970+
971+
const wasDefaultPrevented = !fireEvent.keyDown(
972+
viewItem?.$element().get(0) as HTMLElement,
973+
{ key: ' ' },
974+
);
975+
976+
expect(wasDefaultPrevented).toBe(true);
977+
});
978+
937979
it('should move focus to last appointment on End key', () => {
938980
const viewModel = [
939981
mockGridViewModel({ ...defaultAppointmentData }, { sortedIndex: 0 }),
@@ -1031,50 +1073,137 @@ describe('Appointments', () => {
10311073
expect(onDeleteKeyPress).not.toHaveBeenCalled();
10321074
});
10331075

1034-
it('should call onItemActivate when Enter is pressed', () => {
1035-
const onItemActivate = jest.fn();
1076+
it('should show appointment popup when Enter is pressed', () => {
1077+
const showEditAppointmentPopup = jest.fn();
10361078
const viewModel = [
10371079
mockGridViewModel({ ...defaultAppointmentData }, { sortedIndex: 0 }),
10381080
];
10391081

10401082
const instance = createAppointments({
10411083
...getProperties(),
1042-
onItemActivate,
1043-
getSortedItems: () => viewModel as unknown as SortedEntity[],
1084+
showEditAppointmentPopup,
10441085
});
10451086
instance.option('viewModel', viewModel);
10461087

10471088
const viewItem = instance.getViewItemBySortedIndex(0);
10481089
(viewItem?.$element().get(0) as HTMLElement).click();
10491090
fireEvent.keyDown(viewItem?.$element().get(0) as HTMLElement, { key: 'Enter' });
10501091

1051-
expect(onItemActivate).toHaveBeenCalledTimes(1);
1052-
expect(onItemActivate).toHaveBeenCalledWith(
1053-
expect.objectContaining({ data: defaultAppointmentData }),
1092+
expect(showEditAppointmentPopup).toHaveBeenCalledTimes(1);
1093+
expect(showEditAppointmentPopup).toHaveBeenCalledWith(
1094+
defaultAppointmentData,
1095+
expect.objectContaining({ ...defaultAppointmentData }),
10541096
);
10551097
});
1056-
it('should call onItemActivate when Space is pressed', () => {
1057-
const onItemActivate = jest.fn();
1098+
1099+
it('should call onAppointmentDblClick when Enter is pressed', () => {
1100+
const onAppointmentDblClick = jest.fn();
10581101
const viewModel = [
10591102
mockGridViewModel({ ...defaultAppointmentData }, { sortedIndex: 0 }),
10601103
];
10611104

10621105
const instance = createAppointments({
10631106
...getProperties(),
1064-
onItemActivate,
1065-
getSortedItems: () => viewModel as unknown as SortedEntity[],
1107+
onAppointmentDblClick,
1108+
});
1109+
instance.option('viewModel', viewModel);
1110+
1111+
const viewItem = instance.getViewItemBySortedIndex(0);
1112+
(viewItem?.$element().get(0) as HTMLElement).click();
1113+
fireEvent.keyDown(viewItem?.$element().get(0) as HTMLElement, { key: 'Enter' });
1114+
1115+
expect(onAppointmentDblClick).toHaveBeenCalledTimes(1);
1116+
expect(onAppointmentDblClick).toHaveBeenCalledWith(
1117+
expect.objectContaining({ appointmentData: defaultAppointmentData }),
1118+
);
1119+
});
1120+
1121+
it('should show appointment popup when Space is pressed', () => {
1122+
const showEditAppointmentPopup = jest.fn();
1123+
const viewModel = [
1124+
mockGridViewModel({ ...defaultAppointmentData }, { sortedIndex: 0 }),
1125+
];
1126+
1127+
const instance = createAppointments({
1128+
...getProperties(),
1129+
showEditAppointmentPopup,
10661130
});
10671131
instance.option('viewModel', viewModel);
10681132

10691133
const viewItem = instance.getViewItemBySortedIndex(0);
10701134
(viewItem?.$element().get(0) as HTMLElement).click();
10711135
fireEvent.keyDown(viewItem?.$element().get(0) as HTMLElement, { key: ' ' });
10721136

1073-
expect(onItemActivate).toHaveBeenCalledTimes(1);
1074-
expect(onItemActivate).toHaveBeenCalledWith(
1075-
expect.objectContaining({ data: defaultAppointmentData }),
1137+
expect(showEditAppointmentPopup).toHaveBeenCalledTimes(1);
1138+
expect(showEditAppointmentPopup).toHaveBeenCalledWith(
1139+
defaultAppointmentData,
1140+
expect.objectContaining({ ...defaultAppointmentData }),
10761141
);
10771142
});
1143+
1144+
it('should call onAppointmentDblClick when Space is pressed', () => {
1145+
const onAppointmentDblClick = jest.fn();
1146+
const viewModel = [
1147+
mockGridViewModel({ ...defaultAppointmentData }, { sortedIndex: 0 }),
1148+
];
1149+
1150+
const instance = createAppointments({
1151+
...getProperties(),
1152+
onAppointmentDblClick,
1153+
});
1154+
instance.option('viewModel', viewModel);
1155+
1156+
const viewItem = instance.getViewItemBySortedIndex(0);
1157+
(viewItem?.$element().get(0) as HTMLElement).click();
1158+
fireEvent.keyDown(viewItem?.$element().get(0) as HTMLElement, { key: ' ' });
1159+
1160+
expect(onAppointmentDblClick).toHaveBeenCalledTimes(1);
1161+
expect(onAppointmentDblClick).toHaveBeenCalledWith(
1162+
expect.objectContaining({ appointmentData: defaultAppointmentData }),
1163+
);
1164+
});
1165+
1166+
it('should show tooltip when Enter is pressed on appointment collector', () => {
1167+
const showTooltipForCollector = jest.fn();
1168+
const showEditAppointmentPopup = jest.fn();
1169+
const viewModel = [
1170+
mockAppointmentCollectorViewModel({ ...defaultAppointmentData }, { sortedIndex: 0 }),
1171+
];
1172+
1173+
const instance = createAppointments({
1174+
...getProperties(),
1175+
showTooltipForCollector,
1176+
showEditAppointmentPopup,
1177+
});
1178+
instance.option('viewModel', viewModel);
1179+
1180+
const viewItem = instance.getViewItemBySortedIndex(0);
1181+
fireEvent.keyDown(viewItem?.$element().get(0) as HTMLElement, { key: 'Enter' });
1182+
1183+
expect(showTooltipForCollector).toHaveBeenCalledTimes(1);
1184+
expect(showEditAppointmentPopup).not.toHaveBeenCalled();
1185+
});
1186+
1187+
it('should show tooltip when Space is pressed on appointment collector', () => {
1188+
const showTooltipForCollector = jest.fn();
1189+
const showEditAppointmentPopup = jest.fn();
1190+
const viewModel = [
1191+
mockAppointmentCollectorViewModel({ ...defaultAppointmentData }, { sortedIndex: 0 }),
1192+
];
1193+
1194+
const instance = createAppointments({
1195+
...getProperties(),
1196+
showTooltipForCollector,
1197+
showEditAppointmentPopup,
1198+
});
1199+
instance.option('viewModel', viewModel);
1200+
1201+
const viewItem = instance.getViewItemBySortedIndex(0);
1202+
fireEvent.keyDown(viewItem?.$element().get(0) as HTMLElement, { key: ' ' });
1203+
1204+
expect(showTooltipForCollector).toHaveBeenCalledTimes(1);
1205+
expect(showEditAppointmentPopup).not.toHaveBeenCalled();
1206+
});
10781207
});
10791208
});
10801209

‎packages/devextreme/js/__internal/scheduler/appointments_new/appointments.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,9 @@ export class Appointments extends DOMComponent<Appointments, AppointmentsPropert
8989
override _init(): void {
9090
super._init();
9191

92-
this.focusController = new AppointmentsFocusController(this);
92+
this.focusController = new AppointmentsFocusController(this, {
93+
onAppointmentEnterKeyDown: this.onAppointmentDblClick.bind(this),
94+
});
9395

9496
this._templateManager.addDefaultTemplates({
9597
appointment: new EmptyTemplate(),

0 commit comments

Comments
 (0)