This is an automated email from the ASF dual-hosted git repository.

chanholee pushed a commit to branch branch-0.12
in repository https://gitbox.apache.org/repos/asf/zeppelin.git


The following commit(s) were added to refs/heads/branch-0.12 by this push:
     new a8385a0b73 [ZEPPELIN-6298] Fix cursor-related issues in New UI's 
Paragraph
a8385a0b73 is described below

commit a8385a0b731c55eca17766289ad0481ccc8013cb
Author: YONGJAE LEE(이용재) <dev.yongjae...@gmail.com>
AuthorDate: Sun Sep 7 21:55:33 2025 +0900

    [ZEPPELIN-6298] Fix cursor-related issues in New UI's Paragraph
    
    ### What is this PR for?
    **Description:**
    Cursor behavior in the New UI’s paragraph needs to be fixed for several 
cursor related actions, including double-clicking, running all above/below, 
adding(clone), and removing paragraphs.
    
    When **cloneParagraph()** is called, it internally calls 
**addParagraph()**, which has already been tested. The same 
addParagraph-related code is also applied in #5044. If either PR is merged 
first, I will rebase accordingly. I have also confirmed that 
**cloneParagraph()** works correctly through #5044.
    
    Due to timing issues, I used `setTimeout` for **removeParagraph()** and 
**doubleClickParagraph()**. Since this is in the UI area, it likely won’t have 
major side effects, but I will look into it further.
    
    **Expected:**
    - When **doubleClickParagraph()** is executed, the cursor should move to 
the end of the paragraph.
    - When **runAllAbove()** or **runAllBelow()** is executed, the current 
cursor position should be remembered, and after execution, focus should return 
to the previous cursor position.
    - When **addParagraph()** is executed, the newly added paragraph should 
receive focus.
    - When **removeParagraph()** is executed, focus should move to the 
paragraph that takes the deleted paragraph’s place.
    
    **Actual (New UI):**
    - When **doubleClickParagraph()** is executed, the cursor moves to the 
beginning instead of the end.
    - After **runAllAbove()** or **runAllBelow()**, focus is lost completely.
    - When **addParagraph()** is executed, the new paragraph does not 
automatically receive focus.
    - After **removeParagraph()**, focus may not move to the correct paragraph.
    
    **[Appropriate action - Classic UI]**
    
    
https://github.com/user-attachments/assets/fc0066f7-4e03-4e3b-9d5b-2f33df415ba7
    
    Run all above -> Run all below -> Double click .md paragraph  -> Add 
paragraph -> Delete paragraph
    
    **[AS-IS]**
    
    
https://github.com/user-attachments/assets/f699f788-cf29-4c4c-8c47-2ef34d7962f0
    
    Run all above -> Run all below -> Double click .md paragraph  -> Add 
paragraph -> Delete paragraph
    
    **[TO-BE]**
    
    
https://github.com/user-attachments/assets/1206c524-103f-4328-85ee-04408073b628
    
    Run all above -> Run all below -> Double click .md paragraph  -> Add 
paragraph -> Delete paragraph
    
    ### What type of PR is it?
    Bug Fix
    
    ### Todos
    
    ### What is the Jira issue?
    * [[ZEPPELIN-6298](https://issues.apache.org/jira/browse/ZEPPELIN-6298)]
    
    ### 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
    
    Closes #5057 from dididy/fix/ZEPPELIN-6298.
    
    Signed-off-by: ChanHo Lee <chanho...@apache.org>
    (cherry picked from commit 4fbfaec6160f338248c44b72938ca50d38068d19)
    Signed-off-by: ChanHo Lee <chanho...@apache.org>
---
 .../pages/workspace/notebook/notebook.component.ts | 21 +++++----
 .../paragraph/code-editor/code-editor.component.ts | 40 ++++++++++++++--
 .../notebook/paragraph/paragraph.component.ts      | 54 ++++++++++++++--------
 3 files changed, 82 insertions(+), 33 deletions(-)

diff --git 
a/zeppelin-web-angular/src/app/pages/workspace/notebook/notebook.component.ts 
b/zeppelin-web-angular/src/app/pages/workspace/notebook/notebook.component.ts
index f7511cd75c..57e5b20749 100644
--- 
a/zeppelin-web-angular/src/app/pages/workspace/notebook/notebook.component.ts
+++ 
b/zeppelin-web-angular/src/app/pages/workspace/notebook/notebook.component.ts
@@ -128,7 +128,14 @@ export class NotebookComponent extends 
MessageListenersManager implements OnInit
       return;
     }
     const definedNote = this.note;
-    definedNote.paragraphs = definedNote.paragraphs.filter(p => p.id !== 
data.id);
+    const paragraphIndex = definedNote.paragraphs.findIndex(p => p.id === 
data.id);
+    definedNote.paragraphs = definedNote.paragraphs.filter((p, index) => index 
!== paragraphIndex);
+    const adjustedCursorIndex =
+      paragraphIndex === definedNote.paragraphs.length ? paragraphIndex - 1 : 
paragraphIndex + 1;
+    const targetParagraph = this.listOfNotebookParagraphComponent.find((_, 
index) => index === adjustedCursorIndex);
+    if (targetParagraph) {
+      targetParagraph.focusEditor();
+    }
     this.cdr.markForCheck();
   }
 
