diff --git a/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.spec.ts b/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.spec.ts index daea90d2d66..3e8d10cb1e2 100644 --- a/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.spec.ts +++ b/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.spec.ts @@ -68,6 +68,7 @@ import { of, Subject, throwError } from "rxjs"; import { WorkflowVersionService } from "../../../../dashboard/service/user/workflow-version/workflow-version.service"; import { GuiConfigService } from "../../../../common/service/gui-config.service"; import { PresetWrapperComponent } from "src/app/common/formly/preset-wrapper/preset-wrapper.component"; +import * as Y from "yjs"; const { marbles } = configure({ run: false }); @@ -2899,4 +2900,166 @@ describe("OperatorPropertyEditFrameComponent", () => { ); }); }); + + /** + * The spec's default TestBed swaps the template for a stub, so the collaborative title editor — + * the Quill instance the component mounts on `#customName` and the keyboard bindings it configures + * — is never built by anything above. This block renders the real template and drives that editor. + * It comes last in the file deliberately: Quill 2 has no `destroy()`, so each instance leaves a + * MutationObserver and document listeners behind for `fixture.destroy()` to detach, and it must not + * be combined with `fakeAsync` (zone.js patches MutationObserver). + */ + describe("quill title editing", () => { + let quillFixture: ComponentFixture; + let quillComponent: OperatorPropertyEditFrameComponent; + let texeraGraph: WorkflowGraph; + + beforeEach(async () => { + TestBed.resetTestingModule(); + await TestBed.configureTestingModule({ + providers: [ + WorkflowActionService, + { provide: OperatorMetadataService, useClass: StubOperatorMetadataService }, + { provide: ComputingUnitStatusService, useClass: MockComputingUnitStatusService }, + DatePipe, + ...commonTestProviders, + ], + imports: [ + OperatorPropertyEditFrameComponent, + BrowserAnimationsModule, + FormsModule, + FormlyModule.forRoot(TEXERA_FORMLY_CONFIG), + FormlyNgZorroAntdModule, + ReactiveFormsModule, + HttpClientTestingModule, + ], + }).compileComponents(); + + quillFixture = TestBed.createComponent(OperatorPropertyEditFrameComponent); + quillComponent = quillFixture.componentInstance; + // Downcast to the concrete graph: `getSharedOperatorType` is not on the readonly view. + texeraGraph = TestBed.inject(WorkflowActionService).getTexeraGraph() as WorkflowGraph; + quillFixture.detectChanges(); + }); + + afterEach(() => { + vi.restoreAllMocks(); + quillFixture.destroy(); + }); + + /** Opens the editor the only way a user can — the title section's edit button. */ + function openEditorFromButton(): Y.Text { + const sharedOperator = new Y.Doc().getMap("operator"); + quillComponent.currentOperatorId = mockScanPredicate.operatorID; + // `interactive` defaults to false, which renders the edit button disabled — a disabled button + // swallows the click silently and the editor would never mount. + quillComponent.interactive = true; + vi.spyOn(texeraGraph, "getSharedOperatorType").mockReturnValue(sharedOperator as any); + quillFixture.detectChanges(); + + quillFixture.debugElement.query(By.css("#formly-title button")).nativeElement.click(); + quillFixture.detectChanges(); + + expect(quillComponent.editingTitle).toBe(true); + expect(quillComponent.quillBinding).toBeDefined(); + // The map genuinely lacked the key, so this Y.Text can only have come from the connect call. + const sharedTitle = sharedOperator.get("customDisplayName"); + expect(sharedTitle).toBeInstanceOf(Y.Text); + return sharedTitle as Y.Text; + } + + /** + * An operator display name is a single-line value, so the editor binds Enter to "commit and + * close" instead of letting Quill insert a newline. Everything typed here goes straight into the + * shared Y.Text, so a stray "\n" is published to every co-editor, persisted with the workflow, + * and read back on each later open. Quill 2 buckets keyboard bindings by `event.key` and runs its + * built-in `handleEnter` ahead of anything registered under the legacy `13` keycode, which is how + * the suppression stopped working when the frontend moved to Quill 2 (#8053). The template's + * `(keyup.enter)` closes the editor either way — a keydown-only press is what separates a working + * binding from a broken one, in both the text left behind and `editingTitle`. + */ + const ENTER: KeyboardEventInit = { key: "Enter", keyCode: 13, which: 13 }; + + /** + * Quill's keydown listener returns early unless the editor `hasFocus()` and reports a selection, + * so both have to be real here — without them no binding runs at all and every assertion below + * would pass vacuously. `keyCode` / `which` mirror what a browser sends: Quill matches a binding + * on either, and jsdom's default of 0 would make a `13`-keyed binding unreachable rather than + * merely out-prioritised, hiding the very defect these tests pin. + */ + function pressKey(caret: number, init: KeyboardEventInit): KeyboardEvent { + quillComponent.quill.root.focus(); + quillComponent.quill.setSelection(caret, 0); + const keyPress = new KeyboardEvent("keydown", { bubbles: true, cancelable: true, ...init }); + quillComponent.quill.root.dispatchEvent(keyPress); + quillFixture.detectChanges(); + return keyPress; + } + + it("should show a remote rename in the mounted editor", () => { + const sharedTitle = openEditorFromButton(); + + sharedTitle.insert(0, "renamed remotely"); + + // Without this the keyboard tests below could pass against an editor wired to nothing. + expect((document.getElementById("customName") as HTMLElement).textContent).toContain("renamed remotely"); + }); + + it("should commit the rename on Enter without appending a newline to the shared name", () => { + const sharedTitle = openEditorFromButton(); + sharedTitle.insert(0, "renamed"); + + pressKey(sharedTitle.length, ENTER); + + expect(sharedTitle.toString()).toBe("renamed"); + // `editingTitle` is what proves the editor's own binding ran: no keyup was dispatched, so the + // template's fallback cannot be the thing that closed the editor. + expect(quillComponent.editingTitle).toBe(false); + expect(quillComponent.quillBinding).toBeUndefined(); + }); + + it("should not split the shared name when Enter is pressed mid-word", () => { + const sharedTitle = openEditorFromButton(); + sharedTitle.insert(0, "renamed"); + + pressKey(3, ENTER); + + expect(sharedTitle.toString()).toBe("renamed"); + expect(quillComponent.editingTitle).toBe(false); + }); + + it("should leave an untouched name empty when Enter commits it", () => { + const sharedTitle = openEditorFromButton(); + + pressKey(0, ENTER); + + // The empty document is the case Quill's own handler turns into a lone "\n" — a display name + // that reads as blank but is no longer equal to the empty string it started as. + expect(sharedTitle.toString()).toBe(""); + expect(quillComponent.editingTitle).toBe(false); + }); + + it("should commit on Shift+Enter without appending a newline either", () => { + const sharedTitle = openEditorFromButton(); + sharedTitle.insert(0, "renamed"); + + pressKey(sharedTitle.length, { ...ENTER, shiftKey: true }); + + expect(sharedTitle.toString()).toBe("renamed"); + expect(quillComponent.editingTitle).toBe(false); + }); + + it("should leave ordinary typing alone", () => { + const sharedTitle = openEditorFromButton(); + sharedTitle.insert(0, "renamed"); + + const keyPress = pressKey(sharedTitle.length, { key: "a", keyCode: 65, which: 65 }); + + // Suppressing Enter must not become suppressing the keyboard: an ordinary character is left + // for the browser to insert, and the editor stays open so the rename can continue. + expect(keyPress.defaultPrevented).toBe(false); + expect(quillComponent.editingTitle).toBe(true); + expect(quillComponent.quillBinding).toBeDefined(); + }); + }); }); diff --git a/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.ts b/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.ts index beedbabd90f..9c8287b8a86 100644 --- a/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.ts +++ b/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.ts @@ -1415,11 +1415,11 @@ export class OperatorPropertyEditFrameComponent implements OnInit, OnChanges, On keyboard: { bindings: { enter: { - key: 13, + key: "Enter", handler: () => this.disconnectQuillFromText(), }, shift_enter: { - key: 13, + key: "Enter", shiftKey: true, handler: () => this.disconnectQuillFromText(), }, diff --git a/frontend/src/app/workspace/component/property-editor/port-property-edit-frame/port-property-edit-frame.component.spec.ts b/frontend/src/app/workspace/component/property-editor/port-property-edit-frame/port-property-edit-frame.component.spec.ts index d12b316d334..3d751dffec7 100644 --- a/frontend/src/app/workspace/component/property-editor/port-property-edit-frame/port-property-edit-frame.component.spec.ts +++ b/frontend/src/app/workspace/component/property-editor/port-property-edit-frame/port-property-edit-frame.component.spec.ts @@ -512,7 +512,7 @@ describe("PortPropertyEditFrameComponent", () => { * `(focusout)` bindings — unexercised. Without these the whole feature can be unwired from the * UI without a single test noticing. */ - function openEditorFromButton(): void { + function openEditorFromButton(): Y.Map { const sharedPortDescription = new Y.Doc().getMap("portDescription"); component.currentPortID = inputPort; vi.spyOn(texeraGraph, "getSharedPortDescriptionType").mockReturnValue(sharedPortDescription); @@ -522,6 +522,7 @@ describe("PortPropertyEditFrameComponent", () => { expect(component.editingTitle).toBe(true); expect(component.quillBinding).toBeDefined(); + return sharedPortDescription; } it("should open the collaborative editor from the edit button and close it on Enter", () => { @@ -543,5 +544,90 @@ describe("PortPropertyEditFrameComponent", () => { expect(component.editingTitle).toBe(false); expect(component.quillBinding).toBeUndefined(); }); + + /** + * A port display name is a single-line value, so the editor binds Enter to "commit and close" + * instead of letting Quill insert a newline. Everything typed here goes straight into the shared + * Y.Text, so a stray "\n" is published to every co-editor, persisted with the workflow, and read + * back on each later open. Quill 2 buckets keyboard bindings by `event.key` and runs its built-in + * `handleEnter` ahead of anything registered under the legacy `13` keycode, which is how the + * suppression stopped working when the frontend moved to Quill 2 (#8053). The template's + * `(keyup.enter)` closes the editor either way — a keydown-only press is what separates a working + * binding from a broken one, in both the text left behind and `editingTitle`. + */ + const ENTER: KeyboardEventInit = { key: "Enter", keyCode: 13, which: 13 }; + + /** + * Quill's keydown listener returns early unless the editor `hasFocus()` and reports a selection, + * so both have to be real here — without them no binding runs at all and every assertion below + * would pass vacuously. `keyCode` / `which` mirror what a browser sends: Quill matches a binding + * on either, and jsdom's default of 0 would make a `13`-keyed binding unreachable rather than + * merely out-prioritised, hiding the very defect these tests pin. + */ + function pressKey(caret: number, init: KeyboardEventInit): KeyboardEvent { + component.quill.root.focus(); + component.quill.setSelection(caret, 0); + const keyPress = new KeyboardEvent("keydown", { bubbles: true, cancelable: true, ...init }); + component.quill.root.dispatchEvent(keyPress); + fixture.detectChanges(); + return keyPress; + } + + it("should commit the rename on Enter without appending a newline to the shared name", () => { + const sharedTitle = openEditorFromButton().get("displayName") as Y.Text; + sharedTitle.insert(0, "renamed"); + + pressKey(sharedTitle.length, ENTER); + + expect(sharedTitle.toString()).toBe("renamed"); + // `editingTitle` is what proves the editor's own binding ran: no keyup was dispatched, so the + // template's fallback cannot be the thing that closed the editor. + expect(component.editingTitle).toBe(false); + expect(component.quillBinding).toBeUndefined(); + }); + + it("should not split the shared name when Enter is pressed mid-word", () => { + const sharedTitle = openEditorFromButton().get("displayName") as Y.Text; + sharedTitle.insert(0, "renamed"); + + pressKey(3, ENTER); + + expect(sharedTitle.toString()).toBe("renamed"); + expect(component.editingTitle).toBe(false); + }); + + it("should leave an untouched name empty when Enter commits it", () => { + const sharedTitle = openEditorFromButton().get("displayName") as Y.Text; + + pressKey(0, ENTER); + + // The empty document is the case Quill's own handler turns into a lone "\n" — a display name + // that reads as blank but is no longer equal to the empty string it started as. + expect(sharedTitle.toString()).toBe(""); + expect(component.editingTitle).toBe(false); + }); + + it("should commit on Shift+Enter without appending a newline either", () => { + const sharedTitle = openEditorFromButton().get("displayName") as Y.Text; + sharedTitle.insert(0, "renamed"); + + pressKey(sharedTitle.length, { ...ENTER, shiftKey: true }); + + expect(sharedTitle.toString()).toBe("renamed"); + expect(component.editingTitle).toBe(false); + }); + + it("should leave ordinary typing alone", () => { + const sharedTitle = openEditorFromButton().get("displayName") as Y.Text; + sharedTitle.insert(0, "renamed"); + + const keyPress = pressKey(sharedTitle.length, { key: "a", keyCode: 65, which: 65 }); + + // Suppressing Enter must not become suppressing the keyboard: an ordinary character is left + // for the browser to insert, and the editor stays open so the rename can continue. + expect(keyPress.defaultPrevented).toBe(false); + expect(component.editingTitle).toBe(true); + expect(component.quillBinding).toBeDefined(); + }); }); }); diff --git a/frontend/src/app/workspace/component/property-editor/port-property-edit-frame/port-property-edit-frame.component.ts b/frontend/src/app/workspace/component/property-editor/port-property-edit-frame/port-property-edit-frame.component.ts index 3eeb4e3fc1c..e8808c2279b 100644 --- a/frontend/src/app/workspace/component/property-editor/port-property-edit-frame/port-property-edit-frame.component.ts +++ b/frontend/src/app/workspace/component/property-editor/port-property-edit-frame/port-property-edit-frame.component.ts @@ -248,11 +248,11 @@ export class PortPropertyEditFrameComponent implements OnInit, OnChanges { keyboard: { bindings: { enter: { - key: 13, + key: "Enter", handler: () => this.disconnectQuillFromText(), }, shift_enter: { - key: 13, + key: "Enter", shiftKey: true, handler: () => this.disconnectQuillFromText(), }, diff --git a/frontend/src/jsdom-svg-polyfill.ts b/frontend/src/jsdom-svg-polyfill.ts index 1fe62e21477..9aae982cc20 100644 --- a/frontend/src/jsdom-svg-polyfill.ts +++ b/frontend/src/jsdom-svg-polyfill.ts @@ -111,6 +111,18 @@ installIfMissing(G.SVGGraphicsElement?.prototype, { getBBox: fakeRect as AnyFn, }); +// `Range.prototype.getBoundingClientRect` — jsdom has no layout engine and +// doesn't implement it. Quill's `Selection#getBounds` calls it on a live +// Range to place the caret, and quill-cursors calls it again for every +// remote cursor an awareness update carries, so any spec that mounts a +// collaborative editor throws `range.getBoundingClientRect is not a +// function`. A zeroed rect is enough — jsdom never paints, and specs that +// need real caret geometry belong in browser mode. `top` / `left` / `right` +// / `height` are the four fields `getBounds` reads back off it. +installIfMissing(G.Range?.prototype, { + getBoundingClientRect: (() => ({ ...fakeRect(), top: 0, right: 0, bottom: 0, left: 0 })) as AnyFn, +}); + // Constructable Stylesheets API (`new CSSStyleSheet().replaceSync(...)`) — // jsdom doesn't ship it, but @codingame/monaco-vscode-api v25 calls it at // module load. Stub with an inert constructor; specs don't visually render