Skip to content

Commit a8385a0

Browse files
dididytbonelee
authored andcommitted
[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 4fbfaec) Signed-off-by: ChanHo Lee <[email protected]>
1 parent 5fdd0a1 commit a8385a0

File tree

3 files changed

+82
-33
lines changed

3 files changed

+82
-33
lines changed

zeppelin-web-angular/src/app/pages/workspace/notebook/notebook.component.ts

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,14 @@ export class NotebookComponent extends MessageListenersManager implements OnInit
128128
return;
129129
}
130130
const definedNote = this.note;
131-
definedNote.paragraphs = definedNote.paragraphs.filter(p => p.id !== data.id);
131+
const paragraphIndex = definedNote.paragraphs.findIndex(p => p.id === data.id);
132+
definedNote.paragraphs = definedNote.paragraphs.filter((p, index) => index !== paragraphIndex);
133+
const adjustedCursorIndex =
134+
paragraphIndex === definedNote.paragraphs.length ? paragraphIndex - 1 : paragraphIndex + 1;
135+
const targetParagraph = this.listOfNotebookParagraphComponent.find((_, index) => index === adjustedCursorIndex);
136+
if (targetParagraph) {
137+
targetParagraph.focusEditor();
138+
}
132139
this.cdr.markForCheck();
133140
}
134141

@@ -142,15 +149,11 @@ export class NotebookComponent extends MessageListenersManager implements OnInit
142149
return;
143150
}
144151
const definedNote = this.note;
145-
definedNote.paragraphs.splice(data.index, 0, data.paragraph).map(p => {
146-
return {
147-
...p,
148-
focus: p.id === data.paragraph.id
149-
};
150-
});
151-
definedNote.paragraphs = [...definedNote.paragraphs];
152+
definedNote.paragraphs.splice(data.index, 0, data.paragraph);
153+
const paragraphIndex = definedNote.paragraphs.findIndex(p => p.id === data.paragraph.id);
154+
155+
definedNote.paragraphs[paragraphIndex].focus = true;
152156
this.cdr.markForCheck();
153-
// TODO(hsuanxyz) focus on paragraph
154157
}
155158

156159
@MessageListener(OP.SAVE_NOTE_FORMS)

zeppelin-web-angular/src/app/pages/workspace/notebook/paragraph/code-editor/code-editor.component.ts

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import {
2323
Output,
2424
SimpleChanges
2525
} from '@angular/core';
26-
import { editor as MonacoEditor, IDisposable, KeyCode } from 'monaco-editor';
26+
import { editor as MonacoEditor, IDisposable, IPosition, KeyCode, Position } from 'monaco-editor';
2727

