feat(editor): Improve sticky note experience on new canvas (no-changelog) (#11010)
Some checks are pending
Test Master / install-and-build (push) Waiting to run
Test Master / Unit tests (18.x) (push) Blocked by required conditions
Test Master / Unit tests (20.x) (push) Blocked by required conditions
Test Master / Unit tests (22.4) (push) Blocked by required conditions
Test Master / Lint (push) Blocked by required conditions
Test Master / Notify Slack on failure (push) Blocked by required conditions

This commit is contained in:
Alex Grozav 2024-09-30 13:27:37 +03:00 committed by GitHub
parent 6120b3a053
commit c09ae3c18c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 353 additions and 77 deletions

View file

@ -1,5 +1,6 @@
<script setup lang="ts">
import { defineAsyncComponent, reactive } from 'vue';
/* eslint-disable vue/no-multiple-template-root */
import { defineAsyncComponent, onBeforeUnmount, onMounted, ref } from 'vue';
import { getMidCanvasPosition } from '@/utils/nodeViewUtils';
import {
DEFAULT_STICKY_HEIGHT,
@ -10,6 +11,7 @@ import {
import { useUIStore } from '@/stores/ui.store';
import type { AddedNodesAndConnections, ToggleNodeCreatorOptions } from '@/Interface';
import { useActions } from './NodeCreator/composables/useActions';
import { useThrottleFn } from '@vueuse/core';
import KeyboardShortcutTooltip from '@/components/KeyboardShortcutTooltip.vue';
type Props = {
@ -31,41 +33,26 @@ const emit = defineEmits<{
toggleNodeCreator: [value: ToggleNodeCreatorOptions];
}>();
const state = reactive({
showStickyButton: false,
});
const uiStore = useUIStore();
const { getAddedNodesAndConnections } = useActions();
function onCreateMenuHoverIn(mouseinEvent: MouseEvent) {
const buttonsWrapper = mouseinEvent.target as Element;
const wrapperRef = ref<HTMLElement | undefined>();
const wrapperBoundingRect = ref<DOMRect | undefined>();
const isStickyNotesButtonVisible = ref(true);
// Once the popup menu is hovered, it's pointer events are disabled so it's not interfering with element underneath it.
state.showStickyButton = true;
const moveCallback = (mousemoveEvent: MouseEvent) => {
if (buttonsWrapper) {
const wrapperBounds = buttonsWrapper.getBoundingClientRect();
const wrapperH = wrapperBounds.height;
const wrapperW = wrapperBounds.width;
const wrapperLeftNear = wrapperBounds.left;
const wrapperLeftFar = wrapperLeftNear + wrapperW;
const wrapperTopNear = wrapperBounds.top;
const wrapperTopFar = wrapperTopNear + wrapperH;
const inside =
mousemoveEvent.pageX > wrapperLeftNear &&
mousemoveEvent.pageX < wrapperLeftFar &&
mousemoveEvent.pageY > wrapperTopNear &&
mousemoveEvent.pageY < wrapperTopFar;
if (!inside) {
state.showStickyButton = false;
document.removeEventListener('mousemove', moveCallback, false);
}
}
};
document.addEventListener('mousemove', moveCallback, false);
}
const onMouseMove = useThrottleFn((event: MouseEvent) => {
if (wrapperBoundingRect.value) {
const offset = 100;
isStickyNotesButtonVisible.value =
event.clientX >= wrapperBoundingRect.value.left - offset &&
event.clientX <= wrapperBoundingRect.value.right + offset &&
event.clientY >= wrapperBoundingRect.value.top - offset &&
event.clientY <= wrapperBoundingRect.value.bottom + offset;
} else {
isStickyNotesButtonVisible.value = true;
}
}, 250);
function openNodeCreator() {
emit('toggleNodeCreator', {
@ -98,59 +85,61 @@ function nodeTypeSelected(nodeTypes: string[]) {
emit('addNodes', getAddedNodesAndConnections(nodeTypes.map((type) => ({ type }))));
closeNodeCreator(true);
}
onMounted(() => {
wrapperBoundingRect.value = wrapperRef.value?.getBoundingClientRect();
document.addEventListener('mousemove', onMouseMove);
});
onBeforeUnmount(() => {
document.removeEventListener('mousemove', onMouseMove);
});
</script>
<template>
<div>
<div
v-if="!createNodeActive"
:class="[$style.nodeButtonsWrapper, state.showStickyButton ? $style.noEvents : '']"
@mouseenter="onCreateMenuHoverIn"
>
<div :class="$style.nodeCreatorButton" data-test-id="node-creator-plus-button">
<div v-if="!createNodeActive" :class="$style.nodeButtonsWrapper">
<div :class="$style.nodeCreatorButton" ref="wrapperRef" data-test-id="node-creator-plus-button">
<KeyboardShortcutTooltip
:label="$locale.baseText('nodeView.openNodesPanel')"
:shortcut="{ keys: ['Tab'] }"
placement="left"
>
<n8n-icon-button
size="large"
icon="plus"
type="tertiary"
:class="$style.nodeCreatorPlus"
@click="openNodeCreator"
/>
</KeyboardShortcutTooltip>
<div
:class="[$style.addStickyButton, isStickyNotesButtonVisible ? $style.visibleButton : '']"
data-test-id="add-sticky-button"
@click="addStickyNote"
>
<KeyboardShortcutTooltip
:label="$locale.baseText('nodeView.openNodesPanel')"
:shortcut="{ keys: ['Tab'] }"
:label="$locale.baseText('nodeView.addStickyHint')"
:shortcut="{ keys: ['s'], shiftKey: true }"
placement="left"
>
<n8n-icon-button
size="large"
icon="plus"
type="tertiary"
:class="$style.nodeCreatorPlus"
@click="openNodeCreator"
/>
<n8n-icon-button type="tertiary" :icon="['far', 'note-sticky']" />
</KeyboardShortcutTooltip>
<div
:class="[$style.addStickyButton, state.showStickyButton ? $style.visibleButton : '']"
data-test-id="add-sticky-button"
@click="addStickyNote"
>
<KeyboardShortcutTooltip
:label="$locale.baseText('nodeView.addStickyHint')"
:shortcut="{ keys: ['s'], shiftKey: true }"
placement="left"
>
<n8n-icon-button type="tertiary" :icon="['far', 'note-sticky']" />
</KeyboardShortcutTooltip>
</div>
</div>
</div>
<Suspense>
<LazyNodeCreator
:active="createNodeActive"
@node-type-selected="nodeTypeSelected"
@close-node-creator="closeNodeCreator"
/>
</Suspense>
</div>
<Suspense>
<LazyNodeCreator
:active="createNodeActive"
@node-type-selected="nodeTypeSelected"
@close-node-creator="closeNodeCreator"
/>
</Suspense>
</template>
<style lang="scss" module>
.nodeButtonsWrapper {
position: absolute;
width: 150px;
height: 200px;
top: 0;
right: 0;
display: flex;

View file

@ -27,6 +27,7 @@ const renderOptions = computed(() => render.value.options as CanvasNodeStickyNot
const classes = computed(() => ({
[$style.sticky]: true,
[$style.selected]: isSelected.value,
['sticky--active']: isActive.value, // Used to increase the z-index of the sticky note when editing
}));
/**

View file

@ -543,6 +543,141 @@ describe('useCanvasMapping', () => {
});
});
});
describe('additionalNodePropertiesById', () => {
it('should return empty object when there are no sticky nodes', () => {
const nodes = ref([]);
const connections = {};
const workflowObject = createTestWorkflowObject();
const { additionalNodePropertiesById } = useCanvasMapping({
nodes: ref(nodes),
connections: ref(connections),
workflowObject: ref(workflowObject) as Ref<Workflow>,
});
expect(additionalNodePropertiesById.value).toEqual({});
});
it('should calculate zIndex correctly for a single sticky node', () => {
const nodes = [
createTestNode({
id: '1',
type: STICKY_NODE_TYPE,
position: [0, 0],
parameters: { width: 100, height: 100 },
}),
];
const connections = {};
const workflowObject = createTestWorkflowObject();
const { additionalNodePropertiesById } = useCanvasMapping({
nodes: ref(nodes),
connections: ref(connections),
workflowObject: ref(workflowObject) as Ref<Workflow>,
});
expect(additionalNodePropertiesById.value[nodes[0].id]).toEqual({
style: { zIndex: -100 },
});
});
it('should calculate zIndex correctly for multiple sticky nodes with no overlap', () => {
const nodes = [
createTestNode({
id: '1',
type: STICKY_NODE_TYPE,
position: [0, 0],
parameters: { width: 100, height: 100 },
}),
createTestNode({
id: '2',
type: STICKY_NODE_TYPE,
position: [200, 200],
parameters: { width: 100, height: 100 },
}),
];
const connections = {};
const workflowObject = createTestWorkflowObject();
const { additionalNodePropertiesById } = useCanvasMapping({
nodes: ref(nodes),
connections: ref(connections),
workflowObject: ref(workflowObject) as Ref<Workflow>,
});
expect(additionalNodePropertiesById.value[nodes[0].id]).toEqual({
style: { zIndex: -100 },
});
expect(additionalNodePropertiesById.value[nodes[1].id]).toEqual({ style: { zIndex: -99 } });
});
it('should calculate zIndex correctly for overlapping sticky nodes', () => {
const nodes = [
createTestNode({
id: '1',
type: STICKY_NODE_TYPE,
position: [50, 50],
parameters: { width: 100, height: 100 },
}),
createTestNode({
id: '2',
type: STICKY_NODE_TYPE,
position: [0, 0],
parameters: { width: 150, height: 150 },
}),
];
const connections = {};
const workflowObject = createTestWorkflowObject();
const { additionalNodePropertiesById } = useCanvasMapping({
nodes: ref(nodes),
connections: ref(connections),
workflowObject: ref(workflowObject) as Ref<Workflow>,
});
expect(additionalNodePropertiesById.value[nodes[0].id]).toEqual({
style: { zIndex: -99 },
});
expect(additionalNodePropertiesById.value[nodes[1].id]).toEqual({
style: { zIndex: -100 },
});
});
it('calculates zIndex correctly for multiple overlapping sticky nodes', () => {
const nodes = [
createTestNode({
id: '1',
type: STICKY_NODE_TYPE,
position: [0, 0],
parameters: { width: 100, height: 100 },
}),
createTestNode({
id: '2',
type: STICKY_NODE_TYPE,
position: [25, 25],
parameters: { width: 50, height: 50 },
}),
createTestNode({
id: '3',
type: STICKY_NODE_TYPE,
position: [50, 50],
parameters: { width: 100, height: 100 },
}),
];
const connections = {};
const workflowObject = createTestWorkflowObject();
const { additionalNodePropertiesById } = useCanvasMapping({
nodes: ref(nodes),
connections: ref(connections),
workflowObject: ref(workflowObject) as Ref<Workflow>,
});
expect(additionalNodePropertiesById.value[nodes[0].id]).toEqual({
style: { zIndex: -100 },
});
expect(additionalNodePropertiesById.value[nodes[1].id]).toEqual({
style: { zIndex: -98 },
});
expect(additionalNodePropertiesById.value[nodes[2].id]).toEqual({
style: { zIndex: -99 },
});
});
});
});
describe('connections', () => {

View file

@ -9,6 +9,7 @@ import { useWorkflowsStore } from '@/stores/workflows.store';
import type { Ref } from 'vue';
import { computed } from 'vue';
import type {
BoundingBox,
CanvasConnection,
CanvasConnectionData,
CanvasConnectionPort,
@ -22,6 +23,7 @@ import type {
} from '@/types';
import { CanvasConnectionMode, CanvasNodeRenderType } from '@/types';
import {
checkOverlap,
mapLegacyConnectionsToCanvasConnections,
mapLegacyEndpointsToCanvasConnectionPort,
parseCanvasConnectionHandleString,
@ -349,17 +351,68 @@ export function useCanvasMapping({
);
const additionalNodePropertiesById = computed(() => {
return nodes.value.reduce<Record<string, Partial<CanvasNode>>>((acc, node) => {
type StickyNoteBoundingBox = BoundingBox & {
id: string;
area: number;
zIndex: number;
};
const stickyNodeBaseZIndex = -100;
const stickyNodeBoundingBoxes = nodes.value.reduce<StickyNoteBoundingBox[]>((acc, node) => {
if (node.type === STICKY_NODE_TYPE) {
acc[node.id] = {
style: {
zIndex: -1,
},
};
const x = node.position[0];
const y = node.position[1];
const width = node.parameters.width as number;
const height = node.parameters.height as number;
acc.push({
id: node.id,
x,
y,
width,
height,
area: width * height,
zIndex: stickyNodeBaseZIndex,
});
}
return acc;
}, {});
}, []);
const sortedStickyNodeBoundingBoxes = stickyNodeBoundingBoxes.sort((a, b) => b.area - a.area);
sortedStickyNodeBoundingBoxes.forEach((node, index) => {
node.zIndex = stickyNodeBaseZIndex + index;
});
for (let i = 0; i < sortedStickyNodeBoundingBoxes.length; i++) {
const node1 = sortedStickyNodeBoundingBoxes[i];
for (let j = i + 1; j < sortedStickyNodeBoundingBoxes.length; j++) {
const node2 = sortedStickyNodeBoundingBoxes[j];
if (checkOverlap(node1, node2)) {
if (node1.area < node2.area && node1.zIndex <= node2.zIndex) {
// Ensure node1 (smaller area) has a higher zIndex than node2 (larger area)
node1.zIndex = node2.zIndex + 1;
} else if (node2.area < node1.area && node2.zIndex <= node1.zIndex) {
// Ensure node2 (smaller area) has a higher zIndex than node1 (larger area)
node2.zIndex = node1.zIndex + 1;
}
}
}
}
return sortedStickyNodeBoundingBoxes.reduce<Record<string, Partial<CanvasNode>>>(
(acc, node) => {
acc[node.id] = {
style: {
zIndex: node.zIndex,
},
};
return acc;
},
{},
);
});
const mappedNodes = computed<CanvasNode[]>(() => [
@ -491,6 +544,7 @@ export function useCanvasMapping({
}
return {
additionalNodePropertiesById,
nodeExecutionRunDataOutputMapById,
connections: mappedConnections,
nodes: mappedNodes,

View file

@ -1,9 +1,35 @@
.vue-flow__resize-control.line {
border-color: transparent;
z-index: 1;
&.top {
height: var(--spacing-s);
border-top-width: var(--spacing-s);
}
&.right {
width: var(--spacing-s);
border-right-width: var(--spacing-s);
}
&.bottom {
height: var(--spacing-s);
border-bottom-width: var(--spacing-s);
}
&.left {
width: var(--spacing-s);
border-left-width: var(--spacing-s);
}
}
.vue-flow__resize-control.handle {
background-color: transparent;
width: var(--spacing-s);
height: var(--spacing-s);
border: 0;
border-radius: 0;
z-index: 1;
}
.vue-flow__minimap {
@ -40,4 +66,8 @@
&.dragging {
cursor: grabbing;
}
&:has(.sticky--active) {
z-index: 1 !important;
}
}

View file

@ -185,3 +185,10 @@ export type ExecutionOutputMap = {
[outputIndex: string]: ExecutionOutputMapData;
};
};
export type BoundingBox = {
x: number;
y: number;
width: number;
height: number;
};

View file

@ -6,6 +6,7 @@ import {
parseCanvasConnectionHandleString,
createCanvasConnectionHandleString,
createCanvasConnectionId,
checkOverlap,
} from '@/utils/canvasUtilsV2';
import { NodeConnectionType, type IConnections, type INodeTypeDescription } from 'n8n-workflow';
import type { CanvasConnection } from '@/types';
@ -832,3 +833,47 @@ describe('getUniqueNodeName', () => {
expect(result).toBe('Node A mock-uuid');
});
});
describe('checkOverlap', () => {
it('should return true when nodes overlap', () => {
const node1 = { x: 0, y: 0, width: 10, height: 10 };
const node2 = { x: 5, y: 5, width: 10, height: 10 };
expect(checkOverlap(node1, node2)).toBe(true);
});
it('should return false when node1 is completely to the left of node2', () => {
const node1 = { x: 0, y: 0, width: 10, height: 10 };
const node2 = { x: 15, y: 0, width: 10, height: 10 };
expect(checkOverlap(node1, node2)).toBe(false);
});
it('should return false when node2 is completely to the left of node1', () => {
const node1 = { x: 15, y: 0, width: 10, height: 10 };
const node2 = { x: 0, y: 0, width: 10, height: 10 };
expect(checkOverlap(node1, node2)).toBe(false);
});
it('should return false when node1 is completely above node2', () => {
const node1 = { x: 0, y: 0, width: 10, height: 10 };
const node2 = { x: 0, y: 15, width: 10, height: 10 };
expect(checkOverlap(node1, node2)).toBe(false);
});
it('should return false when node2 is completely above node1', () => {
const node1 = { x: 0, y: 15, width: 10, height: 10 };
const node2 = { x: 0, y: 0, width: 10, height: 10 };
expect(checkOverlap(node1, node2)).toBe(false);
});
it('should return false when nodes touch at the edges', () => {
const node1 = { x: 0, y: 0, width: 10, height: 10 };
const node2 = { x: 10, y: 0, width: 10, height: 10 };
expect(checkOverlap(node1, node2)).toBe(false);
});
it('should return false when nodes touch at the corners', () => {
const node1 = { x: 0, y: 0, width: 10, height: 10 };
const node2 = { x: 10, y: 10, width: 10, height: 10 };
expect(checkOverlap(node1, node2)).toBe(false);
});
});

View file

@ -1,6 +1,6 @@
import type { IConnection, IConnections, INodeTypeDescription } from 'n8n-workflow';
import type { INodeUi } from '@/Interface';
import type { CanvasConnection, CanvasConnectionPort } from '@/types';
import type { BoundingBox, CanvasConnection, CanvasConnectionPort } from '@/types';
import { CanvasConnectionMode } from '@/types';
import type { Connection } from '@vue-flow/core';
import { v4 as uuid } from 'uuid';
@ -207,3 +207,18 @@ export function getUniqueNodeName(name: string, existingNames: Set<string>): str
return `${name} ${uuid()}`;
}
export function checkOverlap(node1: BoundingBox, node2: BoundingBox) {
return !(
// node1 is completely to the left of node2
(
node1.x + node1.width <= node2.x ||
// node2 is completely to the left of node1
node2.x + node2.width <= node1.x ||
// node1 is completely above node2
node1.y + node1.height <= node2.y ||
// node2 is completely above node1
node2.y + node2.height <= node1.y
)
);
}