@@ -142,15 +149,11 @@ export class NotebookComponent extends 
MessageListenersManager implements OnInit
       return;
     }
     const definedNote = this.note;
-    definedNote.paragraphs.splice(data.index, 0, data.paragraph).map(p => {
-      return {
-        ...p,
-        focus: p.id === data.paragraph.id
-      };
-    });
-    definedNote.paragraphs = [...definedNote.paragraphs];
+    definedNote.paragraphs.splice(data.index, 0, data.paragraph);
+    const paragraphIndex = definedNote.paragraphs.findIndex(p => p.id === 
data.paragraph.id);
+
+    definedNote.paragraphs[paragraphIndex].focus = true;
     this.cdr.markForCheck();
-    // TODO(hsuanxyz) focus on paragraph
   }
 
   @MessageListener(OP.SAVE_NOTE_FORMS)
diff --git 
a/zeppelin-web-angular/src/app/pages/workspace/notebook/paragraph/code-editor/code-editor.component.ts
 
b/zeppelin-web-angular/src/app/pages/workspace/notebook/paragraph/code-editor/code-editor.component.ts
index 5cf9d2623f..fdbc3f37d0 100644
--- 
a/zeppelin-web-angular/src/app/pages/workspace/notebook/paragraph/code-editor/code-editor.component.ts
+++ 
b/zeppelin-web-angular/src/app/pages/workspace/notebook/paragraph/code-editor/code-editor.component.ts
@@ -23,7 +23,7 @@ import {
   Output,
   SimpleChanges
 } from '@angular/core';
-import { editor as MonacoEditor, IDisposable, KeyCode } from 'monaco-editor';
+import { editor as MonacoEditor, IDisposable, IPosition, KeyCode, Position } 
from 'monaco-editor';
 
 import { InterpreterBindingItem } from '@zeppelin/sdk';
 import { CompletionService, MessageService } from '@zeppelin/services';
@@ -41,8 +41,7 @@ type IEditor = MonacoEditor.IEditor;
   changeDetection: ChangeDetectionStrategy.OnPush
 })
 export class NotebookParagraphCodeEditorComponent implements OnChanges, 
