fix(editor): Disable context menu actions in read-only mode (#7789)

Github issue / Community forum post (link here to close automatically):
This commit is contained in:
Elias Meire 2023-11-23 17:10:45 +01:00 committed by GitHub
parent 865192adf0
commit 902beffce5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 361 additions and 117 deletions

View file

@ -4,38 +4,47 @@ import { N8nActionDropdown } from 'n8n-design-system';
import type { INode } from 'n8n-workflow'; import type { INode } from 'n8n-workflow';
import { watch, ref } from 'vue'; import { watch, ref } from 'vue';
const { isOpen, actions, position, targetNodes, target, close } = useContextMenu(); const contextMenu = useContextMenu();
const contextMenu = ref<InstanceType<typeof N8nActionDropdown>>(); const { position, isOpen, actions, target } = contextMenu;
const dropdown = ref<InstanceType<typeof N8nActionDropdown>>();
const emit = defineEmits<{ (event: 'action', action: ContextMenuAction, nodes: INode[]): void }>(); const emit = defineEmits<{ (event: 'action', action: ContextMenuAction, nodes: INode[]): void }>();
watch( watch(
isOpen, isOpen,
() => { () => {
if (isOpen) { if (isOpen) {
contextMenu.value?.open(); dropdown.value?.open();
} else { } else {
contextMenu.value?.close(); dropdown.value?.close();
} }
}, },
{ flush: 'post' }, { flush: 'post' },
); );
function onActionSelect(item: string) { function onActionSelect(item: string) {
emit('action', item as ContextMenuAction, targetNodes.value); const action = item as ContextMenuAction;
contextMenu._dispatchAction(action);
emit('action', action, contextMenu.targetNodes.value);
} }
function onVisibleChange(open: boolean) { function onVisibleChange(open: boolean) {
if (!open) { if (!open) {
close(); contextMenu.close();
} }
} }
</script> </script>
<template> <template>
<Teleport v-if="isOpen" to="body"> <Teleport v-if="isOpen" to="body">
<div :class="$style.contextMenu" :style="{ top: `${position[1]}px`, left: `${position[0]}px` }"> <div
:class="$style.contextMenu"
:style="{
left: `${position[0]}px`,
top: `${position[1]}px`,
}"
>
<n8n-action-dropdown <n8n-action-dropdown
ref="contextMenu" ref="dropdown"
:items="actions" :items="actions"
placement="bottom-start" placement="bottom-start"
data-test-id="context-menu" data-test-id="context-menu"

View file

