N8N-2586 Improve Waiting Webhook call state in WF Canvas #2430

SchnapsterDog · 2021-11-12T10:00:34Z

No description provided.

packages/editor-ui/src/components/Node.vue

…, it is a trigger node, if it is only one trigger node in WF

packages/editor-ui/src/components/Node.vue

…ctveTriggerNodesInWokrflow, Add style to trigger node's tooltip, remove comments

…nterface, Updated Logic for TriggerNode Tooltip based on the new prop

SchnapsterDog · 2021-11-16T11:31:27Z

@mutdmour Added the new changes for Node-specific overrides (https://www.notion.so/n8n/Improve-Waiting-for-Webhook-Call-state-aa3579a128454e0280dbc93fb8161843)

mutdmour

thanks for making all the fixes. I think we are almost there.

packages/editor-ui/src/components/Node.vue

mutdmour · 2021-11-17T08:31:47Z

packages/editor-ui/src/components/Node.vue

+			handler(isWorkflowRunning) {
+				if (isWorkflowRunning && this.isTriggerNode && this.isSingleActiveTriggerNode && !this.isTriggerNodeTooltipEmpty) {
+					setTimeout(() => {
+						this.showWebhookNodeTooltip = isWorkflowRunning;


this is still buggy.. I think you need to check here again that it's still running

to reproduce:

execute workflow

pause before tooltip shows

if node errors out, still shows tooltip

same thing if node runs successfully before timeout

This three scenarios are fixed with the latest push.
-If node runs successfully before timeout, it would not show the tooltip.
-Also if node hasIssues will not show the tooltip.
-Also if Paused node is paused before the tooltip is shown.

And one more scenario. If one of the nodes are paused during execution, the watcher update the prop.

packages/editor-ui/src/components/Node.vue

packages/editor-ui/src/store.ts

… has issues, if paused, if wokrlfow is running, Refactor Getter

mutdmour · 2021-11-18T16:31:38Z

@SchnapsterDog one more scenario that just came to me

to reproduce:

run workflow with webhook node
open node before tooltip is shown

…it is Draged&Droped on the WF

SchnapsterDog · 2021-11-19T09:23:02Z

@SchnapsterDog one more scenario that just came to me

to reproduce:

run workflow with webhook node

open node before tooltip is shown

This is also fixed.

…ically

…8n-2586-improve-waiting-for-webhook-call-state

…execution

packages/editor-ui/src/components/Node.vue

mutdmour · 2021-11-25T13:28:44Z

packages/editor-ui/src/components/Node.vue

+		getTriggerNodeTooltip (): string | undefined {
+			if (this.nodeType !== null && this.nodeType.hasOwnProperty('eventTriggerDescription')) {
+				return this.nodeType.eventTriggerDescription;
+			} else {
+				return `Waiting for you to create an event in ${this.getTrimmedTriggerNodeTypeName()}`;
+			}
+		},
+		getTrimmedTriggerNodeTypeName() {
+			return this.nodeType && this.nodeType.displayName.replace(/Trigger/, "");
+		},


can you make these into one computed property.. it does not need to be a method...

Done into one computed property.

mutdmour · 2021-11-25T13:29:09Z

packages/editor-ui/src/components/Node.vue

 		nodeType (): INodeTypeDescription | null {
 			return this.$store.getters.nodeType(this.data.type);
 		},
+		node (): INodeUi | undefined {
+			return this.$store.getters.nodesByName[this.name] as INodeUi;


Suggested change

return this.$store.getters.nodesByName[this.name] as INodeUi;

return this.$store.getters.nodesByName[this.name] as INodeUi | undefined;

mutdmour · 2021-11-25T13:31:30Z

packages/editor-ui/src/store.ts

@@ -738,6 +738,11 @@ export const store = new Vuex.Store({
 			return state.activeWorkflows;
 		},

+		activeWorkflowTriggerNodes: (state, getters) => {


maybe rename this to workflowTriggerNodes.. I suggested active before because I thought we would also check if nodes are disabled.
Otherwise update code to only return active (not disabled) nodes

mutdmour · 2021-11-25T13:34:55Z

packages/editor-ui/src/components/Node.vue

+					if (this.workflowRunning) {
+						this.showTriggerNodeTooltip = true;
+					}


we should always check in the handler that we should show tooltip. otherwise we have a race condition between our two handlers.

Suggested change

if (this.workflowRunning) {

this.showTriggerNodeTooltip = true;

}

this.showTriggerNodeTooltip = this.shouldShowTooltip;

mutdmour · 2021-11-25T13:35:30Z

packages/editor-ui/src/components/Node.vue

@@ -207,13 +239,35 @@ export default mixins(externalHooks, nodeBase, nodeHelpers, workflowHelpers).ext
 		shiftOutputCount (): boolean {
 			return !!(this.nodeType && this.nodeType.outputs.length > 2);
 		},
-	},
+		shouldShowTooltip () : boolean {


nitpick - just can be a little more clear... since we have another tooltip in nodes (error tooltip)

Suggested change

shouldShowTooltip () : boolean {

shouldShowTriggerTooltip () : boolean {

packages/editor-ui/src/components/Node.vue

mutdmour

Good job 🙏

SchnapsterDog · 2021-11-25T16:18:47Z

Good job 🙏

Thanks for your directions :)

janober · 2021-11-25T22:34:02Z

Thanks a lot. Got merged.

janober · 2021-11-26T12:58:18Z

Got released with n8n@0.151.0

N8N-2586 Improve Waiting Webhook call state in WF Canvas

e48374e

SchnapsterDog self-assigned this Nov 12, 2021

SchnapsterDog requested a review from mutdmour November 12, 2021 10:00

N8N-2586 Added watcher for showing Webhook's Node Tooltip on execution

eda8d36

SchnapsterDog marked this pull request as ready for review November 12, 2021 11:01

SchnapsterDog requested a review from BHesseldieck November 12, 2021 11:13

mutdmour requested changes Nov 12, 2021

View reviewed changes

N8N-2586 Show helping tooltip for trigger node if wokrflow is running…

9264797

…, it is a trigger node, if it is only one trigger node in WF

SchnapsterDog requested a review from mutdmour November 12, 2021 23:18

mutdmour reviewed Nov 15, 2021

View reviewed changes

packages/editor-ui/src/components/Node.vue Outdated Show resolved Hide resolved

N8N-2586 Rework/Move logic to computed property, Created getter for A…

7fa9c21

…ctveTriggerNodesInWokrflow, Add style to trigger node's tooltip, remove comments

SchnapsterDog requested a review from mutdmour November 16, 2021 08:44

N8N-2586 Added EventTriggerDescription prop in INodeTypeDescription I…

0711ce6

…nterface, Updated Logic for TriggerNode Tooltip based on the new prop

mutdmour reviewed Nov 17, 2021

View reviewed changes

N8N-2586 Add new use cases/watcher to show Trigger Nodes Tooltip / If…

b76d01c

… has issues, if paused, if wokrlfow is running, Refactor Getter

SchnapsterDog added 2 commits November 19, 2021 10:19

N8N-2586 Added z-index to tooltip, Added new Scenario for Tooltip if …

957b387

…it is Draged&Droped on the WF

N8N-2586 Refactor computed property for draged nodes

fae9b07

SchnapsterDog requested a review from mutdmour November 19, 2021 09:23

SchnapsterDog added 6 commits November 19, 2021 10:34

N8N-2586 Fixed conflicts

4704282

N8N-2586 Fixed Conflicts

f0bec7f

N8N-2586 Fixed Tooltip

4d1c96b

N8N-2586 Dont show tooltip on core trigger nodes that execute automat…

f47dbb8

…ically

Merge branch 'master' of https://github.com/n8n-io/n8n into feature/n…

9d601f4

…8n-2586-improve-waiting-for-webhook-call-state

N8N-2586 Fixed Webhook tooltip when adding/deleting canvas during WF …

8089fe5

…execution

mutdmour reviewed Nov 24, 2021

View reviewed changes

N8N-2586 Updated Logic, Simplify the code

e54137a

SchnapsterDog requested a review from mutdmour November 25, 2021 12:23

mutdmour reviewed Nov 25, 2021

View reviewed changes

N8N-2586 Simplify Code

754bf72

SchnapsterDog requested a review from mutdmour November 25, 2021 14:39

SchnapsterDog and others added 5 commits November 25, 2021 15:54

N8N-2586 Added check for nodetype

230372e

update dragging to use local state

6a7d801

N8N-2586 Added eventTriggerDescription to Interval Node

2887a49

add comment, use new getter

5832e08

update to always check

9d7ad66

mutdmour approved these changes Nov 25, 2021

View reviewed changes

janober merged commit adc2515 into master Nov 25, 2021

janober deleted the feature/n8n-2586-improve-waiting-for-webhook-call-state branch November 25, 2021 22:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

N8N-2586 Improve Waiting Webhook call state in WF Canvas #2430

N8N-2586 Improve Waiting Webhook call state in WF Canvas #2430

SchnapsterDog commented Nov 12, 2021

SchnapsterDog commented Nov 16, 2021

mutdmour left a comment

mutdmour Nov 17, 2021

mutdmour Nov 17, 2021

mutdmour Nov 17, 2021

SchnapsterDog Nov 17, 2021

mutdmour commented Nov 18, 2021

SchnapsterDog commented Nov 19, 2021

mutdmour Nov 25, 2021

SchnapsterDog Nov 25, 2021

mutdmour Nov 25, 2021

SchnapsterDog Nov 25, 2021

mutdmour Nov 25, 2021

SchnapsterDog Nov 25, 2021

mutdmour Nov 25, 2021

SchnapsterDog Nov 25, 2021

mutdmour Nov 25, 2021

SchnapsterDog Nov 25, 2021

mutdmour left a comment •

edited

Loading

SchnapsterDog commented Nov 25, 2021

janober commented Nov 25, 2021

janober commented Nov 26, 2021

	return this.$store.getters.nodesByName[this.name] as INodeUi;
	return this.$store.getters.nodesByName[this.name] as INodeUi \| undefined;

	shouldShowTooltip () : boolean {
	shouldShowTriggerTooltip () : boolean {

N8N-2586 Improve Waiting Webhook call state in WF Canvas #2430

N8N-2586 Improve Waiting Webhook call state in WF Canvas #2430

Conversation

SchnapsterDog commented Nov 12, 2021

SchnapsterDog commented Nov 16, 2021

mutdmour left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mutdmour commented Nov 18, 2021

SchnapsterDog commented Nov 19, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mutdmour left a comment • edited Loading

Choose a reason for hiding this comment

SchnapsterDog commented Nov 25, 2021

janober commented Nov 25, 2021

janober commented Nov 26, 2021

mutdmour left a comment •

edited

Loading