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) {