fix(frontend): suppress newline on Enter in title editors - #8055
Open
aglinxinyuan wants to merge 1 commit into
Open
fix(frontend): suppress newline on Enter in title editors#8055aglinxinyuan wants to merge 1 commit into
aglinxinyuan wants to merge 1 commit into
Conversation
The operator and port title editors bind Enter to "commit and close" so a display name stays single-line. Quill 2 buckets keyboard bindings by `event.key` and registers its own `handleEnter` under "Enter", while the editors registered theirs under the legacy `13` keycode. Quill's handler therefore ran first and short-circuited the match loop, inserting a newline into the shared Y.Text and never reaching the editors' handler. Only the template's `(keyup.enter)` still closed the editor, so the rename looked like it worked while a "\n" was published to every co-editor and persisted. Key the bindings "Enter" so they resolve into the same bucket ahead of `handleEnter`. Regression tests mount the real editor and press Enter on keydown only, so the template fallback cannot mask a broken binding; they cover a caret at the end, mid-word, an empty name, Shift+Enter, and an ordinary keystroke that must still reach the editor. jsdom has no layout engine, so `Range#getBoundingClientRect` is stubbed alongside the existing test-env polyfills — Quill calls it to place the caret. Closes apache#8053
Contributor
Automated Reviewer SuggestionsBased on the
|
Contributor
Backport auto-label reportThis
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8055 +/- ##
============================================
+ Coverage 93.41% 93.44% +0.02%
Complexity 4696 4696
============================================
Files 1179 1179
Lines 47713 47713
Branches 5314 5314
============================================
+ Hits 44573 44586 +13
+ Misses 1698 1682 -16
- Partials 1442 1445 +3
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.keyfirst:Quill's own
handleEnteris registered under"Enter"; the editors registeredtheirs under the legacy
13keycode. So Quill's handler ran first, inserted thenewline, 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.
Keying both bindings
"Enter"puts them in the same bucket, registered ahead ofhandleEnter(user options are added before Quill's built-ins).Sentiment Analysis"Sentiment Analysis\n""Sentiment Analysis"Sentiment Analysis"Sen\ntiment Analysis""Sentiment Analysis""\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.
CollabWrapperComponentcarries the samekey: 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 isnever dispatched, so it cannot mask a broken binding. Each asserts the shared
Y.TextandeditingTitle. 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
mainand pass here; the negative one passes bothways, as a control:
316 passed | 1 skipped (317). Full frontend suite:204 files, 5213 passed.yarn build(production) succeeds.npx prettier --checkandnpx eslintclean.jsdom has no layout engine, so
Range#getBoundingClientRectis stubbed next tothe 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: 13stored"Sentiment Analysis\n"and leftthe 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