Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(editor): Overhaul expression editor modal #4631

Merged
merged 37 commits into from
Dec 1, 2022
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
ec4c011
feat(editor): Integrate CodeMirror into expression editor modal (#4563)
ivov Nov 14, 2022
1b0797f
:twisted_rightwards_arrows: Merge master
ivov Nov 14, 2022
f9f2a2a
feat(editor): Adjust resolved expression to match parameter input hin…
ivov Nov 15, 2022
2a763ce
feat(editor): Apply styling to expression editor modal (#4607)
ivov Nov 15, 2022
6c09797
feat(core): Improve errors in evaluated expression (#4619)
ivov Nov 16, 2022
5e288b4
:twisted_rightwards_arrows: Merge master
ivov Nov 16, 2022
2405b33
feat(editor): Dynamically delay evaluation resolution (#4625)
ivov Nov 16, 2022
77d69da
refactor(editor): Pre-review cleanup (#4627)
ivov Nov 17, 2022
c743021
:twisted_rightwards_arrows: Merge master
ivov Nov 17, 2022
60bddf9
feat(editor): Improve escaping behavior (#4641)
ivov Nov 18, 2022
e79081a
refactor(editor): Apply styling feedback to expression editor modal (…
ivov Nov 21, 2022
5e183d6
:twisted_rightwards_arrows: Merge master
ivov Nov 21, 2022
2b76cee
:twisted_rightwards_arrows: Merge master again
ivov Nov 22, 2022
9de9179
:twisted_rightwards_arrows: Merge master
ivov Nov 23, 2022
add8b0a
refactor(editor): Apply feedback 2022.11.22 (#4697)
ivov Nov 23, 2022
80ed82b
refactor(editor): Apply feedback 2022.11.23 (#4705)
ivov Nov 23, 2022
fb3c06d
:twisted_rightwards_arrows: Merge master
ivov Nov 25, 2022
5db61bd
:zap: Fix import
ivov Nov 25, 2022
892e91e
:twisted_rightwards_arrows: Merge master
ivov Nov 25, 2022
c49593c
:test_tube: Add e2e tests
ivov Nov 25, 2022
95024be
:zap: Cleanup
ivov Nov 28, 2022
9994d0c
:zap: Add telemetry
ivov Nov 28, 2022
523d799
:fire: Remove log
ivov Nov 28, 2022
18b7d09
:zap: Expose error properties
ivov Nov 28, 2022
6a9dc58
:test_tube: Rename test
ivov Nov 30, 2022
7fe3d27
:zap: Move `getCurrentWorkflow()` call
ivov Nov 30, 2022
b164d19
:twisted_rightwards_arrows: Merge master
ivov Nov 30, 2022
66570b6
:rewind: Revert highlighting removal per feedback
ivov Nov 30, 2022
1005252
:twisted_rightwards_arrows: Merge master
ivov Nov 30, 2022
8562b50
:zap: Add i18n keys
ivov Nov 30, 2022
2e85c9c
:truck: Move computed property to local state
ivov Nov 30, 2022
0092a0d
:art: Use CSS vars
ivov Nov 30, 2022
2dbf032
:twisted_rightwards_arrows: Merge master
ivov Dec 1, 2022
3ac0a8a
:zap: Update `pnpm-lock.yaml`
ivov Dec 1, 2022
a19bdcb
:zap: Apply readonly state
ivov Dec 1, 2022
d0aed48
:zap: Use prop
ivov Dec 1, 2022
a28a816
:zap: Complete fix
ivov Dec 1, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 65 additions & 0 deletions cypress/e2e/9-expression-editor-modal.cy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import { WorkflowPage as WorkflowPageClass } from '../pages/workflow';

const WorkflowPage = new WorkflowPageClass();

describe('Expression editor modal', () => {
before(() => {
cy.task('db:reset');
cy.skipSetup();
});

beforeEach(() => {
WorkflowPage.actions.visit();
WorkflowPage.actions.addInitialNodeToCanvas('Manual Trigger');
WorkflowPage.actions.addNodeToCanvas('Hacker News');
WorkflowPage.actions.openNodeNdv('Hacker News');
WorkflowPage.actions.openExpressionEditor();
});

it('should resolve primitive resolvables', () => {
WorkflowPage.getters.expressionModalInput().type('{{');
WorkflowPage.getters.expressionModalInput().type('1 + 2');
WorkflowPage.getters.expressionModalOutput().contains(/^3$/);
WorkflowPage.getters.expressionModalInput().clear();

WorkflowPage.getters.expressionModalInput().type('{{');
WorkflowPage.getters.expressionModalInput().type('"ab" + "cd"');
WorkflowPage.getters.expressionModalOutput().contains(/^abcd$/);

WorkflowPage.getters.expressionModalInput().clear();

WorkflowPage.getters.expressionModalInput().type('{{');
WorkflowPage.getters.expressionModalInput().type('true && false');
WorkflowPage.getters.expressionModalOutput().contains(/^false$/);
});

it('should resolve object resolvables', () => {
WorkflowPage.getters.expressionModalInput().type('{{');
WorkflowPage.getters.expressionModalInput().type('{{} a: 1');
WorkflowPage.getters.expressionModalOutput().contains(/^\[Object: \{"a":1\}\]$/);

WorkflowPage.getters.expressionModalInput().clear();

WorkflowPage.getters.expressionModalInput().type('{{');
WorkflowPage.getters.expressionModalInput().type('{{} a: 1 }.a{del}{del}');
WorkflowPage.getters.expressionModalOutput().contains(/^1$/);
});

it('should resolve array resolvables', () => {
WorkflowPage.getters.expressionModalInput().type('{{');
WorkflowPage.getters.expressionModalInput().type('[1, 2, 3]');
WorkflowPage.getters.expressionModalOutput().contains(/^\[Array: \[1,2,3\]\]$/);

WorkflowPage.getters.expressionModalInput().clear();

WorkflowPage.getters.expressionModalInput().type('{{');
WorkflowPage.getters.expressionModalInput().type('[1, 2, 3][0]');
WorkflowPage.getters.expressionModalOutput().contains(/^1$/);
});

it('should resolve $parameter[]', () => {
WorkflowPage.getters.expressionModalInput().type('{{');
WorkflowPage.getters.expressionModalInput().type('$parameter["operation"]');
WorkflowPage.getters.expressionModalOutput().contains(/^get$/);
});
});
5 changes: 5 additions & 0 deletions cypress/pages/workflow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ export class WorkflowPage extends BasePage {
firstStepButton: () => cy.getByTestId('canvas-add-button'),
isWorkflowSaved: () => this.getters.saveButton().should('match', 'span'), // In Element UI, disabled button turn into spans 🤷‍♂️
isWorkflowActivated: () => this.getters.activatorSwitch().should('have.class', 'is-checked'),
expressionModalInput: () => cy.getByTestId('expression-modal-input'),
expressionModalOutput: () => cy.getByTestId('expression-modal-output'),
};
actions = {
visit: () => {
Expand All @@ -53,6 +55,9 @@ export class WorkflowPage extends BasePage {
openNodeNdv: (nodeTypeName: string) => {
this.getters.canvasNodeBox(nodeTypeName).dblclick();
},
openExpressionEditor: () => {
cy.get('input[value="expression"]').parent('label').click();
},
typeIntoParameterInput: (parameterName: string, content: string) => {
this.getters.ndvParameterInput(parameterName).type(content);
},
Expand Down
3 changes: 1 addition & 2 deletions docker/images/n8n-custom/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ FROM n8nio/base:${NODE_VERSION} as builder
COPY turbo.json package.json .npmrc pnpm-lock.yaml pnpm-workspace.yaml tsconfig.json ./
COPY scripts ./scripts
COPY packages ./packages
COPY patches ./patches

RUN corepack enable && corepack prepare --activate
RUN chown -R node:node .
Expand All @@ -17,7 +16,7 @@ RUN pnpm build
RUN rm -rf node_modules
RUN NODE_ENV=production pnpm install --prod --no-optional
RUN find . -type f -name "*.ts" -o -name "*.js.map" -o -name "*.vue" -o -name "tsconfig.json" -o -name "*.tsbuildinfo" | xargs rm
RUN rm -rf patches .npmrc *.yaml node_modules/.cache packages/**/node_modules/.cache packages/**/.turbo .config .cache .local .node /tmp/*
RUN rm -rf .npmrc *.yaml node_modules/.cache packages/**/node_modules/.cache packages/**/.turbo .config .cache .local .node /tmp/*


# 2. Start with a new clean image with just the code that is needed to run n8n
Expand Down
3 changes: 0 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,6 @@
"typescript": "^4.8.4"
},
"pnpm": {
"patchedDependencies": {
"[email protected]": "patches/[email protected]"
},
"onlyBuiltDependencies": [
"sqlite3",
"vue-demi"
Expand Down
2 changes: 2 additions & 0 deletions packages/editor-ui/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ module.exports = {
extraFileExtensions: ['.vue'],
},

ignorePatterns: ['*.d.cts'],

rules: {
// TODO: Remove these
'id-denylist': 'off',
Expand Down
7 changes: 2 additions & 5 deletions packages/editor-ui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@
"@codemirror/lang-javascript": "^6.0.2",
"@codemirror/language": "^6.2.1",
"@codemirror/lint": "^6.0.0",
"@codemirror/state": "^6.1.1",
"@codemirror/view": "^6.2.1",
"@codemirror/state": "^6.1.4",
"@codemirror/view": "^6.5.1",
"@fontsource/open-sans": "^4.5.0",
"@fortawesome/fontawesome-svg-core": "^1.2.35",
"@fortawesome/free-regular-svg-icons": "^6.1.1",
Expand Down Expand Up @@ -61,8 +61,6 @@
"normalize-wheel": "^1.0.1",
"pinia": "^2.0.22",
"prismjs": "^1.17.1",
"quill": "2.0.0-dev.4",
"quill-autoformat": "^0.1.1",
"timeago.js": "^4.0.2",
"uuid": "^8.3.2",
"v-click-outside": "^3.1.2",
Expand Down Expand Up @@ -92,7 +90,6 @@
"@types/lodash.get": "^4.4.6",
"@types/lodash.set": "^4.3.6",
"@types/luxon": "^2.0.9",
"@types/quill": "^2.0.1",
"@types/uuid": "^8.3.2",
"@vitejs/plugin-legacy": "^1.8.2",
"@vitejs/plugin-vue2": "^1.1.2",
Expand Down
95 changes: 87 additions & 8 deletions packages/editor-ui/src/components/ExpressionEdit.vue
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,42 @@
<el-col :span="16" class="right-side">
<div class="expression-editor-wrapper">
<div class="editor-description">
{{ $locale.baseText('expressionEdit.expression') }}
<div>
{{ $locale.baseText('expressionEdit.expression') }}
</div>
<div class="hint">
<span>
{{ $locale.baseText('expressionEdit.anythingInside') }}
</span>
<div class="expression-syntax-example" v-text="`{{ }}`"></div>
<span>
{{ $locale.baseText('expressionEdit.isJavaScript') }}
</span>
<n8n-link size="medium" :to="expressionsDocsUrl">
{{ $locale.baseText('expressionEdit.learnMore') }}
</n8n-link>
</div>
</div>
<div class="expression-editor ph-no-capture">
<expression-input :parameter="parameter" ref="inputFieldExpression" rows="8" :value="value" :path="path" @change="valueChanged" @keydown.stop="noOp"></expression-input>
<expression-modal-input
:value="value"
@change="valueChanged"
ref="inputFieldExpression"
data-test-id="expression-modal-input"
/>
</div>
</div>

<div class="expression-result-wrapper">
<div class="editor-description">
{{ $locale.baseText('expressionEdit.result') }}
{{ $locale.baseText('expressionEdit.resultOfItem1') }}
</div>
<div class="ph-no-capture">
<expression-input :parameter="parameter" resolvedValue="true" ref="expressionResult" rows="8" :value="displayValue" :path="path"></expression-input>
<expression-modal-output
:segments="segments"
ref="expressionResult"
data-test-id="expression-modal-output"
/>
</div>
</div>

Expand All @@ -43,21 +66,26 @@
</template>

<script lang="ts">
import ExpressionInput from '@/components/ExpressionInput.vue';
import ExpressionModalInput from '@/components/ExpressionEditorModal/ExpressionModalInput.vue';
import ExpressionModalOutput from '@/components/ExpressionEditorModal/ExpressionModalOutput.vue';
import VariableSelector from '@/components/VariableSelector.vue';

import { IVariableItemSelected } from '@/Interface';

import { externalHooks } from '@/mixins/externalHooks';
import { genericHelpers } from '@/mixins/genericHelpers';

import { EXPRESSIONS_DOCS_URL } from '@/constants';

import mixins from 'vue-typed-mixins';
import { hasExpressionMapping } from '@/utils';
import { debounceHelper } from '@/mixins/debounce';
import { mapStores } from 'pinia';
import { useWorkflowsStore } from '@/stores/workflows';
import { useNDVStore } from '@/stores/ndv';

import type { Resolvable, Segment } from './ExpressionEditorModal/types';

export default mixins(
externalHooks,
genericHelpers,
Expand All @@ -72,24 +100,30 @@ export default mixins(
'eventSource',
],
components: {
ExpressionInput,
ExpressionModalInput,
ExpressionModalOutput,
VariableSelector,
},
data () {
return {
displayValue: '',
latestValue: '',
segments: [] as Segment[],
};
},
computed: {
...mapStores(
useNDVStore,
useWorkflowsStore,
),
expressionsDocsUrl(): string {
return EXPRESSIONS_DOCS_URL;
},
ivov marked this conversation as resolved.
Show resolved Hide resolved
},
methods: {
valueChanged (value: string, forceUpdate = false) {
valueChanged ({ value, segments }: { value: string, segments: Segment[] }, forceUpdate = false) {
this.latestValue = value;
this.segments = segments;

if (forceUpdate === true) {
this.updateDisplayValue();
Expand Down Expand Up @@ -180,14 +214,34 @@ export default mixins(
this.$externalHooks().run('expressionEdit.dialogVisibleChanged', { dialogVisible: newValue, parameter: this.parameter, value: this.value, resolvedExpressionValue });

if (!newValue) {
const resolvables = this.segments.filter((s): s is Resolvable => s.kind === 'resolvable');
const errorResolvables = resolvables.filter(r => r.error);

const exposeErrorProperties = (error: Error) => {
return Object.getOwnPropertyNames(error).reduce<Record<string, unknown>>((acc, key) => {
// @ts-ignore
return acc[key] = error[key], acc;
}, {});
};

const telemetryPayload = {
empty_expression: (this.value === '=') || (this.value === '={{}}') || !this.value,
workflow_id: this.workflowsStore.workflowId,
source: this.eventSource,
session_id: this.ndvStore.sessionId,
has_parameter: this.value.includes('$parameter'),
has_mapping: hasExpressionMapping(this.value),
node_type: this.ndvStore.activeNode?.type ?? '',
handlebar_count: resolvables.length,
handlebar_error_count: errorResolvables.length,
full_errors: errorResolvables.map(errorResolvable => {
return errorResolvable.fullError
? { ...exposeErrorProperties(errorResolvable.fullError), stack: errorResolvable.fullError.stack }
: null;
}),
short_errors: errorResolvables.map(r => r.resolved ?? null),
};

this.$telemetry.track('User closed Expression Editor', telemetryPayload);
this.$externalHooks().run('expressionEdit.closeDialog', telemetryPayload);
}
Expand All @@ -200,7 +254,32 @@ export default mixins(
.editor-description {
line-height: 1.5;
font-weight: bold;
padding: 0 0 0.5em 0.2em;;
padding: 0 0 0.5em 0.2em;
display: flex;
justify-content: space-between;

.hint {
color: var(--color-text-base);
font-weight: normal;
display: flex;

@media (max-width: $breakpoint-xs) {
display: none;
}

span {
margin-right: var(--spacing-4xs);
}
.expression-syntax-example {
display: inline-block;
margin-top: 3px;
height: 16px;
line-height: 1;
background-color: #f0f0f0;
ivov marked this conversation as resolved.
Show resolved Hide resolved
color: var(--color-text-dark);
margin-right: var(--spacing-4xs);
}
}
}

.expression-result-wrapper,
Expand Down
Loading