From 994754bf39976c5bb33fd1c30a0eb82cc518850b Mon Sep 17 00:00:00 2001 From: Elias Meire Date: Fri, 2 Feb 2024 16:02:41 +0100 Subject: [PATCH] feat(editor): Add delete and disable button to nodes on hover (#8482) --- cypress/e2e/12-canvas-actions.cy.ts | 50 +++- cypress/e2e/12-canvas.cy.ts | 25 +- packages/editor-ui/src/components/Node.vue | 217 +++++++++--------- .../__snapshots__/useContextMenu.test.ts.snap | 60 ++--- .../src/composables/useNodeHelpers.ts | 11 +- .../src/plugins/i18n/locales/en.json | 28 +-- packages/editor-ui/src/plugins/icons/index.ts | 2 + packages/editor-ui/src/views/NodeView.vue | 2 + 8 files changed, 225 insertions(+), 170 deletions(-) diff --git a/cypress/e2e/12-canvas-actions.cy.ts b/cypress/e2e/12-canvas-actions.cy.ts index 91f6b65884..3c517b6c98 100644 --- a/cypress/e2e/12-canvas-actions.cy.ts +++ b/cypress/e2e/12-canvas-actions.cy.ts @@ -28,6 +28,8 @@ describe('Canvas Actions', () => { WorkflowPage.actions.addNodeToCanvas(EDIT_FIELDS_SET_NODE_NAME); WorkflowPage.getters.nodeViewBackground().click(600, 200, { force: true }); cy.get('.jtk-connector').should('have.length', 1); + + WorkflowPage.getters.nodeViewBackground().click(600, 400, { force: true }); WorkflowPage.actions.addNodeToCanvas(EDIT_FIELDS_SET_NODE_NAME); // Change connection from Set to Set1 @@ -154,17 +156,43 @@ describe('Canvas Actions', () => { WorkflowPage.getters.nodeConnections().should('have.length', 0); }); - it('should execute node', () => { - WorkflowPage.actions.addNodeToCanvas(MANUAL_TRIGGER_NODE_NAME); - WorkflowPage.actions.addNodeToCanvas(CODE_NODE_NAME); - WorkflowPage.getters - .canvasNodes() - .last() - .find('[data-test-id="execute-node-button"]') - .click({ force: true }); - WorkflowPage.getters.successToast().should('contain', 'Node executed successfully'); - WorkflowPage.actions.executeNode(CODE_NODE_NAME); - WorkflowPage.getters.successToast().should('contain', 'Node executed successfully'); + describe('Node hover actions', () => { + it('should execute node', () => { + WorkflowPage.actions.addNodeToCanvas(MANUAL_TRIGGER_NODE_NAME); + WorkflowPage.actions.addNodeToCanvas(CODE_NODE_NAME); + WorkflowPage.getters + .canvasNodes() + .last() + .findChildByTestId('execute-node-button') + .click({ force: true }); + WorkflowPage.actions.executeNode(CODE_NODE_NAME); + WorkflowPage.getters.successToast().should('have.length', 2); + WorkflowPage.getters.successToast().should('contain.text', 'Node executed successfully'); + }); + + it('should disable and enable node', () => { + WorkflowPage.actions.addNodeToCanvas(MANUAL_TRIGGER_NODE_NAME); + WorkflowPage.actions.addNodeToCanvas(CODE_NODE_NAME); + const disableButton = WorkflowPage.getters + .canvasNodes() + .last() + .findChildByTestId('disable-node-button'); + disableButton.click({ force: true }); + WorkflowPage.getters.disabledNodes().should('have.length', 1); + disableButton.click({ force: true }); + WorkflowPage.getters.disabledNodes().should('have.length', 0); + }); + + it('should delete node', () => { + WorkflowPage.actions.addNodeToCanvas(MANUAL_TRIGGER_NODE_NAME); + WorkflowPage.actions.addNodeToCanvas(CODE_NODE_NAME); + WorkflowPage.getters + .canvasNodes() + .last() + .find('[data-test-id="delete-node-button"]') + .click({ force: true }); + WorkflowPage.getters.canvasNodes().should('have.length', 1); + }); }); it('should copy selected nodes', () => { diff --git a/cypress/e2e/12-canvas.cy.ts b/cypress/e2e/12-canvas.cy.ts index 359f0cf851..c1e06c107d 100644 --- a/cypress/e2e/12-canvas.cy.ts +++ b/cypress/e2e/12-canvas.cy.ts @@ -313,21 +313,38 @@ describe('Canvas Node Manipulation and Navigation', () => { WorkflowPage.actions.addNodeToCanvas(CODE_NODE_NAME); cy.get('body').type('{esc}'); cy.get('body').type('{esc}'); - WorkflowPage.actions.selectAll(); // Keyboard shortcut + WorkflowPage.actions.selectAll(); WorkflowPage.actions.hitDisableNodeShortcut(); WorkflowPage.getters.disabledNodes().should('have.length', 2); WorkflowPage.actions.hitDisableNodeShortcut(); WorkflowPage.getters.disabledNodes().should('have.length', 0); + WorkflowPage.actions.deselectAll(); + WorkflowPage.getters.canvasNodeByName(MANUAL_TRIGGER_NODE_DISPLAY_NAME).click(); + WorkflowPage.actions.hitDisableNodeShortcut(); + WorkflowPage.getters.disabledNodes().should('have.length', 1); + WorkflowPage.actions.selectAll(); + WorkflowPage.actions.hitDisableNodeShortcut(); + WorkflowPage.getters.disabledNodes().should('have.length', 2); // Context menu - WorkflowPage.actions.openContextMenu(); - WorkflowPage.actions.contextMenuAction('toggle_activation'); - WorkflowPage.getters.disabledNodes().should('have.length', 2); + WorkflowPage.actions.selectAll(); WorkflowPage.actions.openContextMenu(); WorkflowPage.actions.contextMenuAction('toggle_activation'); WorkflowPage.getters.disabledNodes().should('have.length', 0); + WorkflowPage.actions.openContextMenu(); + WorkflowPage.actions.contextMenuAction('toggle_activation'); + WorkflowPage.getters.disabledNodes().should('have.length', 2); + WorkflowPage.actions.deselectAll(); + WorkflowPage.getters.canvasNodeByName(MANUAL_TRIGGER_NODE_DISPLAY_NAME).click(); + WorkflowPage.actions.openContextMenu(); + WorkflowPage.actions.contextMenuAction('toggle_activation'); + WorkflowPage.getters.disabledNodes().should('have.length', 1); + WorkflowPage.actions.selectAll(); + WorkflowPage.actions.openContextMenu(); + WorkflowPage.actions.contextMenuAction('toggle_activation'); + WorkflowPage.getters.disabledNodes().should('have.length', 2); }); it('should rename node (context menu or shortcut)', () => { diff --git a/packages/editor-ui/src/components/Node.vue b/packages/editor-ui/src/components/Node.vue index 33ef613e39..8d23b7e8b4 100644 --- a/packages/editor-ui/src/components/Node.vue +++ b/packages/editor-ui/src/components/Node.vue @@ -106,24 +106,6 @@ /> -
- - -
+
+ + + + +
+
@@ -437,12 +467,10 @@ export default defineComponent({ } return issues; }, - nodeDisabledIcon(): string { - if (this.data.disabled === false) { - return 'pause'; - } else { - return 'play'; - } + nodeDisabledTitle(): string { + return this.data.disabled + ? this.$locale.baseText('node.enable') + : this.$locale.baseText('node.disable'); }, position(): XYPosition { return this.node ? this.node.position : [0, 0]; @@ -680,6 +708,7 @@ export default defineComponent({ }); } }, + executeNode() { this.$emit('runWorkflow', this.data.name, 'Node.executeNode'); this.$telemetry.track('User clicked node hover button', { @@ -689,6 +718,25 @@ export default defineComponent({ }); }, + deleteNode() { + this.$telemetry.track('User clicked node hover button', { + node_type: this.data.type, + button_name: 'delete', + workflow_id: this.workflowsStore.workflowId, + }); + + this.$emit('removeNode', this.data.name); + }, + + toggleDisableNode() { + this.$telemetry.track('User clicked node hover button', { + node_type: this.data.type, + button_name: 'disable', + workflow_id: this.workflowsStore.workflowId, + }); + this.$emit('toggleDisableNode', this.data); + }, + onClick(event: MouseEvent) { void this.callDebounced(this.onClickDebounced, { debounceTime: 50, trailing: true }, event); }, @@ -778,6 +826,42 @@ export default defineComponent({ } } + &.touch-active, + &:hover, + &.menu-open { + .node-options { + opacity: 1; + } + } + + .node-options { + :deep(.button) { + --button-font-color: var(--color-text-light); + --button-border-radius: 0; + } + cursor: default; + position: absolute; + bottom: 100%; + z-index: 11; + min-width: 100%; + display: flex; + left: calc(-1 * var(--spacing-4xs)); + right: calc(-1 * var(--spacing-4xs)); + justify-content: center; + align-items: center; + padding-bottom: var(--spacing-2xs); + font-size: var(--font-size-s); + opacity: 0; + transition: opacity 100ms ease-in; + + &-inner { + display: flex; + align-items: center; + background-color: var(--color-canvas-background); + border-radius: var(--border-radius-base); + } + } + .node-default { position: absolute; width: 100%; @@ -803,15 +887,6 @@ export default defineComponent({ } } - &.touch-active, - &:hover, - &.menu-open { - .node-options { - pointer-events: all; - opacity: 1; - } - } - .node-executing-info { display: none; position: absolute; @@ -868,65 +943,6 @@ export default defineComponent({ .waiting { color: var(--color-secondary); } - - .node-options { - --node-options-height: 26px; - :deep(.button) { - --button-font-color: var(--color-text-light); - } - position: absolute; - display: flex; - align-items: center; - justify-content: space-between; - gap: var(--spacing-2xs); - transition: opacity 100ms ease-in; - opacity: 0; - pointer-events: none; - top: calc(-1 * (var(--node-options-height) + var(--spacing-4xs))); - left: 0; - width: var(--node-width); - height: var(--node-options-height); - font-size: var(--font-size-s); - z-index: 10; - text-align: center; - - .option { - display: inline-block; - - &.touch { - display: none; - } - - &:hover { - color: $color-primary; - } - - .execute-icon { - position: relative; - font-size: var(----font-size-xl); - } - } - - &:after { - content: ''; - display: block; - position: absolute; - left: 0; - right: 0; - top: -1rem; - bottom: -1rem; - z-index: -1; - } - } - - &.is-touch-device .node-options { - left: -25px; - width: 150px; - - .option.touch { - display: initial; - } - } } &--config { @@ -935,20 +951,12 @@ export default defineComponent({ --node-height: 75px; .node-default { - .node-options { - background: color-mix(in srgb, var(--color-canvas-background) 80%, transparent); - height: 25px; - } - .node-icon { scale: 0.75; } - } - .node-default { .node-box { border: 2px solid var(--color-foreground-xdark); - //background-color: $node-background-type-other; border-radius: 50px; &.executing { @@ -1027,11 +1035,6 @@ export default defineComponent({ left: var(--configurable-node-icon-offset); } - .node-options { - left: 0; - height: 25px; - } - .node-executing-info { left: -67px; } @@ -1172,10 +1175,6 @@ export default defineComponent({ z-index: 100; } -.node-options { - z-index: 10; -} - .drop-add-node-label { z-index: 10; } diff --git a/packages/editor-ui/src/composables/__tests__/__snapshots__/useContextMenu.test.ts.snap b/packages/editor-ui/src/composables/__tests__/__snapshots__/useContextMenu.test.ts.snap index 1a7ac989f0..ec9c211eac 100644 --- a/packages/editor-ui/src/composables/__tests__/__snapshots__/useContextMenu.test.ts.snap +++ b/packages/editor-ui/src/composables/__tests__/__snapshots__/useContextMenu.test.ts.snap @@ -4,7 +4,7 @@ exports[`useContextMenu > Read-only mode > should return the correct actions whe [ { "id": "open", - "label": "Open node...", + "label": "Open...", "shortcut": { "keys": [ "↵", @@ -14,12 +14,12 @@ exports[`useContextMenu > Read-only mode > should return the correct actions whe { "disabled": true, "id": "execute", - "label": "Test node", + "label": "Test step", }, { "disabled": true, "id": "rename", - "label": "Rename node", + "label": "Rename", "shortcut": { "keys": [ "F2", @@ -29,7 +29,7 @@ exports[`useContextMenu > Read-only mode > should return the correct actions whe { "disabled": true, "id": "toggle_activation", - "label": "Deactivate node", + "label": "Deactivate", "shortcut": { "keys": [ "D", @@ -39,7 +39,7 @@ exports[`useContextMenu > Read-only mode > should return the correct actions whe { "disabled": true, "id": "toggle_pin", - "label": "Pin node", + "label": "Pin", "shortcut": { "keys": [ "p", @@ -48,7 +48,7 @@ exports[`useContextMenu > Read-only mode > should return the correct actions whe }, { "id": "copy", - "label": "Copy node", + "label": "Copy", "shortcut": { "keys": [ "C", @@ -59,7 +59,7 @@ exports[`useContextMenu > Read-only mode > should return the correct actions whe { "disabled": true, "id": "duplicate", - "label": "Duplicate node", + "label": "Duplicate", "shortcut": { "keys": [ "D", @@ -88,7 +88,7 @@ exports[`useContextMenu > Read-only mode > should return the correct actions whe "disabled": true, "divided": true, "id": "delete", - "label": "Delete node", + "label": "Delete", "shortcut": { "keys": [ "Del", @@ -117,7 +117,7 @@ exports[`useContextMenu > Read-only mode > should return the correct actions whe }, { "id": "copy", - "label": "Copy sticky note", + "label": "Copy", "shortcut": { "keys": [ "C", @@ -128,7 +128,7 @@ exports[`useContextMenu > Read-only mode > should return the correct actions whe { "disabled": true, "id": "duplicate", - "label": "Duplicate sticky note", + "label": "Duplicate", "shortcut": { "keys": [ "D", @@ -157,7 +157,7 @@ exports[`useContextMenu > Read-only mode > should return the correct actions whe "disabled": true, "divided": true, "id": "delete", - "label": "Delete sticky note", + "label": "Delete", "shortcut": { "keys": [ "Del", @@ -171,7 +171,7 @@ exports[`useContextMenu > should return the correct actions opening the menu fro [ { "id": "open", - "label": "Open node...", + "label": "Open...", "shortcut": { "keys": [ "↵", @@ -181,12 +181,12 @@ exports[`useContextMenu > should return the correct actions opening the menu fro { "disabled": false, "id": "execute", - "label": "Test node", + "label": "Test step", }, { "disabled": false, "id": "rename", - "label": "Rename node", + "label": "Rename", "shortcut": { "keys": [ "F2", @@ -196,7 +196,7 @@ exports[`useContextMenu > should return the correct actions opening the menu fro { "disabled": false, "id": "toggle_activation", - "label": "Deactivate node", + "label": "Deactivate", "shortcut": { "keys": [ "D", @@ -206,7 +206,7 @@ exports[`useContextMenu > should return the correct actions opening the menu fro { "disabled": true, "id": "toggle_pin", - "label": "Pin node", + "label": "Pin", "shortcut": { "keys": [ "p", @@ -215,7 +215,7 @@ exports[`useContextMenu > should return the correct actions opening the menu fro }, { "id": "copy", - "label": "Copy node", + "label": "Copy", "shortcut": { "keys": [ "C", @@ -226,7 +226,7 @@ exports[`useContextMenu > should return the correct actions opening the menu fro { "disabled": true, "id": "duplicate", - "label": "Duplicate node", + "label": "Duplicate", "shortcut": { "keys": [ "D", @@ -255,7 +255,7 @@ exports[`useContextMenu > should return the correct actions opening the menu fro "disabled": false, "divided": true, "id": "delete", - "label": "Delete node", + "label": "Delete", "shortcut": { "keys": [ "Del", @@ -269,7 +269,7 @@ exports[`useContextMenu > should return the correct actions when right clicking [ { "id": "open", - "label": "Open node...", + "label": "Open...", "shortcut": { "keys": [ "↵", @@ -279,12 +279,12 @@ exports[`useContextMenu > should return the correct actions when right clicking { "disabled": false, "id": "execute", - "label": "Test node", + "label": "Test step", }, { "disabled": false, "id": "rename", - "label": "Rename node", + "label": "Rename", "shortcut": { "keys": [ "F2", @@ -294,7 +294,7 @@ exports[`useContextMenu > should return the correct actions when right clicking { "disabled": false, "id": "toggle_activation", - "label": "Deactivate node", + "label": "Deactivate", "shortcut": { "keys": [ "D", @@ -304,7 +304,7 @@ exports[`useContextMenu > should return the correct actions when right clicking { "disabled": true, "id": "toggle_pin", - "label": "Pin node", + "label": "Pin", "shortcut": { "keys": [ "p", @@ -313,7 +313,7 @@ exports[`useContextMenu > should return the correct actions when right clicking }, { "id": "copy", - "label": "Copy node", + "label": "Copy", "shortcut": { "keys": [ "C", @@ -324,7 +324,7 @@ exports[`useContextMenu > should return the correct actions when right clicking { "disabled": true, "id": "duplicate", - "label": "Duplicate node", + "label": "Duplicate", "shortcut": { "keys": [ "D", @@ -353,7 +353,7 @@ exports[`useContextMenu > should return the correct actions when right clicking "disabled": false, "divided": true, "id": "delete", - "label": "Delete node", + "label": "Delete", "shortcut": { "keys": [ "Del", @@ -382,7 +382,7 @@ exports[`useContextMenu > should return the correct actions when right clicking }, { "id": "copy", - "label": "Copy sticky note", + "label": "Copy", "shortcut": { "keys": [ "C", @@ -393,7 +393,7 @@ exports[`useContextMenu > should return the correct actions when right clicking { "disabled": true, "id": "duplicate", - "label": "Duplicate sticky note", + "label": "Duplicate", "shortcut": { "keys": [ "D", @@ -422,7 +422,7 @@ exports[`useContextMenu > should return the correct actions when right clicking "disabled": false, "divided": true, "id": "delete", - "label": "Delete sticky note", + "label": "Delete", "shortcut": { "keys": [ "Del", diff --git a/packages/editor-ui/src/composables/useNodeHelpers.ts b/packages/editor-ui/src/composables/useNodeHelpers.ts index 9b1379f812..00ac6255c1 100644 --- a/packages/editor-ui/src/composables/useNodeHelpers.ts +++ b/packages/editor-ui/src/composables/useNodeHelpers.ts @@ -617,13 +617,18 @@ export function useNodeHelpers() { if (trackHistory) { historyStore.startRecordingUndo(); } + + const newDisabledState = nodes.some((node) => !node.disabled); for (const node of nodes) { - const oldState = node.disabled; + if (newDisabledState === node.disabled) { + continue; + } + // Toggle disabled flag const updateInformation = { name: node.name, properties: { - disabled: !oldState, + disabled: newDisabledState, } as IDataObject, } as INodeUpdatePropertiesInformation; @@ -640,7 +645,7 @@ export function useNodeHelpers() { updateNodesInputIssues(); if (trackHistory) { historyStore.pushCommandToUndo( - new EnableNodeToggleCommand(node.name, oldState === true, node.disabled === true), + new EnableNodeToggleCommand(node.name, node.disabled === true, newDisabledState), ); } } diff --git a/packages/editor-ui/src/plugins/i18n/locales/en.json b/packages/editor-ui/src/plugins/i18n/locales/en.json index 5e6423e3e9..2508419bd4 100644 --- a/packages/editor-ui/src/plugins/i18n/locales/en.json +++ b/packages/editor-ui/src/plugins/i18n/locales/en.json @@ -841,9 +841,11 @@ "node.thisIsATriggerNode": "This is a Trigger node. Learn more", "node.activateDeactivateNode": "Activate/Deactivate Node", "node.changeColor": "Change color", - "node.disabled": "Disabled", - "node.testStep": "Test Step", - "node.deleteNode": "Delete node", + "node.disabled": "Deactivated", + "node.testStep": "Test step", + "node.disable": "Deactivate", + "node.enable": "Activate", + "node.delete": "Delete", "node.issues": "Issues", "node.nodeIsExecuting": "Node is executing", "node.nodeIsWaitingTill": "Node is waiting until {date} {time}", @@ -1104,16 +1106,16 @@ "contextMenu.sticky": "sticky note | sticky notes", "contextMenu.selectAll": "Select all", "contextMenu.deselectAll": "Clear selection", - "contextMenu.duplicate": "Duplicate {subject} | Duplicate {count} {subject}", - "contextMenu.open": "Open node...", - "contextMenu.test": "Test node", - "contextMenu.rename": "Rename node", - "contextMenu.copy": "Copy {subject} | Copy {count} {subject}", - "contextMenu.deactivate": "Deactivate {subject} | Deactivate {count} {subject}", - "contextMenu.activate": "Activate node | Activate {count} nodes", - "contextMenu.pin": "Pin node | Pin {count} nodes", - "contextMenu.unpin": "Unpin node | Unpin {count} nodes", - "contextMenu.delete": "Delete {subject} | Delete {count} {subject}", + "contextMenu.duplicate": "Duplicate | Duplicate {count} {subject}", + "contextMenu.open": "Open...", + "contextMenu.test": "Test step", + "contextMenu.rename": "Rename", + "contextMenu.copy": "Copy | Copy {count} {subject}", + "contextMenu.deactivate": "Deactivate | Deactivate {count} {subject}", + "contextMenu.activate": "Activate | Activate {count} nodes", + "contextMenu.pin": "Pin | Pin {count} nodes", + "contextMenu.unpin": "Unpin | Unpin {count} nodes", + "contextMenu.delete": "Delete | Delete {count} {subject}", "contextMenu.addNode": "Add node", "contextMenu.addSticky": "Add sticky note", "contextMenu.editSticky": "Edit sticky note", diff --git a/packages/editor-ui/src/plugins/icons/index.ts b/packages/editor-ui/src/plugins/icons/index.ts index 338ba2295a..0b23cc90d0 100644 --- a/packages/editor-ui/src/plugins/icons/index.ts +++ b/packages/editor-ui/src/plugins/icons/index.ts @@ -151,6 +151,7 @@ import { faTools, faProjectDiagram, faStream, + faPowerOff, } from '@fortawesome/free-solid-svg-icons'; import { faVariable, faXmark, faVault } from './custom'; import { faStickyNote } from '@fortawesome/free-regular-svg-icons'; @@ -315,6 +316,7 @@ export const FontAwesomePlugin: Plugin<{}> = { addIcon(faGem); addIcon(faXmark); addIcon(faDownload); + addIcon(faPowerOff); app.component('FontAwesomeIcon', FontAwesomeIcon); }, diff --git a/packages/editor-ui/src/views/NodeView.vue b/packages/editor-ui/src/views/NodeView.vue index 1ca1b875fa..fbc28bb615 100644 --- a/packages/editor-ui/src/views/NodeView.vue +++ b/packages/editor-ui/src/views/NodeView.vue @@ -61,6 +61,8 @@ @runWorkflow="onRunNode" @moved="onNodeMoved" @run="onNodeRun" + @removeNode="(name) => removeNode(name, true)" + @toggleDisableNode="(node) => toggleActivationNodes([node])" >