Skip to content

Commit fc840db

Browse files
fix: remove toolbar window listeners on unmount (#2783)
* fix(super-editor): remove toolbar window listeners on unmount * fix: setup window listeners on onMounted and onActivated * test: assert Toolbar window listener cleanup * fix(super-editor): unmount toolbar Vue app on SuperToolbar.destroy Without this, the Toolbar component's onBeforeUnmount hook never runs when SuperDoc is destroyed, so the window resize/keydown listeners this PR registers are never removed. Verified in the dev app: dispatching a resize event after destroy still fired the toolbar handler; with the unmount call the handlers are cleanly removed. Also: - drop dead teardownWindowListeners() call inside setupWindowListeners (addEventListener is idempotent for the same type+listener pair) - add afterEach(vi.restoreAllMocks) to Toolbar.test.js so spies don't leak across tests if an assertion throws --------- Co-authored-by: Caio Pizzol <caio@harbourshare.com>
1 parent f229576 commit fc840db

3 files changed

Lines changed: 142 additions & 12 deletions

File tree

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
import { describe, it, expect, vi, afterEach } from 'vitest';
2+
import { mount } from '@vue/test-utils';
3+
import { defineComponent, ref, KeepAlive } from 'vue';
4+
import Toolbar from './Toolbar.vue';
5+
6+
const ToolbarKeepAliveHost = defineComponent({
7+
components: { KeepAlive, Toolbar },
8+
setup() {
9+
const visible = ref(true);
10+
return { visible };
11+
},
12+
template: '<KeepAlive><Toolbar v-if="visible" /></KeepAlive>',
13+
});
14+
15+
function createMockToolbar() {
16+
return {
17+
config: {
18+
toolbarGroups: ['left', 'center', 'right'],
19+
toolbarButtonsExclude: [],
20+
},
21+
getToolbarItemByGroup: () => [],
22+
getToolbarItemByName: () => null,
23+
onToolbarResize: vi.fn(),
24+
emitCommand: vi.fn(),
25+
overflowItems: [],
26+
activeEditor: null,
27+
};
28+
}
29+
30+
describe('Toolbar', () => {
31+
afterEach(() => {
32+
vi.restoreAllMocks();
33+
});
34+
35+
it('removes resize and keydown listeners on unmount (not only on KeepAlive deactivate)', () => {
36+
const mockToolbar = createMockToolbar();
37+
const addSpy = vi.spyOn(window, 'addEventListener');
38+
const removeSpy = vi.spyOn(window, 'removeEventListener');
39+
40+
const wrapper = mount(Toolbar, {
41+
global: {
42+
stubs: { ButtonGroup: true },
43+
plugins: [
44+
(app) => {
45+
app.config.globalProperties.$toolbar = mockToolbar;
46+
},
47+
],
48+
},
49+
});
50+
51+
const resizeHandler = addSpy.mock.calls.find((c) => c[0] === 'resize')?.[1];
52+
const keydownHandler = addSpy.mock.calls.find((c) => c[0] === 'keydown')?.[1];
53+
expect(resizeHandler).toBeTypeOf('function');
54+
expect(keydownHandler).toBeTypeOf('function');
55+
56+
removeSpy.mockClear();
57+
wrapper.unmount();
58+
59+
expect(removeSpy).toHaveBeenCalledWith('resize', resizeHandler);
60+
expect(removeSpy).toHaveBeenCalledWith('keydown', keydownHandler);
61+
62+
addSpy.mockRestore();
63+
removeSpy.mockRestore();
64+
});
65+
66+
it('removes window listeners on KeepAlive deactivate and restores them on activate', async () => {
67+
const mockToolbar = createMockToolbar();
68+
const addSpy = vi.spyOn(window, 'addEventListener');
69+
const removeSpy = vi.spyOn(window, 'removeEventListener');
70+
71+
const wrapper = mount(ToolbarKeepAliveHost, {
72+
global: {
73+
stubs: { ButtonGroup: true },
74+
plugins: [
75+
(app) => {
76+
app.config.globalProperties.$toolbar = mockToolbar;
77+
},
78+
],
79+
},
80+
});
81+
82+
const resizeHandler = addSpy.mock.calls.find((c) => c[0] === 'resize')?.[1];
83+
const keydownHandler = addSpy.mock.calls.find((c) => c[0] === 'keydown')?.[1];
84+
expect(resizeHandler).toBeTypeOf('function');
85+
expect(keydownHandler).toBeTypeOf('function');
86+
87+
addSpy.mockClear();
88+
removeSpy.mockClear();
89+
90+
wrapper.vm.visible = false;
91+
await wrapper.vm.$nextTick();
92+
93+
expect(removeSpy).toHaveBeenCalledWith('resize', resizeHandler);
94+
expect(removeSpy).toHaveBeenCalledWith('keydown', keydownHandler);
95+
96+
addSpy.mockClear();
97+
removeSpy.mockClear();
98+
99+
wrapper.vm.visible = true;
100+
await wrapper.vm.$nextTick();
101+
102+
expect(addSpy).toHaveBeenCalledWith('resize', resizeHandler);
103+
expect(addSpy).toHaveBeenCalledWith('keydown', keydownHandler);
104+
105+
removeSpy.mockClear();
106+
wrapper.unmount();
107+
108+
expect(removeSpy).toHaveBeenCalledWith('resize', resizeHandler);
109+
expect(removeSpy).toHaveBeenCalledWith('keydown', keydownHandler);
110+
111+
addSpy.mockRestore();
112+
removeSpy.mockRestore();
113+
});
114+
});

packages/super-editor/src/editors/v1/components/toolbar/Toolbar.vue

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,14 @@
11
<script setup>
2-
import { ref, getCurrentInstance, onMounted, onDeactivated, nextTick, computed } from 'vue';
2+
import {
3+
ref,
4+
getCurrentInstance,
5+
onMounted,
6+
onActivated,
7+
onDeactivated,
8+
onBeforeUnmount,
9+
nextTick,
10+
computed,
11+
} from 'vue';
312
import { throttle } from './helpers.js';
413
import ButtonGroup from './ButtonGroup.vue';
514
@@ -40,16 +49,6 @@ const getFilteredItems = (position) => {
4049
return proxy.$toolbar.getToolbarItemByGroup(position).filter((item) => !excludeButtonsList.includes(item.name.value));
4150
};
4251
43-
onMounted(() => {
44-
window.addEventListener('resize', onResizeThrottled);
45-
window.addEventListener('keydown', onKeyDown);
46-
});
47-
48-
onDeactivated(() => {
49-
window.removeEventListener('resize', onResizeThrottled);
50-
window.removeEventListener('keydown', onKeyDown);
51-
});
52-
5352
const onKeyDown = async (e) => {
5453
if (e.metaKey && e.key === 'f') {
5554
const searchItem = proxy.$toolbar.getToolbarItemByName('search');
@@ -70,6 +69,21 @@ const onWindowResized = async () => {
7069
};
7170
const onResizeThrottled = throttle(onWindowResized, 300);
7271
72+
function teardownWindowListeners() {
73+
window.removeEventListener('resize', onResizeThrottled);
74+
window.removeEventListener('keydown', onKeyDown);
75+
}
76+
77+
function setupWindowListeners() {
78+
window.addEventListener('resize', onResizeThrottled);
79+
window.addEventListener('keydown', onKeyDown);
80+
}
81+
82+
onMounted(setupWindowListeners);
83+
onActivated(setupWindowListeners);
84+
onDeactivated(teardownWindowListeners);
85+
onBeforeUnmount(teardownWindowListeners);
86+
7387
const handleCommand = ({ item, argument, option }) => {
7488
proxy.$toolbar.emitCommand({ item, argument, option });
7589
};

packages/super-editor/src/editors/v1/components/toolbar/super-toolbar.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -976,7 +976,8 @@ export class SuperToolbar extends EventEmitter {
976976

977977
/**
978978
* Cleans up resources when the toolbar is destroyed.
979-
* Clears any pending timeouts to prevent callbacks firing after unmount.
979+
* Clears any pending timeouts and unmounts the Vue app so the Toolbar
980+
* component's onBeforeUnmount hook runs and removes its window listeners.
980981
* @returns {void}
981982
*/
982983
destroy() {
@@ -986,5 +987,6 @@ export class SuperToolbar extends EventEmitter {
986987
}
987988

988989
this.destroyHeadlessToolbar();
990+
this.app?.unmount();
989991
}
990992
}

0 commit comments

Comments
 (0)