Repository navigation
fix(date-picker): avoid double overlay detach when destroyed while open - #17958
ivanvpetrov wants to merge 4 commits into
Conversation
ivanvpetrov
added
❌ status: awaiting-test
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues were identified for the change preventing duplicate overlay detachment when open date pickers are destroyed.
No open issues 🎉
What changed in this PR
Fixes #17938 by preventing duplicate calendar overlay detachment when a date picker or date range picker is destroyed while open, including during grid date-cell editing.
Changes:
- Guard both pickers’ destruction-time detach calls with
getOverlayByIdand always clear_overlayId. - Add regression tests for destruction while open in dropdown and dialog modes.
- Extend the date picker’s overlay mock with
getOverlayById.
| File | Description |
|---|---|
projects/igniteui-angular/date-picker/src/date-range-picker/date-range-picker.component.ts |
Guard overlay detachment and clear the stored overlay ID. |
projects/igniteui-angular/date-picker/src/date-range-picker/date-range-picker.component.spec.ts |
Test destruction while open in both interaction modes. |
projects/igniteui-angular/date-picker/src/date-picker/date-picker.component.ts |
Guard overlay detachment and unconditionally clear the overlay ID. |
projects/igniteui-angular/date-picker/src/date-picker/date-picker.component.spec.ts |
Add destruction regression tests and update the overlay mock. |
Review effort: Balanced
viktorkombov
added
💥 status: in-test
|
Please add a CHANGELOG entry under |
viktorkombov
added
✅ status: verified
I've added a CHANGELOG entry. I'll also address the advanced filtering dialog in a separate PRs |
Closes #17938
Description
Destroying an
igx-date-pickerorigx-date-range-pickerwhile its calendar is open no longer logsigxOverlay.detach was called with wrong id.ngOnDestroy, both pickers now detach the calendar only if the overlay is still registered (getOverlayById). The grid already guards its overlays the same way.ngOnDestroynow always resets_overlayId. The date range picker never cleared it before.Motivation / Context
The calendar container is created in the picker's own
ViewContainerRef. When the picker is destroyed, Angular destroys that container before it runs the picker'sngOnDestroy. The overlay service registers a destroy callback on components it attaches, so it detaches the calendar overlay at that point. It doesn't emitclosed, so the picker still holds the old overlay id. The picker'sngOnDestroythen callsdetachwith that id, which no longer exists, and the service logs the warning.In the grid this happens every time a date cell edit ends while the calendar is open (Alt + Arrow Down, then Enter or Escape), because ending the edit removes the editor's picker.
The overlay service's own detach-on-destroy behaviour is correct and stays as is.
Type of Change (check all that apply):
Component(s) / Area(s) Affected:
Date Picker, Date Range Picker (and the grid's date cell editor, which uses the date picker)
How Has This Been Tested?
New
Destroyspecs for both pickers render the picker inside an@if. For each of dropdown and dialog mode, they open the calendar, remove the picker, and assert that the calendar container is gone and that the detach warning wasn't logged. The new specs fail without the fix and pass with it.The date picker unit tests mock the overlay service, so the mock now also has
getOverlayById.Runs:
ng test igniteui-angular --no-watch --include 'projects/igniteui-angular/date-picker/**/*.spec.ts': 214 passedgrid-cell-editing.spec.tsandgrid-row-editing.spec.ts: 62 passednpm run lint:lib: no new errors or warningsTest Configuration:
Screenshots / Recordings
N/A: the only visible effect was a console warning.
Checklist:
feature/README.MDupdates for the feature docsREADME.MDCHANGELOG.MDupdates for newly added functionalityng updatemigrations for the breaking changes (migrations guidelines)