[ZEPPELIN-6294] Made "Clone paragraph" button work properly in New UI

[dididy]

What is this PR for?

Description: Clicking the paragraph control’s Clone paragraph button that trigger COPY_PARAGRAPH. Upon receiving PARAGRAPH_ADDED, the Classic UI includes data.paragraph.text as expected, but the New UI receives an empty data.paragraph.text. Additionally, after cloning, the cursor is not positioned in the cloned paragraph in the New UI.

Expected:

• Cloned paragraph contains the original text.
• Cursor focuses/positions inside the cloned paragraph.

Actual (New UI):

• data.paragraph.text is empty on PARAGRAPH_ADDED.
• Cursor not focused / not moved to the cloned paragraph.

[Appropriate action - Classic UI]

https://github.com/user-attachments/assets/dd96ceaa-b0f1-4b1e-ad11-46560a0e2b79

[AS-IS]

https://github.com/user-attachments/assets/39530005-b169-4301-9416-ed14cb6888fc

[TO-BE]

https://github.com/user-attachments/assets/81302edb-0d52-49e9-a554-e2509a7bca18

What type of PR is it?

Bug Fix

Todos

What is the Jira issue?

• [ZEPPELIN-6294]

How should this be tested?

Screenshots (if appropriate)

Questions:

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

[dididy]

I really appreciate about your review. 👍 👍

As you mentioned, the issue was caused by EDITOR_SETTING being triggered before PARAGRAPH, which led to short-circuit processing. However, modifying the condition in message.ts, L187 didn’t seem optimal from a performance standpoint.(I'm not entirely sure if that's really the case)

So I adjusted the timing in the relevant section of the app/pages/workspace/notebook/paragraph/code-editor/code-editor.component.ts instead. Because the getEditorSetting method that triggers EDITOR_SETTING is only executed when the parameter of setParagraphMode is true, and the only place it is used in this form is inside onDidChangeModelContent. Therefore, I modified it so that when setParagraphMode(true) is called in that area, the timing is adjusted using setTimeout.

After reverted NotebookServer code, everything worked as intended. Thanks for pointing out a better way to improve it.

[dididy]

Nice fix! Since the setTimeout(..., 200) is a heuristic to dodge a race, could we add a short comment explaining (1) why it's needed and (2) a TODO o how to make this deterministic or order-independent?

https://github.com/apache/zeppelin/pull/5044/commits/ab073599e086c6f9fb9084122814c0b0778d970d

Ensuring the execution order of EDITOR_SETTING seems necessary because it can otherwise cause a bug that passes through the short-circuit condition. Although this issue does not reproduce for everyone, controlling the execution timing appears to be required.

EDITOR_SETTING is executed when the first paragraph component is rendered. From the Classic UI console logs above, we can see that it runs after INTERPRETER_BINDINGS when first entering the note. Therefore, I added a variable editorSettingTriggerAllowed with an initial value of false in /app/pages/workspace/notebook/paragraph/code-editor/code-editor.component.ts. The getEditorSetting method now only executes if this variable is true.

Then, in /app/pages/workspace/notebook/notebook.component.ts, I added code that sets editorSettingTriggerAllowed to true for all paragraphs and calls getEditorSetting when receiving INTERPRETER_BINDINGS.

The part where I noticed the issue was at the moment of receiving PARAGRAPH_ADDED op. When a new paragraph component is added by PARAGRAPH_ADDED, getEditorSetting is immediately executed. This triggers EDITOR_SETTING, but since execution is now blocked by editorSettingTriggerAllowed, as I fixed, it does not run at that point. Instead, editorSettingTriggerAllowed is set to true and getEditorSetting is executed when the PARAGRAPH op(/app/core/paragraph-base/paragraph-base.ts) is received.

[dididy]

c90cd09

• Made notebookParagraphCodeEditorComponent optional.

5e35dbf

• Added a comment for editorSettingTriggerAllowed.

6c993a0

• There are two related actions: CLONE_PARAGRAPH and INSERT_PARAGRAPH. Each needs to be handled slightly differently.

Behavior

• CLONE_PARAGRAPH
  1. send > CLONE_PARAGRAPH
  2. receive > PARAGRAPH_ADDED → PARAGRAPH → trigger EDITOR_SETTING
• INSERT_PARAGRAPH
  1. send > INSERT_PARAGRAPH
  2. receive > trigger EDITOR_SETTING after PARAGRAPH_ADDED

Implementation

• CLONE_PARAGRAPH was already handled correctly.
• For INSERT_PARAGRAPH, we added:
  • A variable isTriggeredByInsertParagraph in notebook.component.ts.
  • Set it to true when insertParagraph in paragraph.component.ts is executed.
• This allows branching based on whether PARAGRAPH_ADDED was triggered by an INSERT_PARAGRAPH or a CLONE_PARAGRAPH.

Reasoning

• CLONE_PARAGRAPH: EDITOR_SETTING runs after receiving PARAGRAPH.
• INSERT_PARAGRAPH: EDITOR_SETTING runs when PARAGRAPH_ADDED is received.

[dididy]

I rebased due to conflicts caused by merged PRs.