@ -65,6 +65,7 @@
> >
<template #reference> <template #reference>
<div <div
ref="colorPopoverTrigger"
class="option" class="option"
data-test-id="change-sticky-color" data-test-id="change-sticky-color"
:title="$locale.baseText('node.changeColor')" :title="$locale.baseText('node.changeColor')"
@ -100,7 +101,7 @@
</template> </template>
<script lang="ts"> <script lang="ts">
import { defineComponent } from 'vue'; import { defineComponent, ref } from 'vue';
import { mapStores } from 'pinia'; import { mapStores } from 'pinia';
import { externalHooks } from '@/mixins/externalHooks'; import { externalHooks } from '@/mixins/externalHooks';
@ -127,8 +128,18 @@ export default defineComponent({
name: 'Sticky', name: 'Sticky',
mixins: [externalHooks, nodeBase, nodeHelpers, workflowHelpers], mixins: [externalHooks, nodeBase, nodeHelpers, workflowHelpers],
setup() { setup() {
const contextMenu = useContextMenu(); const colorPopoverTrigger = ref<HTMLDivElement>();
return { contextMenu }; const forceActions = ref(false);
const setForceActions = (value: boolean) => {
forceActions.value = value;
};
const contextMenu = useContextMenu((action) => {
if (action === 'change_color') {
setForceActions(true);
colorPopoverTrigger.value?.click();
}
});
return { colorPopoverTrigger, contextMenu, forceActions, setForceActions };
}, },
props: { props: {
nodeViewScale: { nodeViewScale: {
@ -208,17 +219,16 @@ export default defineComponent({
}, },
data() { data() {
return { return {
forceActions: false,
isResizing: false, isResizing: false,
isTouchActive: false, isTouchActive: false,
}; };
}, },
methods: { methods: {
onShowPopover() { onShowPopover() {
this.forceActions = true; this.setForceActions(true);
}, },
onHidePopover() { onHidePopover() {
this.forceActions = false; this.setForceActions(false);
}, },
async deleteNode() { async deleteNode() {
// Wait a tick else vue causes problems because the data is gone // Wait a tick else vue causes problems because the data is gone

View file

@ -1,6 +1,6 @@
// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html
exports[`useContextMenu > should return the correct actions opening the menu from the button 1`] = ` exports[`useContextMenu > Read-only mode > should return the correct actions when right clicking a Node 1`] = `
[ [
{ {
"id": "open", "id": "open",
@ -12,6 +12,7 @@ exports[`useContextMenu > should return the correct actions opening the menu fro
}, },
}, },
{ {
"disabled": true,
"id": "execute", "id": "execute",
"label": "Execute node", "label": "Execute node",
}, },
@ -97,106 +98,10 @@ exports[`useContextMenu > should return the correct actions opening the menu fro
] ]
`; `;
exports[`useContextMenu > should return the correct actions when right clicking a Node 1`] = ` exports[`useContextMenu > Read-only mode > should return the correct actions when right clicking a sticky 1`] = `
[ [
{
"id": "open",
"label": "Open node...",
"shortcut": {
"keys": [
"↵",
],
},
},
{
"id": "execute",
"label": "Execute node",
},
{ {
"disabled": true, "disabled": true,
"id": "rename",
"label": "Rename node",
"shortcut": {
"keys": [
"F2",
],
},
},
{
"disabled": true,
"id": "toggle_activation",
"label": "Deactivate node",
"shortcut": {
"keys": [
"D",
],
},
},
{
"disabled": true,
"id": "toggle_pin",
"label": "Pin node",
"shortcut": {
"keys": [
"p",
],
},
},
{
"id": "copy",
"label": "Copy node",
"shortcut": {
"keys": [
"C",
],
"metaKey": true,
},
},
{
"disabled": true,
"id": "duplicate",
"label": "Duplicate node",
"shortcut": {
"keys": [
"D",
],
"metaKey": true,
},
},
{
"disabled": false,
"divided": true,
"id": "select_all",
"label": "Select all",
"shortcut": {
"keys": [
"A",
],
"metaKey": true,
},
},
{
"disabled": false,
"id": "deselect_all",
"label": "Clear selection",
},
{
"disabled": true,
"divided": true,
"id": "delete",
"label": "Delete node",
"shortcut": {
"keys": [
"Del",
],
},
},
]
`;
exports[`useContextMenu > should return the correct actions when right clicking a sticky 1`] = `
[
{
"id": "open", "id": "open",
"label": "Edit sticky note", "label": "Edit sticky note",
"shortcut": { "shortcut": {
@ -205,6 +110,11 @@ exports[`useContextMenu > should return the correct actions when right clicking
], ],
}, },
}, },
{
"disabled": true,
"id": "change_color",
"label": "Change color",
},
{ {
"id": "copy", "id": "copy",
"label": "Copy sticky note", "label": "Copy sticky note",
@ -257,10 +167,275 @@ exports[`useContextMenu > should return the correct actions when right clicking
] ]
`; `;
exports[`useContextMenu > should return the correct actions opening the menu from the button 1`] = `
[
{
"id": "open",
"label": "Open node...",
"shortcut": {
"keys": [
"↵",
],
},
},
{
"disabled": false,
"id": "execute",
"label": "Execute node",
},
{
"disabled": false,
"id": "rename",
"label": "Rename node",
"shortcut": {
"keys": [
"F2",
],
},
},
{
"disabled": false,
"id": "toggle_activation",
"label": "Deactivate node",
"shortcut": {
"keys": [
"D",
],
},
},
{
"disabled": true,
"id": "toggle_pin",
"label": "Pin node",
"shortcut": {
"keys": [
"p",
],
},
},
{
"id": "copy",
"label": "Copy node",
"shortcut": {
"keys": [
"C",
],
"metaKey": true,
},
},
{
"disabled": true,
"id": "duplicate",
"label": "Duplicate node",
"shortcut": {
"keys": [
"D",
],
"metaKey": true,
},
},
{
"disabled": false,
"divided": true,
"id": "select_all",
"label": "Select all",
"shortcut": {
"keys": [
"A",
],
"metaKey": true,
},
},
{
"disabled": false,
"id": "deselect_all",
"label": "Clear selection",
},
{
"disabled": false,
"divided": true,
"id": "delete",
"label": "Delete node",
"shortcut": {
"keys": [
"Del",
],
},
},
]
`;
exports[`useContextMenu > should return the correct actions when right clicking a Node 1`] = `
[
{
"id": "open",
"label": "Open node...",
"shortcut": {
"keys": [
"↵",
],
},
},
{
"disabled": false,
"id": "execute",
"label": "Execute node",
},
{
"disabled": false,
"id": "rename",
"label": "Rename node",
"shortcut": {
"keys": [
"F2",
],
},
},
{
"disabled": false,
"id": "toggle_activation",
"label": "Deactivate node",
"shortcut": {
"keys": [
"D",
],
},
},
{
"disabled": true,
"id": "toggle_pin",
"label": "Pin node",
"shortcut": {
"keys": [
"p",
],
},
},
{
"id": "copy",
"label": "Copy node",
"shortcut": {
"keys": [
"C",
],
"metaKey": true,
},
},
{
"disabled": true,
"id": "duplicate",
"label": "Duplicate node",
"shortcut": {
"keys": [
"D",
],
"metaKey": true,
},
},
{
"disabled": false,
"divided": true,
"id": "select_all",
"label": "Select all",
"shortcut": {
"keys": [
"A",
],
"metaKey": true,
},
},
{
"disabled": false,
"id": "deselect_all",
"label": "Clear selection",
},
{
"disabled": false,
"divided": true,
"id": "delete",
"label": "Delete node",
"shortcut": {
"keys": [
"Del",
],
},
},
]
`;
exports[`useContextMenu > should return the correct actions when right clicking a sticky 1`] = `
[
{
"disabled": false,
"id": "open",
"label": "Edit sticky note",
"shortcut": {
"keys": [
"↵",
],
},
},
{
"disabled": false,
"id": "change_color",
"label": "Change color",
},
{
"id": "copy",
"label": "Copy sticky note",
"shortcut": {
"keys": [
"C",
],
"metaKey": true,
},
},
{
"disabled": true,
"id": "duplicate",
"label": "Duplicate sticky note",
"shortcut": {
"keys": [
"D",
],
"metaKey": true,
},
},
{
"disabled": false,
"divided": true,
"id": "select_all",
"label": "Select all",
"shortcut": {
"keys": [
"A",
],
"metaKey": true,
},
},
{
"disabled": false,
"id": "deselect_all",
"label": "Clear selection",
},
{
"disabled": false,
"divided": true,
"id": "delete",
"label": "Delete sticky note",
"shortcut": {
"keys": [
"Del",
],
},
},
]
`;
exports[`useContextMenu > should support opening and closing (default = right click on canvas) 1`] = ` exports[`useContextMenu > should support opening and closing (default = right click on canvas) 1`] = `
[ [
{ {
"disabled": true, "disabled": false,
"id": "toggle_activation", "id": "toggle_activation",
"label": "Deactivate 2 nodes", "label": "Deactivate 2 nodes",
"shortcut": { "shortcut": {
@ -318,7 +493,7 @@ exports[`useContextMenu > should support opening and closing (default = right cl
"label": "Clear selection", "label": "Clear selection",
}, },
{ {
"disabled": true, "disabled": false,
"divided": true, "divided": true,
"id": "delete", "id": "delete",
"label": "Delete 2 nodes", "label": "Delete 2 nodes",

View file

@ -4,6 +4,7 @@ import { NO_OP_NODE_TYPE, STICKY_NODE_TYPE, STORES } from '@/constants';
import { faker } from '@faker-js/faker'; import { faker } from '@faker-js/faker';
import { createTestingPinia } from '@pinia/testing'; import { createTestingPinia } from '@pinia/testing';
import { setActivePinia } from 'pinia'; import { setActivePinia } from 'pinia';
import { useSourceControlStore, useUIStore } from '@/stores';
const nodeFactory = (data: Partial<INodeUi> = {}): INodeUi => ({ const nodeFactory = (data: Partial<INodeUi> = {}): INodeUi => ({
id: faker.string.uuid(), id: faker.string.uuid(),
@ -16,6 +17,8 @@ const nodeFactory = (data: Partial<INodeUi> = {}): INodeUi => ({
}); });
describe('useContextMenu', () => { describe('useContextMenu', () => {
let sourceControlStore: ReturnType<typeof useSourceControlStore>;
let uiStore: ReturnType<typeof useUIStore>;
const nodes = [nodeFactory(), nodeFactory(), nodeFactory()]; const nodes = [nodeFactory(), nodeFactory(), nodeFactory()];
const selectedNodes = nodes.slice(0, 2); const selectedNodes = nodes.slice(0, 2);
@ -28,6 +31,12 @@ describe('useContextMenu', () => {
}, },
}), }),
); );
sourceControlStore = useSourceControlStore();
uiStore = useUIStore();
vi.spyOn(uiStore, 'isReadOnlyView', 'get').mockReturnValue(false);
vi.spyOn(sourceControlStore, 'preferences', 'get').mockReturnValue({
branchReadOnly: false,
} as never);
}); });
afterEach(() => { afterEach(() => {
@ -89,4 +98,28 @@ describe('useContextMenu', () => {
expect(actions.value).toMatchSnapshot(); expect(actions.value).toMatchSnapshot();
expect(targetNodes.value).toEqual([node]); expect(targetNodes.value).toEqual([node]);
}); });
describe('Read-only mode', () => {
it('should return the correct actions when right clicking a sticky', () => {
vi.spyOn(uiStore, 'isReadOnlyView', 'get').mockReturnValue(true);
const { open, isOpen, actions, targetNodes } = useContextMenu();
const sticky = nodeFactory({ type: STICKY_NODE_TYPE });
open(mockEvent, { source: 'node-right-click', node: sticky });
expect(isOpen.value).toBe(true);
expect(actions.value).toMatchSnapshot();
expect(targetNodes.value).toEqual([sticky]);
});
it('should return the correct actions when right clicking a Node', () => {
vi.spyOn(uiStore, 'isReadOnlyView', 'get').mockReturnValue(true);
const { open, isOpen, actions, targetNodes } = useContextMenu();
const node = nodeFactory();
open(mockEvent, { source: 'node-right-click', node });
expect(isOpen.value).toBe(true);
expect(actions.value).toMatchSnapshot();
expect(targetNodes.value).toEqual([node]);
});
});
}); });

View file

@ -16,6 +16,7 @@ export type ContextMenuTarget =
| { source: 'canvas' } | { source: 'canvas' }
| { source: 'node-right-click'; node: INode } | { source: 'node-right-click'; node: INode }
| { source: 'node-button'; node: INode }; | { source: 'node-button'; node: INode };
export type ContextMenuActionCallback = (action: ContextMenuAction, targets: INode[]) => void;
export type ContextMenuAction = export type ContextMenuAction =
| 'open' | 'open'
| 'copy' | 'copy'
@ -28,14 +29,16 @@ export type ContextMenuAction =
| 'select_all' | 'select_all'
| 'deselect_all' | 'deselect_all'
| 'add_node' | 'add_node'
| 'add_sticky'; | 'add_sticky'
| 'change_color';
const position = ref<XYPosition>([0, 0]); const position = ref<XYPosition>([0, 0]);
const isOpen = ref(false); const isOpen = ref(false);
const target = ref<ContextMenuTarget>({ source: 'canvas' }); const target = ref<ContextMenuTarget>({ source: 'canvas' });
const actions = ref<IActionDropdownItem[]>([]); const actions = ref<IActionDropdownItem[]>([]);
const actionCallback = ref<ContextMenuActionCallback>(() => {});
export const useContextMenu = () => { export const useContextMenu = (onAction: ContextMenuActionCallback = () => {}) => {
const uiStore = useUIStore(); const uiStore = useUIStore();
const nodeTypesStore = useNodeTypesStore(); const nodeTypesStore = useNodeTypesStore();
const workflowsStore = useWorkflowsStore(); const workflowsStore = useWorkflowsStore();
@ -104,6 +107,7 @@ export const useContextMenu = () => {
event.preventDefault(); event.preventDefault();
actionCallback.value = onAction;
target.value = menuTarget; target.value = menuTarget;
position.value = getMousePosition(event); position.value = getMousePosition(event);
isOpen.value = true; isOpen.value = true;
@ -196,6 +200,12 @@ export const useContextMenu = () => {
id: 'open', id: 'open',
label: i18n.baseText('contextMenu.editSticky'), label: i18n.baseText('contextMenu.editSticky'),
shortcut: { keys: ['↵'] }, shortcut: { keys: ['↵'] },
disabled: isReadOnly.value,
},
{
id: 'change_color',
label: i18n.baseText('contextMenu.changeColor'),
disabled: isReadOnly.value,
}, },
] ]
: [ : [
@ -207,6 +217,7 @@ export const useContextMenu = () => {
{ {
id: 'execute', id: 'execute',
label: i18n.baseText('contextMenu.execute'), label: i18n.baseText('contextMenu.execute'),
disabled: isReadOnly.value,
}, },
{ {
id: 'rename', id: 'rename',
@ -223,6 +234,10 @@ export const useContextMenu = () => {
} }
}; };
const _dispatchAction = (action: ContextMenuAction) => {
actionCallback.value(action, targetNodes.value);
};
watch( watch(
() => uiStore.nodeViewOffsetPosition, () => uiStore.nodeViewOffsetPosition,
() => { () => {
@ -238,5 +253,6 @@ export const useContextMenu = () => {
targetNodes, targetNodes,
open, open,
close, close,
_dispatchAction,
}; };
}; };

View file

@ -820,7 +820,7 @@
"noTagsView.withWorkflowTagsYouReFree": "With workflow tags, you're free to create the perfect tagging system for your flows", "noTagsView.withWorkflowTagsYouReFree": "With workflow tags, you're free to create the perfect tagging system for your flows",
"node.thisIsATriggerNode": "This is a Trigger node. <a target=\"_blank\" href=\"https://docs.n8n.io/workflows/components/nodes/\">Learn more</a>", "node.thisIsATriggerNode": "This is a Trigger node. <a target=\"_blank\" href=\"https://docs.n8n.io/workflows/components/nodes/\">Learn more</a>",
"node.activateDeactivateNode": "Activate/Deactivate Node", "node.activateDeactivateNode": "Activate/Deactivate Node",
"node.changeColor": "Change Color", "node.changeColor": "Change color",
"node.disabled": "Disabled", "node.disabled": "Disabled",
"node.executeNode": "Execute node", "node.executeNode": "Execute node",
"node.deleteNode": "Delete node", "node.deleteNode": "Delete node",
@ -1084,6 +1084,7 @@
"contextMenu.addNode": "Add node", "contextMenu.addNode": "Add node",
"contextMenu.addSticky": "Add sticky note", "contextMenu.addSticky": "Add sticky note",
"contextMenu.editSticky": "Edit sticky note", "contextMenu.editSticky": "Edit sticky note",
"contextMenu.changeColor": "Change color",
"nodeWebhooks.clickToCopyWebhookUrls": "Click to copy webhook URLs", "nodeWebhooks.clickToCopyWebhookUrls": "Click to copy webhook URLs",
"nodeWebhooks.clickToCopyWebhookUrls.formTrigger": "Click to copy Form URL", "nodeWebhooks.clickToCopyWebhookUrls.formTrigger": "Click to copy Form URL",
"nodeWebhooks.clickToDisplayWebhookUrls": "Click to display webhook URLs", "nodeWebhooks.clickToDisplayWebhookUrls": "Click to display webhook URLs",