Scheduler — Popup keep resource state between openings#32915
Scheduler — Popup keep resource state between openings#32915aleksei-semikozov wants to merge 4 commits into26_1from
Conversation
aleksei-semikozov
force-pushed
the
scheduler-popup-recreate-on-show
branch
from
c9a2bb6 to
2245863
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates the internal Scheduler appointment popup lifecycle so that opening the appointment popup starts from a fresh popup/form instance, preventing previously-entered (but not saved) form/resource state from leaking into subsequent openings. It also aligns the “Resolve Time Conflicts” demos by removing manual resource-reset logic on popup close.
Changes:
- Recreate (dispose + re-instantiate) the appointment popup on every
show()call to reset internal form state. - Add a test intended to validate the new “fresh form per opening” behavior; remove two existing regression tests around all-day cancel flows.
- Remove demo code that manually clears the
assigneeIdfield when the editing popup hides.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/devextreme/js/__internal/scheduler/appointment_popup/m_popup.ts | Disposes and recreates the popup (and form) on each opening. |
| packages/devextreme/js/__internal/scheduler/appointment_popup/appointment_popup.test.ts | Adds a new “new form per opening” test and removes two all-day cancel regression tests. |
| apps/demos/Demos/Scheduler/ResolveTimeConflicts/jQuery/index.js | Removes manual assigneeId clearing on popup hidden. |
| apps/demos/Demos/Scheduler/ResolveTimeConflicts/Vue/App.vue | Removes manual assigneeId clearing on popup hidden. |
| apps/demos/Demos/Scheduler/ResolveTimeConflicts/ReactJs/App.js | Removes manual assigneeId clearing on popup hidden. |
| apps/demos/Demos/Scheduler/ResolveTimeConflicts/React/App.tsx | Removes manual assigneeId clearing on popup hidden. |
| apps/demos/Demos/Scheduler/ResolveTimeConflicts/Angular/app/app.component.ts | Removes manual assigneeId clearing on popup hidden. |
You can also share your feedback on Copilot code review. Take the survey.
packages/devextreme/js/__internal/scheduler/appointment_popup/m_popup.ts
Show resolved
Hide resolved
packages/devextreme/js/__internal/scheduler/appointment_popup/appointment_popup.test.ts
Show resolved
Hide resolved
packages/devextreme/js/__internal/scheduler/appointment_popup/appointment_popup.test.ts
Show resolved
Hide resolved
| expect(POM.popup.getInputValue('startTimeEditor')).toBe('9:30 AM'); | ||
| expect(POM.popup.getInputValue('endDateEditor')).toBe('5/9/2017'); | ||
| expect(POM.popup.getInputValue('endTimeEditor')).toBe('11:00 AM'); | ||
| }); |
There was a problem hiding this comment.
I think there are more state tests that can be removed. E.g.:
Can you pls check what kind of tests are not needed anymore? Maybe also testcafe tests
There was a problem hiding this comment.
Yes remove more 4 state tests jest test.
Checked testcafe tests for state isolation patterns (close popup → reopen popup). The key indicator is tests where the popup is opened twice within a single test case.
New form (appointmentForm/): No tests open the popup twice.
Legacy form (legacyAppointmentForm/): Found 3 state isolation tests(but wont fixed this unused code)
No description provided.