Skip to content

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

Open
voidmatcha wants to merge 1 commit into
apache:masterfrom
voidmatcha:fix/clone-paragraph-cursor-focus-master
Open

[ZEPPELIN-6429] Focus paragraph editor on clone/insert in New UI#5267
voidmatcha wants to merge 1 commit 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
Copy link
Copy Markdown
Member

jongyoul commented Jun 6, 2026

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

@tbonelee
Copy link
Copy Markdown
Contributor

tbonelee commented Jun 7, 2026

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

Copy link
Copy Markdown
Contributor

@tbonelee tbonelee left a comment

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.

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