fix(editor): Fields reset after closing NDV when side panel NDV is also visible (no-changelog) (#18857)

This commit is contained in:
Suguru Inoue
2025-09-01 10:10:10 +02:00
committed by GitHub
parent 61f79319df
commit 280dd013ba
6 changed files with 73 additions and 28 deletions

View File

@@ -145,6 +145,7 @@ function onAskAssistantButtonClick() {
icon="panel-right" icon="panel-right"
:class="focusPanelActive ? $style.activeButton : ''" :class="focusPanelActive ? $style.activeButton : ''"
:active="focusPanelActive" :active="focusPanelActive"
data-test-id="toggle-focus-panel-button"
@click="toggleFocusPanel" @click="toggleFocusPanel"
/> />
</KeyboardShortcutTooltip> </KeyboardShortcutTooltip>

View File

@@ -201,6 +201,7 @@ const dateTimePickerOptions = ref({
], ],
}); });
const isFocused = ref(false); const isFocused = ref(false);
const isMapperShown = ref(false);
const contextNode = expressionLocalResolveCtx?.value?.workflow.getNode( const contextNode = expressionLocalResolveCtx?.value?.workflow.getNode(
expressionLocalResolveCtx.value.nodeName, expressionLocalResolveCtx.value.nodeName,
@@ -611,11 +612,7 @@ const showDragnDropTip = computed(
const shouldCaptureForPosthog = computed(() => node.value?.type === AI_TRANSFORM_NODE_TYPE); const shouldCaptureForPosthog = computed(() => node.value?.type === AI_TRANSFORM_NODE_TYPE);
const shouldShowMapper = computed( const mapperElRef = computed(() => mapperRef.value?.contentRef);
() =>
isFocused.value &&
(isModelValueExpression.value || props.forceShowExpression || props.modelValue === ''),
);
function isRemoteParameterOption(option: INodePropertyOptions) { function isRemoteParameterOption(option: INodePropertyOptions) {
return remoteParameterOptionsKeys.value.includes(option.name); return remoteParameterOptionsKeys.value.includes(option.name);
@@ -802,6 +799,10 @@ async function setFocus() {
} }
isFocused.value = true; isFocused.value = true;
if (isModelValueExpression.value || props.forceShowExpression || props.modelValue === '') {
isMapperShown.value = true;
}
} }
emit('focus'); emit('focus');
@@ -942,17 +943,19 @@ function expressionUpdated(value: string) {
} }
function onBlur(event?: FocusEvent | KeyboardEvent) { function onBlur(event?: FocusEvent | KeyboardEvent) {
if (
event?.target instanceof HTMLElement &&
mapperRef.value?.contentRef &&
(event.target === mapperRef.value.contentRef ||
mapperRef.value.contentRef.contains(event.target))
) {
return;
}
emit('blur'); emit('blur');
isFocused.value = false; isFocused.value = false;
if (
!(event?.target instanceof Node && mapperElRef.value?.contains(event.target)) &&
!(
event instanceof FocusEvent &&
event.relatedTarget instanceof Node &&
mapperElRef.value?.contains(event.relatedTarget)
)
) {
isMapperShown.value = false;
}
} }
function onPaste(event: ClipboardEvent) { function onPaste(event: ClipboardEvent) {
@@ -998,6 +1001,12 @@ function onUpdateTextInput(value: string) {
onTextInputChange(value); onTextInputChange(value);
} }
function onClickOutsideMapper() {
if (!isFocused.value) {
isMapperShown.value = false;
}
}
async function optionSelected(command: string) { async function optionSelected(command: string) {
const prevValue = props.modelValue; const prevValue = props.modelValue;
@@ -1012,6 +1021,7 @@ async function optionSelected(command: string) {
case 'removeExpression': case 'removeExpression':
isFocused.value = false; isFocused.value = false;
isMapperShown.value = false;
valueChanged( valueChanged(
parseFromExpression( parseFromExpression(
props.modelValue, props.modelValue,
@@ -1210,7 +1220,7 @@ onUpdated(async () => {
} }
}); });
onClickOutside(wrapper, onBlur); onClickOutside(mapperElRef, onClickOutsideMapper);
</script> </script>
<template> <template>
@@ -1239,7 +1249,7 @@ onClickOutside(wrapper, onBlur);
:workflow="expressionLocalResolveCtx?.workflow" :workflow="expressionLocalResolveCtx?.workflow"
:node="node" :node="node"
:input-node-name="expressionLocalResolveCtx?.inputNode?.name" :input-node-name="expressionLocalResolveCtx?.inputNode?.name"
:visible="shouldShowMapper" :visible="isMapperShown"
:virtual-ref="wrapper" :virtual-ref="wrapper"
/> />

View File

@@ -360,6 +360,10 @@ export class CanvasPage extends BasePage {
return this.page.getByTestId('canvas-wrapper'); return this.page.getByTestId('canvas-wrapper');
} }
toggleFocusPanelButton(): Locator {
return this.page.getByTestId('toggle-focus-panel-button');
}
// Actions // Actions
async addInitialNodeToCanvas(nodeName: string): Promise<void> { async addInitialNodeToCanvas(nodeName: string): Promise<void> {

View File

@@ -20,7 +20,7 @@ export class NodeDetailsViewPage extends BasePage {
} }
getParameterByLabel(labelName: string) { getParameterByLabel(labelName: string) {
return this.page.locator('.parameter-item').filter({ hasText: labelName }); return this.getContainer().locator('.parameter-item').filter({ hasText: labelName });
} }
/** /**

View File

@@ -0,0 +1,30 @@
import { test, expect } from '../../fixtures/base';
import type { TestRequirements } from '../../Types';
const requirements: TestRequirements = {
workflow: {
'Test_workflow_1.json': 'Test',
},
storage: {
N8N_EXPERIMENT_OVERRIDES: JSON.stringify({ ndv_in_focus_panel: 'variant' }),
},
};
test.describe('SUG-121 Fields reset after closing NDV', () => {
test('should preserve changes to parameters after closing NDV when focus panel is open', async ({
n8n,
setupRequirements,
}) => {
await setupRequirements(requirements);
await n8n.canvas.clickZoomToFitButton();
await n8n.canvas.toggleFocusPanelButton().click();
await n8n.canvas.canvasPane().click();
await n8n.canvas.nodeByName('Code').dblclick();
await n8n.ndv.getParameterByLabel('JavaScript').getByRole('textbox').fill('alert(1)');
await n8n.ndv.close();
await n8n.canvas.nodeByName('Code').dblclick();
await expect(n8n.ndv.getParameterByLabel('JavaScript').getByRole('textbox')).toHaveText(
'alert(1)',
);
});
});

View File

@@ -10,8 +10,18 @@ export async function setupTestRequirements(
context: BrowserContext, context: BrowserContext,
requirements: TestRequirements, requirements: TestRequirements,
): Promise<void> { ): Promise<void> {
const n8n = new n8nPage(page); // 0. Setup browser storage before creating a new page
if (requirements.storage) {
await context.addInitScript((storage) => {
// Set localStorage items
for (const [key, value] of Object.entries(storage)) {
window.localStorage.setItem(key, value);
}
}, requirements.storage);
}
const api = new ApiHelpers(context.request); const api = new ApiHelpers(context.request);
const n8n = new n8nPage(page, api);
// 1. Setup frontend settings override // 1. Setup frontend settings override
if (requirements.config?.settings) { if (requirements.config?.settings) {
@@ -57,14 +67,4 @@ export async function setupTestRequirements(
} }
} }
} }
// 5. Setup browser storage
if (requirements.storage) {
await context.addInitScript((storage) => {
// Set localStorage items
for (const [key, value] of Object.entries(storage)) {
window.localStorage.setItem(key, value);
}
}, requirements.storage);
}
} }