2828
import { InterpreterBindingItem } from '@zeppelin/sdk';
2929
import { CompletionService, MessageService } from '@zeppelin/services';
@@ -41,8 +41,7 @@ type IEditor = MonacoEditor.IEditor;
4141
changeDetection: ChangeDetectionStrategy.OnPush
4242
})
4343
export class NotebookParagraphCodeEditorComponent implements OnChanges, OnDestroy, AfterViewInit {
44-
// TODO(hsuanxyz):
45-
// 1. cursor position
44+
@Input() position: IPosition | null = null;
4645
@Input() readOnly = false;
4746
@Input() language = 'text';
4847
@Input() paragraphControl!: NotebookParagraphControlComponent;
@@ -83,7 +82,11 @@ export class NotebookParagraphCodeEditorComponent implements OnChanges, OnDestro
8382
editor.onDidBlurEditorText(() => {
8483
this.editorBlur.emit();
8584
}),
86-
85+
editor.onDidChangeCursorPosition(e => {
86+
this.ngZone.run(() => {
87+
this.position = e.position;
88+
});
89+
}),
8790
editor.onDidChangeModelContent(() => {
8891
this.ngZone.run(() => {
8992
const model = editor.getModel();
@@ -175,6 +178,35 @@ export class NotebookParagraphCodeEditorComponent implements OnChanges, OnDestro
175178
}
176179
}
177180

181+
setCursorPosition({ lineNumber, column }: IPosition) {
182+
if (this.editor) {
183+
this.editor.setPosition({ lineNumber, column });
184+
}
185+
}
186+
187+
setRestorePosition() {
188+
if (this.editor) {
189+
const previousPosition = this.position ?? { lineNumber: 0, column: 0 };
190+
this.setCursorPosition(previousPosition);
191+
this.editor.focus();
192+
}
193+
}
194+
195+
setCursorPositionToBeginning() {
196+
if (this.editor) {
197+
this.setCursorPosition({ lineNumber: 0, column: 0 });
198+
this.editor.focus();
199+
}
200+
}
201+
202+
setCursorPositionToEnd() {
203+
if (this.editor) {
204+
const lineNumber = this.editor.getModel()?.getLineCount() ?? 0;
205+
const column = this.editor.getModel()?.getLineMaxColumn(lineNumber) ?? 0;
206+
this.setCursorPosition({ lineNumber, column });
207+
}
208+
}
209+
178210
initializedEditor(editor: IEditor) {
179211
this.editor = editor as IStandaloneCodeEditor;
180212
this.editor.addCommand(

zeppelin-web-angular/src/app/pages/workspace/notebook/paragraph/paragraph.component.ts

Lines changed: 34 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ type Mode = 'edit' | 'command';
6464
})
6565
export class NotebookParagraphComponent extends ParagraphBase implements OnInit, OnChanges, OnDestroy, AfterViewInit {
6666
@ViewChild(NotebookParagraphCodeEditorComponent, { static: false })
67-
notebookParagraphCodeEditorComponent!: NotebookParagraphCodeEditorComponent;
67+
notebookParagraphCodeEditorComponent?: NotebookParagraphCodeEditorComponent;
6868
@ViewChildren(NotebookParagraphResultComponent) notebookParagraphResultComponents!: QueryList<
6969
NotebookParagraphResultComponent
7070
>;
@@ -180,15 +180,22 @@ export class NotebookParagraphComponent extends ParagraphBase implements OnInit,
180180
nzContent: `All the paragraphs can't be deleted`
181181
});
182182
} else {
183-
this.nzModalService.confirm({
184-
nzTitle: 'Delete Paragraph',
185-
nzContent: 'Do you want to delete this paragraph?',
186-
nzOnOk: () => {
187-
this.messageService.paragraphRemove(this.paragraph.id);
188-
this.cdr.markForCheck();
189-
// TODO(hsuanxyz) moveFocusToNextParagraph
190-
}
191-
});
183+
this.nzModalService
184+
.confirm({
185+
nzTitle: 'Delete Paragraph',
186+
nzContent: 'Do you want to delete this paragraph?',
187+
nzAutofocus: null,
188+
nzOnOk: () => true
189+
})
190+
.afterClose.pipe(takeUntil(this.destroy$))
191+
.subscribe(result => {
192+
// In the modal, clicking "Cancel" makes result undefined.
193+
// Clicking "OK" makes result defined and passes the condition below.
194+
if (result) {
195+
this.messageService.paragraphRemove(this.paragraph.id);
196+
this.cdr.markForCheck();
197+
}
198+
});
192199
}
193200
}
194201
}
@@ -206,14 +213,19 @@ export class NotebookParagraphComponent extends ParagraphBase implements OnInit,
206213
params: p.settings.params
207214
};
208215
});
209-
this.nzModalService.confirm({
210-
nzTitle: 'Run all above?',
211-
nzContent: 'Are you sure to run all above paragraphs?',
212-
nzOnOk: () => {
213-
this.messageService.runAllParagraphs(this.note.id, paragraphs);
214-
}
215-
});
216-
// TODO(hsuanxyz): save cursor
216+
this.nzModalService
217+
.confirm({
218+
nzTitle: 'Run all above?',
219+
nzContent: 'Are you sure to run all above paragraphs?',
220+
nzOnOk: () => {
221+
this.messageService.runAllParagraphs(this.note.id, paragraphs);
222+
}
223+
})
224+
.afterClose.pipe(takeUntil(this.destroy$))
225+
.subscribe(() => {
226+
this.waitConfirmFromEdit = false;
227+
this.notebookParagraphCodeEditorComponent?.setRestorePosition();
228+
});
217229
}
218230

219231
doubleClickParagraph() {
@@ -223,7 +235,9 @@ export class NotebookParagraphComponent extends ParagraphBase implements OnInit,
223235
if (this.paragraph.config.editorSetting.editOnDblClick && this.revisionView !== true) {
224236
this.paragraph.config.editorHide = false;
225237
this.paragraph.config.tableHide = true;
226-
// TODO(hsuanxyz): focus editor
238+
this.focusEditor();
239+
this.cdr.detectChanges();
240+
this.notebookParagraphCodeEditorComponent?.setCursorPositionToEnd();
227241
}
228242
}
229243

@@ -251,8 +265,8 @@ export class NotebookParagraphComponent extends ParagraphBase implements OnInit,
251265
.afterClose.pipe(takeUntil(this.destroy$))
252266
.subscribe(() => {
253267
this.waitConfirmFromEdit = false;
268+
this.notebookParagraphCodeEditorComponent?.setRestorePosition();
254269
});
255-
// TODO(hsuanxyz): save cursor
256270
}
257271

258272
cloneParagraph(position: string = 'below', newText?: string) {

0 commit comments

Comments
 (0)