From a632c496d86e0125981ed935731f51c9ba25e1b2 Mon Sep 17 00:00:00 2001 From: Elias Meire Date: Mon, 29 Jan 2024 16:59:51 +0100 Subject: [PATCH 1/9] Add delete and disable button to nodes on hover --- cypress/e2e/12-canvas-actions.cy.ts | 28 ++++++++- packages/editor-ui/src/components/Node.vue | 61 ++++++++++++++++--- .../src/plugins/i18n/locales/en.json | 28 +++++---- packages/editor-ui/src/views/NodeView.vue | 2 + 4 files changed, 96 insertions(+), 23 deletions(-) diff --git a/cypress/e2e/12-canvas-actions.cy.ts b/cypress/e2e/12-canvas-actions.cy.ts index 91f6b65884138..c01051c87567e 100644 --- a/cypress/e2e/12-canvas-actions.cy.ts +++ b/cypress/e2e/12-canvas-actions.cy.ts @@ -160,11 +160,35 @@ describe('Canvas Actions', () => { WorkflowPage.getters .canvasNodes() .last() - .find('[data-test-id="execute-node-button"]') + .findChildByTestId('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'); + WorkflowPage.getters.successToast().should('contain', 'Node executed suÌccessfully'); + }); + + 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/packages/editor-ui/src/components/Node.vue b/packages/editor-ui/src/components/Node.vue index 33ef613e39f41..438c04d3aa647 100644 --- a/packages/editor-ui/src/components/Node.vue +++ b/packages/editor-ui/src/components/Node.vue @@ -106,19 +106,44 @@ /> -
+
+ + 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/views/NodeView.vue b/packages/editor-ui/src/views/NodeView.vue index dd8aa17b3a7a9..b14d7e2cfc131 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])" > @@ -826,6 +826,45 @@ 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; + top: -34px; + 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; + + button { + background-color: var(--color-canvas-background); + } + + &-inner { + display: flex; + align-items: center; + border-radius: var(--border-radius-base); + } + } + .node-default { position: absolute; width: 100%; @@ -851,14 +890,6 @@ export default defineComponent({ } } - &.touch-active, - &:hover, - &.menu-open { - .node-options { - opacity: 1; - } - } - .node-executing-info { display: none; position: absolute; @@ -915,74 +946,6 @@ export default defineComponent({ .waiting { color: var(--color-secondary); } - - .node-options { - :deep(.button) { - --button-font-color: var(--color-text-light); - } - position: absolute; - top: -36px; - 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: var(--spacing-4xs); - font-size: var(--font-size-s); - opacity: 0; - transition: opacity 100ms ease-in; - - button { - background-color: var(--color-canvas-background); - } - - &-inner { - display: flex; - align-items: center; - border-radius: var(--border-radius-base); - } - - &-actions { - display: flex; - align-items: 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 { - .option.touch { - display: initial; - } - } } &--config { @@ -1036,12 +999,6 @@ export default defineComponent({ bottom: 1px !important; right: 1px !important; } - - .node-default .node-options-inner { - padding: 0 var(--spacing-xs); - width: auto; - justify-content: center; - } } } @@ -1084,11 +1041,6 @@ export default defineComponent({ .node-executing-info { left: -67px; } - - .node-options-inner { - width: 100%; - justify-content: space-between; - } } &[data-node-type='@n8n/n8n-nodes-langchain.chatTrigger'] { From e4541b88d3f3dc4e24891b841d7b8cf917c8372a Mon Sep 17 00:00:00 2001 From: Elias Meire Date: Fri, 2 Feb 2024 14:34:54 +0100 Subject: [PATCH 6/9] Fix disable node behavior --- cypress/e2e/12-canvas.cy.ts | 23 ++++++++++++++++--- .../src/composables/useNodeHelpers.ts | 11 ++++++--- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/cypress/e2e/12-canvas.cy.ts b/cypress/e2e/12-canvas.cy.ts index 359f0cf85140c..953ec91c3e4ce 100644 --- a/cypress/e2e/12-canvas.cy.ts +++ b/cypress/e2e/12-canvas.cy.ts @@ -307,27 +307,44 @@ describe('Canvas Node Manipulation and Navigation', () => { WorkflowPage.getters.disabledNodes().should('have.length', 0); }); - it('should disable multiple nodes (context menu or shortcut)', () => { + it.only('should disable multiple nodes (context menu or shortcut)', () => { WorkflowPage.actions.addNodeToCanvas(MANUAL_TRIGGER_NODE_NAME); WorkflowPage.getters.canvasNodeByName(MANUAL_TRIGGER_NODE_DISPLAY_NAME).click(); 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.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', 0); + 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/composables/useNodeHelpers.ts b/packages/editor-ui/src/composables/useNodeHelpers.ts index 9b1379f8120d1..00ac6255c1371 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), ); } } From e1d95bedc563a945683fa9405881f2cc6b4a2d57 Mon Sep 17 00:00:00 2001 From: Elias Meire Date: Fri, 2 Feb 2024 14:39:38 +0100 Subject: [PATCH 7/9] Fix review feedback --- packages/editor-ui/src/components/Node.vue | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/packages/editor-ui/src/components/Node.vue b/packages/editor-ui/src/components/Node.vue index ecfa4528b8358..8d23b7e8b4e89 100644 --- a/packages/editor-ui/src/components/Node.vue +++ b/packages/editor-ui/src/components/Node.vue @@ -841,7 +841,7 @@ export default defineComponent({ } cursor: default; position: absolute; - top: -34px; + bottom: 100%; z-index: 11; min-width: 100%; display: flex; @@ -854,13 +854,10 @@ export default defineComponent({ opacity: 0; transition: opacity 100ms ease-in; - button { - background-color: var(--color-canvas-background); - } - &-inner { display: flex; align-items: center; + background-color: var(--color-canvas-background); border-radius: var(--border-radius-base); } } From ef9d1b1f189b77da942632ba9e54f721e24e22e7 Mon Sep 17 00:00:00 2001 From: Elias Meire Date: Fri, 2 Feb 2024 14:45:41 +0100 Subject: [PATCH 8/9] remove .only --- cypress/e2e/12-canvas.cy.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cypress/e2e/12-canvas.cy.ts b/cypress/e2e/12-canvas.cy.ts index 953ec91c3e4ce..c1e06c107d636 100644 --- a/cypress/e2e/12-canvas.cy.ts +++ b/cypress/e2e/12-canvas.cy.ts @@ -307,7 +307,7 @@ describe('Canvas Node Manipulation and Navigation', () => { WorkflowPage.getters.disabledNodes().should('have.length', 0); }); - it.only('should disable multiple nodes (context menu or shortcut)', () => { + it('should disable multiple nodes (context menu or shortcut)', () => { WorkflowPage.actions.addNodeToCanvas(MANUAL_TRIGGER_NODE_NAME); WorkflowPage.getters.canvasNodeByName(MANUAL_TRIGGER_NODE_DISPLAY_NAME).click(); WorkflowPage.actions.addNodeToCanvas(CODE_NODE_NAME); From 08c8338e6145d094a7b5a93e047e1aff7e04a3dd Mon Sep 17 00:00:00 2001 From: Elias Meire Date: Fri, 2 Feb 2024 15:27:02 +0100 Subject: [PATCH 9/9] Fix e2e tests --- cypress/e2e/12-canvas-actions.cy.ts | 70 +++++++++++++++-------------- 1 file changed, 37 insertions(+), 33 deletions(-) diff --git a/cypress/e2e/12-canvas-actions.cy.ts b/cypress/e2e/12-canvas-actions.cy.ts index c01051c87567e..3c517b6c9840f 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,41 +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() - .findChildByTestId('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 suÌccessfully'); - }); + 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 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 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', () => {