Skip to content

[ZEPPELIN-6429] Focus paragraph editor on clone/insert in New UI#5267

Merged
tbonelee merged 3 commits into
apache:masterfrom
voidmatcha:fix/clone-paragraph-cursor-focus-master
Jun 8, 2026
Merged

[ZEPPELIN-6429] Focus paragraph editor on clone/insert in New UI#5267
tbonelee merged 3 commits into
apache:masterfrom
voidmatcha:fix/clone-paragraph-cursor-focus-master

Conversation

@voidmatcha

Copy link
Copy Markdown
Contributor

What is this PR for?

After cloning or inserting a paragraph in the New UI, the cursor stays on the wrapper element instead of the editor, so you have to click before typing. This focuses the new paragraph's editor one tick after PARAGRAPH_ADDED, gated to clone/insert initiated by this client so auto-append on run and other clients' inserts don't steal focus. It also skips dirty-marking on programmatic editor setValue (isFlush) so the cloned content isn't discarded. (The clone content loss itself is handled separately in #5254 (review); this covers the cursor part.)

What type of PR is it?

Bug Fix

Todos

What is the Jira issue?

ZEPPELIN-6429

How should this be tested?

Screenshots (if appropriate)

M0_COMPARISON_master_asis-vs-tobe.mp4

Questions:

  • Does the license files need to update? No
  • Is there breaking changes for older versions? No
  • Does this needs documentation? No

@jongyoul

jongyoul commented Jun 6, 2026

Copy link
Copy Markdown
Member

@voidmatcha By the way, is the CI failure irrelevant?

@tbonelee

tbonelee commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

@jongyoul The job failed is only relevant to the legacy frontend app, so we could ignore it for here.

jongyoul
jongyoul previously approved these changes Jun 7, 2026

@tbonelee tbonelee left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this. It works as intended, I just have a suggestion about the approach.

localAddFocusPending (message.service.ts:43) assumes the next PARAGRAPH_ADDED is the one caused by this client's own request. That's fine for a single user, but it can break with more than one client on the same note. For example: A clicks clone (flag set to true), and before A's own response returns, a PARAGRAPH_ADDED for a paragraph that user B just added arrives first. A's flag gets consumed by B's paragraph, so A's cursor lands on a paragraph A never created. The root issue is that the client can't tell whether an incoming PARAGRAPH_ADDED came from its own request, so it has to guess.

The msgId we already pass around solves this cleanly. The client sends a msgId on every request (message.ts:165), the server knows it (Message.java:234-239), and several broadcasts already echo it back (e.g. broadcastParagraph, NotebookServer.java:673-674). The only gap is broadcastNewParagraph (NotebookServer.java:695-701), so it's a small change, the same thing broadcastParagraph already does:

// NotebookServer.java:695: take a msgId and pass it through (same as broadcastParagraph)
Message message = new Message(OP.PARAGRAPH_ADDED)
  .withMsgId(msgId)                       // <-- this line
  .put("paragraph", para).put("index", paraIndex);

The server then only reports which request an event came from, and each client decides for itself whether to focus by checking if the msgId is its own:

// focus only if the incoming PARAGRAPH_ADDED's msgId is one this client sent
// (the msgId contains a client id, so match by prefix, or track the msgIds you send)
if (isOwnMessage(msgId)) {
  // focus the new paragraph's editor
}

So before going with the heuristic, what do you think about doing it this way instead? It already works today, so this is really about the approach, and I'm happy to help with it.

Comment thread zeppelin-web-angular/src/app/services/message.service.ts
@tbonelee tbonelee merged commit e1712ec into apache:master Jun 8, 2026
19 checks passed
@tbonelee

tbonelee commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Merged into master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants