This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/texera.git
The following commit(s) were added to refs/heads/main by this push:
new 2db7db0c1e fix(frontend): suppress newline on Enter in title editors
(#8055)
2db7db0c1e is described below
commit 2db7db0c1e5ddf168cd89ef5875d2f91b84600a2
Author: Xinyuan Lin <[email protected]>
AuthorDate: Thu Sep 17 00:34:02 2026 +0000
fix(frontend): suppress newline on Enter in title editors (#8055)
### What changes were proposed in this PR?
The operator and port title editors bind Enter to "commit and close" so
that a
display name stays a single-line value. Since the move to Quill 2
(#6418) that
suppression has been dead: Enter inserted a literal newline into the
shared
`Y.Text`, which is published to every co-editor and persisted with the
workflow.
Root cause — Quill 2 resolves a keydown against two buckets, `event.key`
first:
```
bindings[evt.key] ++ bindings[evt.which] // "Enter" bucket, then 13 bucket
matches.some(handler) // first non-`true` return wins,
loop stops
```
Quill's own `handleEnter` is registered under `"Enter"`; the editors
registered
theirs under the legacy `13` keycode. So Quill's handler ran first,
inserted the
newline, and short-circuited the loop before the editors' handler was
reached.
The template's separate `(keyup.enter)` still closed the editor on key
*release*,
so the rename looked like it worked.
```
Before: Enter -> Quill handleEnter -> "\n" into shared text -> keyup
closes editor
After: Enter -> editor handler -> commit and close, text untouched
```
Keying both bindings `"Enter"` puts them in the same bucket, registered
ahead of
`handleEnter` (user options are added before Quill's built-ins).
| Rename typed | Enter pressed at | Stored name before | Stored name
after |
| --- | --- | --- | --- |
| `Sentiment Analysis` | end | `"Sentiment Analysis\n"` | `"Sentiment
Analysis"` |
| `Sentiment Analysis` | mid-word | `"Sen\ntiment Analysis"` |
`"Sentiment Analysis"` |
| *(untouched)* | start | `"\n"` | `""` |
No visual change: the editor renders identically before and after, which
is why
this went unnoticed — the defect is only in the value that gets stored.
`CollabWrapperComponent` carries the same `key: 13`, but it is
unreachable
(its call site is commented out) and #7351 deletes it, so it is left
alone here.
### Any related issues, documentation, discussions?
Closes #8053. Regression from #6418 (Quill 1 -> 2). Related: #7351.
### How was this PR tested?
New regression tests in both title editors' specs mount the real Quill
instance
and the real y-quill binding, open the editor through the template's
edit button,
and press Enter as a **keydown only** — the template's `(keyup.enter)`
fallback is
never dispatched, so it cannot mask a broken binding. Each asserts the
shared
`Y.Text` and `editingTitle`. Coverage: caret at the end, mid-word, an
empty name,
Shift+Enter, and an ordinary keystroke that must still reach the editor
(`defaultPrevented === false`, editor stays open).
The four positive tests fail on `main` and pass here; the negative one
passes both
ways, as a control:
```
AssertionError: expected 'renamed\n' to be 'renamed'
AssertionError: expected 'ren\named' to be 'renamed'
AssertionError: expected '\n' to be ''
```
```bash
cd frontend && yarn install --frozen-lockfile
```
```bash
cd frontend && npx ng test --include
"src/app/workspace/component/property-editor/**/*.spec.ts" --watch=false
```
`316 passed | 1 skipped (317)`. Full frontend suite: `204 files, 5213
passed`.
`yarn build` (production) succeeds. `npx prettier --check` and `npx
eslint` clean.
jsdom has no layout engine, so `Range#getBoundingClientRect` is stubbed
next to
the existing test-env polyfills — Quill calls it to place the caret and
quill-cursors calls it per remote cursor.
Manually verified in Chromium against Quill 2.0.3 with both binding
forms side by
side and a real Enter keystroke: `key: 13` stored `"Sentiment
Analysis\n"` and left
the editor open; `key: "Enter"` stored `"Sentiment Analysis"` and closed
it.
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code, Opus 5
---
.../operator-property-edit-frame.component.spec.ts | 163 +++++++++++++++++++++
.../operator-property-edit-frame.component.ts | 4 +-
.../port-property-edit-frame.component.spec.ts | 88 ++++++++++-
.../port-property-edit-frame.component.ts | 4 +-
frontend/src/jsdom-svg-polyfill.ts | 12 ++
5 files changed, 266 insertions(+), 5 deletions(-)
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 fa551d1402..9cf755d575 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
@@ -66,6 +66,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 });
@@ -3130,4 +3131,166 @@ describe("OperatorPropertyEditFrameComponent", () => {
expect(decorated).toEqual(["limit"]);
});
});
+
+ /**
+ * 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<OperatorPropertyEditFrameComponent>;
+ 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 033b75bc48..3fd8f1afcc 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
@@ -1511,11 +1511,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 d12b316d33..3d751dffec 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<unknown> {
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 3eeb4e3fc1..e8808c2279 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 7aef063d7d..881a9c5c0f 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
