Scheduler - Appointments Refactoring - Rendering fixes#33249
Scheduler - Appointments Refactoring - Rendering fixes#33249Tucchhaa merged 8 commits intoDevExpress:26_1from
Conversation
| private appointmentBySortIndex: Record<number, AppointmentComponent> = {}; | ||
| private viewItemBySortedIndex: Record<number, ViewItem> = {}; | ||
|
|
||
| private viewItems: ViewItem[] = []; |
There was a problem hiding this comment.
This variable contains the same value as viewItemBySortedIndex, but as a flat array.
It's not used in the current code, but will be used in the next PR with the focus state.
There was a problem hiding this comment.
Pull request overview
Refactors Scheduler “appointments_new” rendering/diffing to preserve existing appointment view instances when sortedIndex shifts (e.g., insert/remove in the middle), and adds a CSS class marker for appointments that have a resource color applied.
Changes:
- Extend view model diff items to carry
oldSortedIndexfor matched items, enabling stable lookup of existing rendered items after re-sorting. - Refactor
Appointmentsrendering to maintain asortedIndex -> view itemmap and reuse items during partial updates. - Add
APPOINTMENT_TYPE_CLASSES.HAS_RESOURCEand apply it when resource color is present (grid + agenda), with accompanying tests.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/devextreme/js/__internal/scheduler/appointments_new/utils/get_view_model_diff.ts | Adds oldSortedIndex to diff output for matched items. |
| packages/devextreme/js/__internal/scheduler/appointments_new/utils/get_view_model_diff.test.ts | Updates expectations to validate oldSortedIndex behavior. |
| packages/devextreme/js/__internal/scheduler/appointments_new/const.ts | Adds HAS_RESOURCE appointment type class constant. |
| packages/devextreme/js/__internal/scheduler/appointments_new/appointments.ts | Refactors rendering to reuse view instances across sortedIndex shifts; adds view-item accessors. |
| packages/devextreme/js/__internal/scheduler/appointments_new/appointments.test.ts | Adds tests ensuring items are not recreated when sortedIndex shifts due to insert/remove. |
| packages/devextreme/js/__internal/scheduler/appointments_new/appointment/base_appointment.ts | Changes resize signature to accept optional geometry param. |
| packages/devextreme/js/__internal/scheduler/appointments_new/appointment/grid_appointment.ts | Updates resize to accept optional geometry; adds HAS_RESOURCE class when color applied. |
| packages/devextreme/js/__internal/scheduler/appointments_new/appointment/grid_appointment.test.ts | Adjusts geometry test to call resize(geometry); adds HAS_RESOURCE class test. |
| packages/devextreme/js/__internal/scheduler/appointments_new/appointment/agenda_appointment.ts | Updates resize to accept optional geometry; adds HAS_RESOURCE class when color applied. |
| packages/devextreme/js/__internal/scheduler/appointments_new/appointment/agenda_appointment.test.ts | Adds geometry tests for init + resize(geometry); adds HAS_RESOURCE class test. |
packages/devextreme/js/__internal/scheduler/appointments_new/appointment/grid_appointment.ts
Show resolved
Hide resolved
packages/devextreme/js/__internal/scheduler/appointments_new/appointment/agenda_appointment.ts
Outdated
Show resolved
Hide resolved
packages/devextreme/js/__internal/scheduler/appointments_new/appointments.ts
Show resolved
Hide resolved
packages/devextreme/js/__internal/scheduler/appointments_new/appointments.ts
Show resolved
Hide resolved
packages/devextreme/js/__internal/scheduler/appointments_new/appointment/base_appointment.ts
Show resolved
Hide resolved
| // TODO: remove passing index to appointmentTemplate, need only to avoid BC | ||
| viewModelDiff.forEach((diffItem, index) => { | ||
| const { allDay, sortedIndex } = diffItem.item; | ||
| const lookupIndex = diffItem.oldSortedIndex ?? sortedIndex; |
There was a problem hiding this comment.
Without lookup index, this case wouldn't work:
const InitialViewModel = [
{ data: 'A', sortedIndex: 0 },
{ data: 'B', sortedIndex: 1 }
]
renderViewModelDiff(InitialViewModel) ->
viewItemBySortedIndex == {
0: 'A'
1: 'B'
}
const nextViewModel = [
{ data: 'A', sortedIndex: 0 },
{ data: 'New', sortedIndex: 1 },
{ data: 'B', sortedIndex: 2 }
]
renderViewModelDiff(InitialViewModel) ->
viewItemBySortedIndex == {
0: 'A'
1: 'New',
2: undefined
}That's because get_view_model_diff doesn't compare sortedIndex between items. So since the 'B' item didn't have any changes, it wouldn't be rerendered, but the index to access the element from this.viewItemBySortedIndex will be 2, which is not defined.
| }); | ||
|
|
||
| describe('Geometry', () => { | ||
| it('should apply geometry on init', async () => { |
There was a problem hiding this comment.
In agenda view on viewModel change all view items are rerendered, so there's no test for .resize() function, as agenda appointment is resized only at the creation
No description provided.