Skip to content

Commit 6883a0e

Browse files
authored
fix(popover, collapsible): DP-185811 cancel transitions on unmount (#1241)
1 parent f285a3e commit 6883a0e

4 files changed

Lines changed: 104 additions & 0 deletions

File tree

packages/dialtone-vue/components/collapsible/collapsible.test.js

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,40 @@ describe('DtCollapsible Tests', () => {
145145
});
146146
});
147147

148+
describe('When component is unmounting', () => {
149+
beforeEach(() => {
150+
wrapper.vm._isUnmounting = true;
151+
});
152+
153+
it('does not emit "opened" when leave transition completes', async () => {
154+
await wrapper.vm.onLeaveTransitionComplete();
155+
expect(wrapper.emitted('opened')).toBeUndefined();
156+
});
157+
158+
it('does not emit "opened" when enter transition completes', async () => {
159+
await wrapper.vm.onEnterTransitionComplete();
160+
expect(wrapper.emitted('opened')).toBeUndefined();
161+
});
162+
163+
describe('in controlled mode (open !== null)', () => {
164+
beforeEach(async () => {
165+
await wrapper.setProps({ open: false });
166+
});
167+
168+
it('does not emit "opened" or "update:open" when leave transition completes', async () => {
169+
await wrapper.vm.onLeaveTransitionComplete();
170+
expect(wrapper.emitted('opened')).toBeUndefined();
171+
expect(wrapper.emitted('update:open')).toBeUndefined();
172+
});
173+
174+
it('does not emit "opened" or "update:open" when enter transition completes', async () => {
175+
await wrapper.vm.onEnterTransitionComplete();
176+
expect(wrapper.emitted('opened')).toBeUndefined();
177+
expect(wrapper.emitted('update:open')).toBeUndefined();
178+
});
179+
});
180+
});
181+
148182
describe('Accessibility Tests', () => {
149183
describe('Content is expanded', () => {
150184
beforeEach(async () => {

packages/dialtone-vue/components/collapsible/collapsible.vue

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,15 +247,23 @@ export default {
247247
this.validateProperAnchor();
248248
},
249249
250+
beforeUnmount () {
251+
this._isUnmounting = true;
252+
// Prevent transition callbacks from calling into dead lifecycle methods
253+
// after this component is torn down (DP-185811).
254+
},
255+
250256
methods: {
251257
onLeaveTransitionComplete () {
258+
if (this._isUnmounting) return;
252259
this.$emit('opened', false);
253260
if (this.open !== null) {
254261
this.$emit('update:open', false);
255262
}
256263
},
257264
258265
onEnterTransitionComplete () {
266+
if (this._isUnmounting) return;
259267
this.$emit('opened', true, this.$refs.content);
260268
if (this.open !== null) {
261269
this.$emit('update:open', true);

packages/dialtone-vue/components/popover/popover.test.js

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,56 @@ describe('DtPopover Tests', () => {
305305
});
306306
});
307307

308+
describe('When component is unmounting', () => {
309+
beforeEach(() => {
310+
wrapper.vm._isUnmounting = true;
311+
});
312+
313+
it('sets transition: none on content element to cancel in-flight transitions', () => {
314+
const contentEl = wrapper.vm.popoverContentEl;
315+
wrapper.unmount();
316+
expect(contentEl.style.transition).toBe('none');
317+
});
318+
319+
it('does not emit "opened" when leave transition completes', async () => {
320+
await wrapper.vm.onLeaveTransitionComplete();
321+
expect(wrapper.emitted('opened')).toBeUndefined();
322+
});
323+
324+
it('emits "opened" and respects open prop when leave transition completes and not unmounting', async () => {
325+
wrapper.vm._isUnmounting = false;
326+
await wrapper.vm.onLeaveTransitionComplete();
327+
expect(wrapper.emitted('opened')).toBeDefined();
328+
expect(wrapper.emitted('opened')[0]).toEqual([false]);
329+
// uncontrolled (open === null): update:open is not emitted
330+
expect(wrapper.emitted('update:open')).toBeUndefined();
331+
// controlled (open !== null): update:open is emitted
332+
await wrapper.setProps({ open: false });
333+
await wrapper.vm.onLeaveTransitionComplete();
334+
expect(wrapper.emitted('update:open')).toBeDefined();
335+
expect(wrapper.emitted('update:open')[0]).toEqual([false]);
336+
});
337+
338+
it('does not emit "opened" when enter transition completes', async () => {
339+
await wrapper.vm.onEnterTransitionComplete();
340+
expect(wrapper.emitted('opened')).toBeUndefined();
341+
});
342+
343+
it('emits "opened" and respects open prop when enter transition completes and not unmounting', async () => {
344+
wrapper.vm._isUnmounting = false;
345+
await wrapper.vm.onEnterTransitionComplete();
346+
expect(wrapper.emitted('opened')).toBeDefined();
347+
expect(wrapper.emitted('opened')[0][0]).toBe(true);
348+
// uncontrolled (open === null): update:open is not emitted
349+
expect(wrapper.emitted('update:open')).toBeUndefined();
350+
// controlled (open !== null): update:open is emitted
351+
await wrapper.setProps({ open: false });
352+
await wrapper.vm.onEnterTransitionComplete();
353+
expect(wrapper.emitted('update:open')).toBeDefined();
354+
expect(wrapper.emitted('update:open')[0]).toEqual([true]);
355+
});
356+
});
357+
308358
describe('When anchor slot content changes', () => {
309359
it('should attach the tippy instance to the new DOM node', async () => {
310360
const component = {

packages/dialtone-vue/components/popover/popover.vue

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -712,6 +712,12 @@ export default {
712712
},
713713
714714
beforeUnmount () {
715+
this._isUnmounting = true;
716+
// Cancel any in-progress CSS transitions so transitionend cannot fire
717+
// after this component is torn down and call into dead lifecycle methods.
718+
if (this.popoverContentEl) {
719+
this.popoverContentEl.style.transition = 'none';
720+
}
715721
this.tip?.destroy();
716722
this.intersectionObserver?.disconnect();
717723
this.mutationObserver?.disconnect();
@@ -904,12 +910,16 @@ export default {
904910
},
905911
906912
async onLeaveTransitionComplete () {
913+
if (this._isUnmounting) return;
907914
if (this.modal) {
908915
await this.focusFirstElement(this.$refs.anchor);
916+
if (this._isUnmounting) return;
909917
// await next tick in case the user wants to change focus themselves.
910918
await this.$nextTick();
919+
if (this._isUnmounting) return;
911920
this.enableScrolling();
912921
}
922+
if (this._isUnmounting) return;
913923
this.tip?.unmount();
914924
this.$emit('opened', false);
915925
if (this.open !== null) {
@@ -918,9 +928,11 @@ export default {
918928
},
919929
920930
async onEnterTransitionComplete () {
931+
if (this._isUnmounting) return;
921932
this.focusInitialElement();
922933
// await next tick in case the user wants to change focus themselves.
923934
await this.$nextTick();
935+
if (this._isUnmounting) return;
924936
this.preventScrolling();
925937
this.$emit('opened', true, this.$refs.popover__content);
926938
if (this.open !== null) {

0 commit comments

Comments
 (0)