OnDestroy, AfterViewInit {
-  // TODO(hsuanxyz):
-  //  1. cursor position
+  @Input() position: IPosition | null = null;
   @Input() readOnly = false;
   @Input() language = 'text';
   @Input() paragraphControl!: NotebookParagraphControlComponent;
@@ -83,7 +82,11 @@ export class NotebookParagraphCodeEditorComponent implements 
OnChanges, OnDestro
       editor.onDidBlurEditorText(() => {
         this.editorBlur.emit();
       }),
-
+      editor.onDidChangeCursorPosition(e => {
+        this.ngZone.run(() => {
+          this.position = e.position;
+        });
+      }),
       editor.onDidChangeModelContent(() => {
         this.ngZone.run(() => {
           const model = editor.getModel();
@@ -175,6 +178,35 @@ export class NotebookParagraphCodeEditorComponent 
implements OnChanges, OnDestro
     }
   }
 
+  setCursorPosition({ lineNumber, column }: IPosition) {
+    if (this.editor) {
+      this.editor.setPosition({ lineNumber, column });
+    }
+  }
+
+  setRestorePosition() {
+    if (this.editor) {
+      const previousPosition = this.position ?? { lineNumber: 0, column: 0 };
+      this.setCursorPosition(previousPosition);
+      this.editor.focus();
+    }
+  }
+
+  setCursorPositionToBeginning() {
+    if (this.editor) {
+      this.setCursorPosition({ lineNumber: 0, column: 0 });
+      this.editor.focus();
+    }
+  }
+
+  setCursorPositionToEnd() {
+    if (this.editor) {
+      const lineNumber = this.editor.getModel()?.getLineCount() ?? 0;
+      const column = this.editor.getModel()?.getLineMaxColumn(lineNumber) ?? 0;
+      this.setCursorPosition({ lineNumber, column });
+    }
+  }
+
   initializedEditor(editor: IEditor) {
     this.editor = editor as IStandaloneCodeEditor;
     this.editor.addCommand(
diff --git 
a/zeppelin-web-angular/src/app/pages/workspace/notebook/paragraph/paragraph.component.ts
 
b/zeppelin-web-angular/src/app/pages/workspace/notebook/paragraph/paragraph.component.ts
index d740b9317f..bdd6adfb05 100644
--- 
a/zeppelin-web-angular/src/app/pages/workspace/notebook/paragraph/paragraph.component.ts
+++ 
b/zeppelin-web-angular/src/app/pages/workspace/notebook/paragraph/paragraph.component.ts
@@ -64,7 +64,7 @@ type Mode = 'edit' | 'command';
 })
 export class NotebookParagraphComponent extends ParagraphBase implements 
OnInit, OnChanges, OnDestroy, AfterViewInit {
   @ViewChild(NotebookParagraphCodeEditorComponent, { static: false })
-  notebookParagraphCodeEditorComponent!: NotebookParagraphCodeEditorComponent;
+  notebookParagraphCodeEditorComponent?: NotebookParagraphCodeEditorComponent;
   @ViewChildren(NotebookParagraphResultComponent) 
notebookParagraphResultComponents!: QueryList<
     NotebookParagraphResultComponent
   >;
@@ -180,15 +180,22 @@ export class NotebookParagraphComponent extends 
ParagraphBase implements OnInit,
           nzContent: `All the paragraphs can't be deleted`
         });
       } else {
-        this.nzModalService.confirm({
-          nzTitle: 'Delete Paragraph',
-          nzContent: 'Do you want to delete this paragraph?',
-          nzOnOk: () => {
-            this.messageService.paragraphRemove(this.paragraph.id);
-            this.cdr.markForCheck();
-            // TODO(hsuanxyz) moveFocusToNextParagraph
-          }
-        });
+        this.nzModalService
+          .confirm({
+            nzTitle: 'Delete Paragraph',
+            nzContent: 'Do you want to delete this paragraph?',
+            nzAutofocus: null,
+            nzOnOk: () => true
+          })
+          .afterClose.pipe(takeUntil(this.destroy$))
+          .subscribe(result => {
+            // In the modal, clicking "Cancel" makes result undefined.
+            // Clicking "OK" makes result defined and passes the condition 
below.
+            if (result) {
+              this.messageService.paragraphRemove(this.paragraph.id);
+              this.cdr.markForCheck();
+            }
+          });
       }
     }
   }
@@ -206,14 +213,19 @@ export class NotebookParagraphComponent extends 
ParagraphBase implements OnInit,
         params: p.settings.params
       };
     });
-    this.nzModalService.confirm({
-      nzTitle: 'Run all above?',
-      nzContent: 'Are you sure to run all above paragraphs?',
-      nzOnOk: () => {
-        this.messageService.runAllParagraphs(this.note.id, paragraphs);
-      }
-    });
-    // TODO(hsuanxyz): save cursor
+    this.nzModalService
+      .confirm({
+        nzTitle: 'Run all above?',
+        nzContent: 'Are you sure to run all above paragraphs?',
+        nzOnOk: () => {
+          this.messageService.runAllParagraphs(this.note.id, paragraphs);
+        }
+      })
+      .afterClose.pipe(takeUntil(this.destroy$))
+      .subscribe(() => {
+        this.waitConfirmFromEdit = false;
+        this.notebookParagraphCodeEditorComponent?.setRestorePosition();
+      });
   }
 
   doubleClickParagraph() {
@@ -223,7 +235,9 @@ export class NotebookParagraphComponent extends 
ParagraphBase implements OnInit,
     if (this.paragraph.config.editorSetting.editOnDblClick && 
this.revisionView !== true) {
       this.paragraph.config.editorHide = false;
       this.paragraph.config.tableHide = true;
-      // TODO(hsuanxyz): focus editor
+      this.focusEditor();
+      this.cdr.detectChanges();
+      this.notebookParagraphCodeEditorComponent?.setCursorPositionToEnd();
     }
   }
 
@@ -251,8 +265,8 @@ export class NotebookParagraphComponent extends 
ParagraphBase implements OnInit,
       .afterClose.pipe(takeUntil(this.destroy$))
       .subscribe(() => {
         this.waitConfirmFromEdit = false;
+        this.notebookParagraphCodeEditorComponent?.setRestorePosition();
       });
-    // TODO(hsuanxyz): save cursor
   }
 
   cloneParagraph(position: string = 'below', newText?: string) {

Reply via email to