Skip to content

Commit 029ae0b

Browse files
Adrienjcabannes
authored andcommitted
fix(catalog-ui): stop click propagation when tree tickeable
The problem was that it was always stop the click propagation when clicking on the node. But when tickeable is false we don't have to stop the click propagation because it is this that does the selection node.
1 parent f0c1fe5 commit 029ae0b

2 files changed

Lines changed: 70 additions & 17 deletions

File tree

apps/catalog-ui/src/components/tree/GenericTree.vue

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,15 +53,15 @@
5353
<template #default-header="prop">
5454
<div
5555
:class="`row items-center full-width tree-header-type-${prop.node.type} tree-header-key-${prop.node.key}`"
56-
@click.stop="toggleNodeSelection(prop.node.key)"
56+
@click="toggleNodeSelection(prop.node.key, $event)"
5757
>
5858
<q-checkbox
5959
v-if="props.tickeable"
6060
:model-value="tickedNodes.includes(prop.node.key)"
6161
v-bind="uiProps.checkbox"
6262
class="tree-header-checkbox"
6363
:data-cy="`generic-tree-checkbox-${prop.node.key}`"
64-
@click="toggleNodeSelection(prop.node.key)"
64+
@click="toggleNodeSelection(prop.node.key, $event)"
6565
/>
6666
<q-icon
6767
v-if="uiProps.types[prop.node.type]?.icon?.name"
@@ -138,6 +138,7 @@ import type {
138138
UiPropsAction,
139139
UiPropsTypes,
140140
} from '../../types/genericTree';
141+
import type { MouseEvent } from 'happy-dom';
141142
142143
const props = defineProps<TreeProps<unknown>>();
143144
const filter = ref<string>('');
@@ -152,11 +153,13 @@ const { toQTreeNodes } = useTree();
152153
/**
153154
* Toggles the selection state of a node.
154155
* @param nodeKey The key of the node to toggle.
156+
* @param event The mouse event on the checkbox or on the node.
155157
*/
156-
function toggleNodeSelection(nodeKey: string): void {
158+
function toggleNodeSelection(nodeKey: string, event: MouseEvent): void {
157159
if (!props.tickeable) {
158160
return;
159161
}
162+
event.stopPropagation();
160163
const index = tickedNodes.value.indexOf(nodeKey);
161164
if (index > -1) {
162165
tickedNodes.value.splice(index, 1);

apps/catalog-ui/tests/unit/components/tree/GenericTree.spec.js

Lines changed: 64 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -290,49 +290,99 @@ describe('Test component: GenericTree', () => {
290290
selectableWrapper = mountComponent({ tickeable: true });
291291
});
292292

293+
/**
294+
* Helper function to create a mock event object with spied stopPropagation.
295+
* @returns Mock event object.
296+
*/
297+
function createMockEvent() {
298+
return {
299+
stopPropagation: vi.fn(),
300+
};
301+
}
302+
303+
it('should call event.stopPropagation() to prevent event bubbling', () => {
304+
const event = createMockEvent();
305+
selectableWrapper.vm.toggleNodeSelection('folder-1', event);
306+
307+
expect(event.stopPropagation).toHaveBeenCalledOnce();
308+
});
309+
293310
it('should add a node to tickedNodes when not already selected', () => {
294-
selectableWrapper.vm.toggleNodeSelection('folder-1');
311+
const event = createMockEvent();
312+
selectableWrapper.vm.toggleNodeSelection('folder-1', event);
295313

296314
expect(selectableWrapper.vm.tickedNodes).toContain('folder-1');
315+
expect(event.stopPropagation).toHaveBeenCalledOnce();
297316
});
298317

299318
it('should remove a node from tickedNodes when already selected', () => {
300-
selectableWrapper.vm.toggleNodeSelection('folder-1');
301-
selectableWrapper.vm.toggleNodeSelection('folder-1');
319+
const event = createMockEvent();
320+
selectableWrapper.vm.toggleNodeSelection('folder-1', event);
321+
selectableWrapper.vm.toggleNodeSelection('folder-1', event);
302322

303323
expect(selectableWrapper.vm.tickedNodes).not.toContain('folder-1');
324+
expect(event.stopPropagation).toHaveBeenCalledTimes(2);
304325
});
305326

306-
it('should handle multiple node selections', () => {
307-
selectableWrapper.vm.toggleNodeSelection('folder-1');
308-
selectableWrapper.vm.toggleNodeSelection('file-1');
327+
it('should handle multiple node selections with different events', () => {
328+
const event1 = createMockEvent();
329+
const event2 = createMockEvent();
330+
selectableWrapper.vm.toggleNodeSelection('folder-1', event1);
331+
selectableWrapper.vm.toggleNodeSelection('file-1', event2);
309332

310333
expect(selectableWrapper.vm.tickedNodes).toContain('folder-1');
311334
expect(selectableWrapper.vm.tickedNodes).toContain('file-1');
312335
expect(selectableWrapper.vm.tickedNodes).toHaveLength(2);
336+
expect(event1.stopPropagation).toHaveBeenCalledOnce();
337+
expect(event2.stopPropagation).toHaveBeenCalledOnce();
313338
});
314339

315340
it('should maintain other selections when toggling a new node', () => {
316-
selectableWrapper.vm.toggleNodeSelection('folder-1');
317-
selectableWrapper.vm.toggleNodeSelection('file-1');
318-
selectableWrapper.vm.toggleNodeSelection('file-1');
341+
const event = createMockEvent();
342+
selectableWrapper.vm.toggleNodeSelection('folder-1', event);
343+
selectableWrapper.vm.toggleNodeSelection('file-1', event);
344+
selectableWrapper.vm.toggleNodeSelection('file-1', event);
319345

320346
expect(selectableWrapper.vm.tickedNodes).toEqual(['folder-1']);
347+
expect(event.stopPropagation).toHaveBeenCalledTimes(3);
321348
});
322349

323-
it('should not toggle when tickeable is false', () => {
350+
it('should not toggle and not call stopPropagation when tickeable is false', () => {
351+
const event = createMockEvent();
324352
const nonSelectableWrapper = mountComponent({ tickeable: false });
325-
nonSelectableWrapper.vm.toggleNodeSelection('folder-1');
353+
nonSelectableWrapper.vm.toggleNodeSelection('folder-1', event);
326354

327355
expect(nonSelectableWrapper.vm.tickedNodes).not.toContain('folder-1');
356+
expect(event.stopPropagation).not.toHaveBeenCalled();
328357
});
329358

330-
it('should emit update:ticked even for a single node toggle', () => {
331-
selectableWrapper.vm.toggleNodeSelection('folder-1');
359+
it('should emit update:ticked with correct payload after adding a node', () => {
360+
const event = createMockEvent();
361+
selectableWrapper.vm.toggleNodeSelection('folder-1', event);
332362

333363
const emitted = selectableWrapper.emitted('update:ticked');
334364
expect(emitted).toBeTruthy();
335-
expect(emitted.length).toBeGreaterThan(0);
365+
expect(emitted[0][0]).toEqual(['folder-1']);
366+
});
367+
368+
it('should emit update:ticked with empty array when toggling off the last node', () => {
369+
const event = createMockEvent();
370+
selectableWrapper.vm.toggleNodeSelection('folder-1', event);
371+
selectableWrapper.vm.toggleNodeSelection('folder-1', event);
372+
373+
const emitted = selectableWrapper.emitted('update:ticked');
374+
expect(emitted[emitted.length - 1][0]).toEqual([]);
375+
});
376+
377+
it('should correctly handle sequential toggles with event stopPropagation', () => {
378+
const event = createMockEvent();
379+
380+
selectableWrapper.vm.toggleNodeSelection('folder-1', event);
381+
selectableWrapper.vm.toggleNodeSelection('file-1', event);
382+
selectableWrapper.vm.toggleNodeSelection('folder-1', event);
383+
384+
expect(event.stopPropagation).toHaveBeenCalledTimes(3);
385+
expect(selectableWrapper.vm.tickedNodes).toEqual(['file-1']);
336386
});
337387
});
338388

0 commit comments

Comments
 (0)