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(이용재) <[email protected]>
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 <[email protected]>
(cherry picked from commit 4fbfaec6160f338248c44b72938ca50d38068d19)
Signed-off-by: ChanHo Lee <[email protected]>
---
.../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) {
