From 38e4b562808dadbe47d4fcdf4c907e6d1c374a33 Mon Sep 17 00:00:00 2001 From: Tomi Turtiainen <10324676+tomi@users.noreply.github.com> Date: Fri, 1 Dec 2023 16:00:48 +0200 Subject: [PATCH 1/3] fix(editor): Tune styles of template credential setup (no-changelog) (#7898) --- .../AppsRequiringCredsNotice.vue | 8 +++++++- .../SetupTemplateFormStep.vue | 5 ++++- .../SetupWorkflowFromTemplateView.vue | 5 +++-- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/packages/editor-ui/src/views/SetupWorkflowFromTemplateView/AppsRequiringCredsNotice.vue b/packages/editor-ui/src/views/SetupWorkflowFromTemplateView/AppsRequiringCredsNotice.vue index 45f7e439200a7..87e44b489f01f 100644 --- a/packages/editor-ui/src/views/SetupWorkflowFromTemplateView/AppsRequiringCredsNotice.vue +++ b/packages/editor-ui/src/views/SetupWorkflowFromTemplateView/AppsRequiringCredsNotice.vue @@ -22,9 +22,15 @@ const appNodeCounts = computed(() => { + + diff --git a/packages/editor-ui/src/views/SetupWorkflowFromTemplateView/SetupTemplateFormStep.vue b/packages/editor-ui/src/views/SetupWorkflowFromTemplateView/SetupTemplateFormStep.vue index 161d501d00a17..ae67e6d2191ad 100644 --- a/packages/editor-ui/src/views/SetupWorkflowFromTemplateView/SetupTemplateFormStep.vue +++ b/packages/editor-ui/src/views/SetupWorkflowFromTemplateView/SetupTemplateFormStep.vue @@ -118,7 +118,7 @@ const onCredentialDeselected = () => { .heading { display: flex; align-items: center; - margin-bottom: var(--spacing-xs); + margin-bottom: var(--spacing-2xs); } .headingOrder { @@ -132,6 +132,8 @@ const onCredentialDeselected = () => { .description { margin-bottom: var(--spacing-l); + font-size: var(--font-size-s); + color: var(--color-text-base); } .credentials { @@ -146,6 +148,7 @@ const onCredentialDeselected = () => { .credentialOk { margin-left: var(--spacing-2xs); + font-size: 24px; } .invisible { diff --git a/packages/editor-ui/src/views/SetupWorkflowFromTemplateView/SetupWorkflowFromTemplateView.vue b/packages/editor-ui/src/views/SetupWorkflowFromTemplateView/SetupWorkflowFromTemplateView.vue index 13ffcb1b0e97b..f985a36f68ca8 100644 --- a/packages/editor-ui/src/views/SetupWorkflowFromTemplateView/SetupWorkflowFromTemplateView.vue +++ b/packages/editor-ui/src/views/SetupWorkflowFromTemplateView/SetupWorkflowFromTemplateView.vue @@ -143,6 +143,7 @@ onMounted(async () => { :disabled="setupTemplateStore.numCredentialsLeft === 0" > { .grid { display: grid; grid-template-columns: repeat(12, 1fr); - padding: var(--spacing-l) var(--spacing-l) 0; + padding: 0 var(--spacing-l); justify-content: center; } @@ -191,7 +192,7 @@ onMounted(async () => { .appCredential:not(:last-of-type) { padding-bottom: var(--spacing-2xl); - border-bottom: 1px solid var(--prim-gray-540); + border-bottom: 1px solid var(--color-foreground-light); } .actions { From 403ba6e9ca374c641ea2e9869ba4c6b57b5d975b Mon Sep 17 00:00:00 2001 From: Mutasem Aldmour <4711238+mutdmour@users.noreply.github.com> Date: Fri, 1 Dec 2023 16:27:51 +0100 Subject: [PATCH 2/3] docs: Create PR template (no-changelog) (#7902) ## Summary Provide details about your pull request and what it adds, fixes, or changes. Photos and videos are recommended. Add PR template #### How to test the change: 1. Create new PR. ## Issues fixed Include links to Github issue or Community forum post or **Linear ticket**: > Important in order to close automatically and provide context to reviewers [ADO-1200](https://linear.app/n8n/issue/ADO-1200/tech-debt-checklist) ## Review / Merge checklist - [X] PR title and summary are descriptive. **Remember, the title automatically goes into the changelog. Use `(no-changelog)` otherwise.** ([convetions](./blob/master/pull_request_title_conventions.md)) - [ ] [Docs updated](https://github.com/n8n-io/n8n-docs) or follow-up ticket created. - [ ] Tests included. > A feature is not complete without tests. > A bug is not considered fixed, unless a test is added to prevent it from happening again. > > *(internal)* You can use Slack commands to trigger [e2e tests](https://www.notion.so/n8n/How-to-use-Test-Instances-d65f49dfc51f441ea44367fb6f67eb0a?pvs=4#a39f9e5ba64a48b58a71d81c837e8227) or [deploy test instance](https://www.notion.so/n8n/How-to-use-Test-Instances-d65f49dfc51f441ea44367fb6f67eb0a?pvs=4#f6a177d32bde4b57ae2da0b8e454bfce) or [deploy early access version on Cloud](https://www.notion.so/n8n/Cloudbot-3dbe779836004972b7057bc989526998?pvs=4#fef2d36ab02247e1a0f65a74f6fb534e). --- .github/pull_request_template.md | 24 ++++- .github/pull_request_title_conventions.md | 112 ++++++++++++++++++++++ 2 files changed, 135 insertions(+), 1 deletion(-) create mode 100644 .github/pull_request_title_conventions.md diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index bd399104db8d6..804b35675f687 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -1 +1,23 @@ -Github issue / Community forum post (link here to close automatically): +## Summary +Provide details about your pull request and what it adds, fixes, or changes. Photos and videos are recommended. + +... + +#### How to test the change: +1. ... + + +## Issues fixed +Include links to Github issue or Community forum post or **Linear ticket**: +> Important in order to close automatically and provide context to reviewers + +... + + +## Review / Merge checklist +- [ ] PR title and summary are descriptive. **Remember, the title automatically goes into the changelog. Use `(no-changelog)` otherwise.** ([conventions](https://github.com/n8n-io/n8n/blob/master/.github/pull_request_title_conventions.md)) +- [ ] [Docs updated](https://github.com/n8n-io/n8n-docs) or follow-up ticket created. +- [ ] Tests included. + > A bug is not considered fixed, unless a test is added to prevent it from happening again. A feature is not complete without tests. + > + > *(internal)* You can use Slack commands to trigger [e2e tests](https://www.notion.so/n8n/How-to-use-Test-Instances-d65f49dfc51f441ea44367fb6f67eb0a?pvs=4#a39f9e5ba64a48b58a71d81c837e8227) or [deploy test instance](https://www.notion.so/n8n/How-to-use-Test-Instances-d65f49dfc51f441ea44367fb6f67eb0a?pvs=4#f6a177d32bde4b57ae2da0b8e454bfce) or [deploy early access version on Cloud](https://www.notion.so/n8n/Cloudbot-3dbe779836004972b7057bc989526998?pvs=4#fef2d36ab02247e1a0f65a74f6fb534e). \ No newline at end of file diff --git a/.github/pull_request_title_conventions.md b/.github/pull_request_title_conventions.md new file mode 100644 index 0000000000000..f6f762048f311 --- /dev/null +++ b/.github/pull_request_title_conventions.md @@ -0,0 +1,112 @@ +# PR Title Convention + +We have very precise rules over how Pull Requests (to the `master` branch) must be formatted. This format basically follows the [Angular Commit Message Convention](https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit). It leads to easier to read commit history and allows for automated generation of release notes: + +A PR title consists of these elements: + +``` +(): + │ │ │ + │ │ └─⫸ Summary: In imperative present tense. + | | Capitalized + | | No period at the end. + │ │ + │ └─⫸ Scope: API|core|editor|* Node + │ + └─⫸ Type: build|ci|docs|feat|fix|perf|refactor|test +``` + +- PR title + - type + - scope (*optional*) + - summary +- PR description + - body (optional) + - blank line + - footer (optional) + +The structure looks like this: + +### **Type** + +Must be one of the following: + +- `feat` - A new feature +- `fix` - A bug fix +- `perf` - A code change that improves performance +- `test` - Adding missing tests or correcting existing tests +- `docs` - Documentation only changes +- `refactor` - A code change that neither fixes a bug nor adds a feature +- `build` - Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm) +- `ci` - Changes to our CI configuration files and scripts (e.g. Github actions) + +If the prefix is `feat`, `fix` or `perf`, it will appear in the changelog. However if there is any BREAKING CHANGE (see Footer section below), the commit will always appear in the changelog. + +### **Scope (optional)** + +The scope should specify the place of the commit change as long as the commit clearly addresses one of the following supported scopes. (Otherwise, omit the scope!) + +- `API` - changes to the *public* API +- `core` - changes to the core / private API / backend of n8n +- `editor` - changes to the Editor UI +- `* Node` - changes to a specific node or trigger node (”`*`” to be replaced with the node name, not its display name), e.g. + - mattermost → Mattermost Node + - microsoftToDo → Microsoft To Do Node + - n8n → n8n Node + +### **Summary** + +The summary contains succinct description of the change: + +- use the imperative, present tense: "change" not "changed" nor "changes" +- capitalize the first letter +- *no* dot (.) at the end +- do *not* include Linear ticket IDs etc. (e.g. N8N-1234) +- suffix with “(no-changelog)” for commits / PRs that should not get mentioned in the changelog. + +### **Body (optional)** + +Just as in the **summary**, use the imperative, present tense: "change" not "changed" nor "changes". The body should include the motivation for the change and contrast this with previous behavior. + +### **Footer (optional)** + +The footer can contain information about breaking changes and deprecations and is also the place to [reference GitHub issues](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword), Linear tickets, and other PRs that this commit closes or is related to. For example: + +``` +BREAKING CHANGE: + + + + +Fixes # +``` + +or + +``` +DEPRECATED: + + + + +Closes # +``` + +A Breaking Change section should start with the phrase "`BREAKING CHANGE:` " followed by a summary of the breaking change, a blank line, and a detailed description of the breaking change that also includes migration instructions. + +> 💡 A breaking change can additionally also be marked by adding a “`!`” to the header, right before the “`:`”, e.g. `feat(editor)!: Remove support for dark mode` +> +> This makes locating breaking changes easier when just skimming through commit messages. + +> 💡 The breaking changes must also be added to the [packages/cli/BREAKING-CHANGES.md](https://github.com/n8n-io/n8n/blob/master/packages/cli/BREAKING-CHANGES.md) file located in the n8n repository. + +Similarly, a Deprecation section should start with "`DEPRECATED:` " followed by a short description of what is deprecated, a blank line, and a detailed description of the deprecation that also mentions the recommended update path. + +### **Revert commits** + +If the commit reverts a previous commit, it should begin with `revert:` , followed by the header of the reverted commit. + +The content of the commit message body should contain: + +- information about the SHA of the commit being reverted in the following format: `This reverts commit `, +- a clear description of the reason for reverting the commit message. \ No newline at end of file From a8049a0def21506ebf4fb1d3b69ae28ec35fdc21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milorad=20FIlipovi=C4=87?= Date: Fri, 1 Dec 2023 17:50:11 +0100 Subject: [PATCH 3/3] perf(editor): Improve node rendering performance when opening large workflows (#7904) ## Summary In an effort to do as little processing as possible in each `Node` component, this PR passes current workflow object to it as a property instead of calling the slow `getCurrentWorkflow` store getter a few times in each node. This should substantially improve loading times for large workflows. As a benchmark, I was using a workflow from [this Linear ticket](https://linear.app/n8n/issue/ADO-1501/deliveryhero-enterprise-instance-very-slow-loading-workflows) and this fix brought down opening time by **20 seconds**. Together with fixes from #7901, this workflow was opening in less than **10 seconds** on my laptop. [Latest e2e run](https://github.com/n8n-io/n8n/actions/runs/7062162739) #### How to test the change: 1. Open a large workflow 2. Observe loading time ## Issues fixed ADO-1523 https://community.n8n.io/t/ui-very-slow-with-more-than-100-nodes/8236/14 ## Review / Merge checklist - [x] PR title and summary are descriptive. **Remember, the title automatically goes into the changelog. Use `(no-changelog)` otherwise.** ([conventions](https://github.com/n8n-io/n8n/blob/master/.github/pull_request_title_conventions.md)) - [ ] [Docs updated](https://github.com/n8n-io/n8n-docs) or follow-up ticket created. - [ ] Tests included. > A bug is not considered fixed, unless a test is added to prevent it from happening again. A feature is not complete without tests. > > *(internal)* You can use Slack commands to trigger [e2e tests](https://www.notion.so/n8n/How-to-use-Test-Instances-d65f49dfc51f441ea44367fb6f67eb0a?pvs=4#a39f9e5ba64a48b58a71d81c837e8227) or [deploy test instance](https://www.notion.so/n8n/How-to-use-Test-Instances-d65f49dfc51f441ea44367fb6f67eb0a?pvs=4#f6a177d32bde4b57ae2da0b8e454bfce) or [deploy early access version on Cloud](https://www.notion.so/n8n/Cloudbot-3dbe779836004972b7057bc989526998?pvs=4#fef2d36ab02247e1a0f65a74f6fb534e). --- packages/editor-ui/src/components/Node.vue | 16 +++++----------- packages/editor-ui/src/mixins/nodeBase.ts | 11 +++++++---- packages/editor-ui/src/views/NodeView.vue | 4 ++++ 3 files changed, 16 insertions(+), 15 deletions(-) diff --git a/packages/editor-ui/src/components/Node.vue b/packages/editor-ui/src/components/Node.vue index e1ff7b8c52f3b..de16549f0d238 100644 --- a/packages/editor-ui/src/components/Node.vue +++ b/packages/editor-ui/src/components/Node.vue @@ -286,15 +286,11 @@ export default defineComponent({ return this.data.type === MANUAL_TRIGGER_NODE_TYPE; }, isConfigNode(): boolean { - return this.nodeTypesStore.isConfigNode( - this.getCurrentWorkflow(), - this.data, - this.data?.type ?? '', - ); + return this.nodeTypesStore.isConfigNode(this.workflow, this.data, this.data?.type ?? ''); }, isConfigurableNode(): boolean { return this.nodeTypesStore.isConfigurableNode( - this.getCurrentWorkflow(), + this.workflow, this.data, this.data?.type ?? '', ); @@ -349,9 +345,8 @@ export default defineComponent({ }; if (this.node && this.nodeType) { - const workflow = this.workflowsStore.getCurrentWorkflow(); const inputs = - NodeHelpers.getNodeInputs(workflow, this.node, this.nodeType) || + NodeHelpers.getNodeInputs(this.workflow, this.node, this.nodeType) || ([] as Array); const inputTypes = NodeHelpers.getConnectionTypes(inputs); @@ -372,7 +367,7 @@ export default defineComponent({ } const outputs = - NodeHelpers.getNodeOutputs(workflow, this.node, this.nodeType) || + NodeHelpers.getNodeOutputs(this.workflow, this.node, this.nodeType) || ([] as Array); const outputTypes = NodeHelpers.getConnectionTypes(outputs); @@ -634,8 +629,7 @@ export default defineComponent({ // and ends up bogging down the UI with big workflows, for example when pasting a workflow or even opening a node... // so we only update it when necessary (when node is mounted and when it's opened and closed (isActive)) try { - const nodeSubtitle = - this.getNodeSubtitle(this.data, this.nodeType, this.getCurrentWorkflow()) || ''; + const nodeSubtitle = this.getNodeSubtitle(this.data, this.nodeType, this.workflow) || ''; this.nodeSubtitle = nodeSubtitle.includes(CUSTOM_API_CALL_KEY) ? '' : nodeSubtitle; } catch (e) { diff --git a/packages/editor-ui/src/mixins/nodeBase.ts b/packages/editor-ui/src/mixins/nodeBase.ts index 97a82ae31bb06..470a2ca167795 100644 --- a/packages/editor-ui/src/mixins/nodeBase.ts +++ b/packages/editor-ui/src/mixins/nodeBase.ts @@ -17,6 +17,7 @@ import type { INodeInputConfiguration, INodeTypeDescription, INodeOutputConfiguration, + Workflow, } from 'n8n-workflow'; import { useUIStore } from '@/stores/ui.store'; import { useWorkflowsStore } from '@/stores/workflows.store'; @@ -104,6 +105,10 @@ export const nodeBase = defineComponent({ showCustomTooltip: { type: Boolean, }, + workflow: { + type: Object as () => Workflow, + required: true, + }, }, methods: { __addEndpointTestingData(endpoint: Endpoint, type: string, inputIndex: number) { @@ -123,9 +128,8 @@ export const nodeBase = defineComponent({ [key: string]: number; } = {}; - const workflow = this.workflowsStore.getCurrentWorkflow(); const inputs: Array = - NodeHelpers.getNodeInputs(workflow, this.data!, nodeTypeData) || []; + NodeHelpers.getNodeInputs(this.workflow, this.data!, nodeTypeData) || []; this.inputs = inputs; const sortedInputs = [...inputs]; @@ -338,8 +342,7 @@ export const nodeBase = defineComponent({ [key: string]: number; } = {}; - const workflow = this.workflowsStore.getCurrentWorkflow(); - this.outputs = NodeHelpers.getNodeOutputs(workflow, this.data, nodeTypeData) || []; + this.outputs = NodeHelpers.getNodeOutputs(this.workflow, this.data, nodeTypeData) || []; // TODO: There are still a lot of references of "main" in NodesView and // other locations. So assume there will be more problems diff --git a/packages/editor-ui/src/views/NodeView.vue b/packages/editor-ui/src/views/NodeView.vue index 06e4186cc0ec9..3ef14e9ce25f4 100644 --- a/packages/editor-ui/src/views/NodeView.vue +++ b/packages/editor-ui/src/views/NodeView.vue @@ -58,6 +58,7 @@ :isActive="!!activeNode && activeNode.name === nodeData.name" :hideActions="pullConnActive" :isProductionExecutionPreview="isProductionExecutionPreview" + :workflow="currentWorkflowObject" >