-
Notifications
You must be signed in to change notification settings - Fork 664
Scheduler: fix Appointment collector button with wrong info about date #32176
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.
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
base: 26_1
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR aims to fix the Scheduler appointment collector button to display correct date information when timezone conversion is applied. The fix refactors the _getDateText method to use timeZoneCalculator.createDate for timezone conversion and adds a TestCafe test to verify the behavior.
Changes:
- Refactored
_getDateTextmethod to convert dates to grid timezone before formatting - Removed unused
_getStartDateand_getEndDatehelper methods - Added TestCafe test for appointment collector timezone handling
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/devextreme/js/__internal/scheduler/m_compact_appointments_helper.ts | Refactored date text generation to apply timezone conversion; removed unused helper methods |
| e2e/testcafe-devextreme/tests/scheduler/timezones/appointmentCollectorTimezone.ts | Added new TestCafe test to verify collector displays correct date after timezone conversion |
| startDate: new Date('2021-03-05T23:45:00.000Z'), | ||
| endDate: new Date('2021-03-05T18:15:00.000Z'), |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The startDate is after the endDate, which is logically incorrect. The startDate is 23:45 UTC while the endDate is 18:15 UTC on the same day. This means the appointment would start 5 hours and 30 minutes after it ends, which doesn't make sense. The dates should be swapped or corrected so that startDate comes before endDate.
| startDate: new Date('2021-03-05T23:45:00.000Z'), | |
| endDate: new Date('2021-03-05T18:15:00.000Z'), | |
| startDate: new Date('2021-03-05T18:15:00.000Z'), | |
| endDate: new Date('2021-03-05T23:45:00.000Z'), |
| return date ? new Date(date) : null; | ||
| } | ||
| _getDateText(appointment) { | ||
| const { startDate, endDate } = appointment; |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The direct destructuring of startDate and endDate from the appointment object bypasses the data accessor system. The old implementation used this.instance._dataAccessors.get('startDate', appointment) and this.instance._dataAccessors.get('endDate', appointment), which correctly handles custom field name mappings when users configure options like startDateExpr and endDateExpr. Direct property access will break when custom field names are used. The code should use the data accessors instead.
| const { startDate, endDate } = appointment; | |
| const startDate = this.instance._dataAccessors.get('startDate', appointment); | |
| const endDate = this.instance._dataAccessors.get('endDate', appointment); |
6a07538 to
681ebd6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| startDate: new Date('2021-03-05T23:45:00.000Z'), | ||
| endDate: new Date('2021-03-05T18:15:00.000Z'), |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The endDate (2021-03-05T18:15:00.000Z) is earlier than the startDate (2021-03-05T23:45:00.000Z), which creates an invalid appointment with negative duration. The endDate should be after the startDate. For example, it should be '2021-03-06T01:15:00.000Z' to represent a valid appointment.
| // @ts-expect-error | ||
| $button.data('items'), |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The @ts-expect-error directive lacks an explanation. Add a comment describing why the type error is expected, such as the mismatch between jQuery's data() method return type and the expected parameter type.
| // @ts-expect-error | ||
| return this.instance._createComponent($button, Button, { |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The @ts-expect-error directive lacks an explanation. Add a comment describing why the type error is expected, such as potential type incompatibility in the _createComponent method signature.
| const date = appointment.endDate; | ||
| return date ? new Date(date) : null; | ||
| } | ||
| _getDateText(appointment, targetedAppointment?) { |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The targetedAppointment parameter should have an explicit type annotation. Based on the usage of displayStartDate and displayEndDate properties, this should be typed as TargetedAppointment | undefined (from the types.ts file) to provide better type safety and IDE support.
No description provided.