tbonelee commented on code in PR #5079:
URL: https://github.com/apache/zeppelin/pull/5079#discussion_r2366302732
##########
zeppelin-web-angular/src/app/pages/workspace/notebook/action-bar/action-bar.component.ts:
##########
@@ -219,7 +222,21 @@ export class NotebookActionBarComponent extends
MessageListenersManager implemen
}
searchCode() {
- // TODO(hsuanxyz)
+ this.search.emit(this.searchText);
+ }
+
+ onSearchMenuOpenChange(open: boolean) {
+ if (!open) {
+ this.searchText = '';
+ this.searchCode();
+ }
+ }
+
+ onFindPrevClick(searchText: string) {}
+ onFindNextClick(searchText: string) {}
+ onReplaceClick(searchText: string, replaceText: string) {}
+ onReplaceAllClick(searchText: string, replaceText: string) {
+ this.search.emit(searchText); // notebook.component.ts에서 일괄 처리하도록 위임
}
Review Comment:
Please make sure to add `TODO` comments whereever the implementation is
still pending.
##########
zeppelin-web-angular/src/app/pages/workspace/notebook/action-bar/action-bar.component.less:
##########
Review Comment:
Could you add a newline at the end of the file?
##########
zeppelin-web-angular/src/app/pages/workspace/notebook/paragraph/paragraph.component.ts:
##########
@@ -96,6 +98,8 @@ export class NotebookParagraphComponent extends ParagraphBase
implements OnInit,
private mode: Mode = 'command';
waitConfirmFromEdit = false;
+ editor?: IStandaloneCodeEditor;
+ highlightDecorations?: string[];
Review Comment:
Are these used anywhere else? If not, we could consider removing them.
##########
zeppelin-web-angular/src/app/pages/workspace/notebook/action-bar/action-bar.component.ts:
##########
@@ -219,7 +222,21 @@ export class NotebookActionBarComponent extends
MessageListenersManager implemen
}
searchCode() {
- // TODO(hsuanxyz)
+ this.search.emit(this.searchText);
+ }
+
+ onSearchMenuOpenChange(open: boolean) {
+ if (!open) {
+ this.searchText = '';
+ this.searchCode();
+ }
+ }
+
+ onFindPrevClick(searchText: string) {}
+ onFindNextClick(searchText: string) {}
+ onReplaceClick(searchText: string, replaceText: string) {}
+ onReplaceAllClick(searchText: string, replaceText: string) {
+ this.search.emit(searchText); // notebook.component.ts에서 일괄 처리하도록 위임
}
Review Comment:
The Korean comment was not removed.
##########
zeppelin-web-angular/src/app/pages/workspace/notebook/notebook.component.ts:
##########
@@ -256,6 +256,14 @@ export class NotebookComponent extends
MessageListenersManager implements OnInit
this.cdr.markForCheck();
}
+ onParagraphSearch(term: string) {
+ if (!term) {
+ this.listOfNotebookParagraphComponent.forEach(comp =>
comp.highlightMatches(''));
+ return;
+ }
+ this.listOfNotebookParagraphComponent.forEach(comp =>
comp.highlightMatches(term));
Review Comment:
This could be simplified as follows:
```suggestion
this.listOfNotebookParagraphComponent.forEach(comp =>
comp.highlightMatches(term || ''));
```
##########
zeppelin-web-angular/src/app/pages/workspace/notebook/paragraph/paragraph.component.ts:
##########
@@ -116,6 +120,12 @@ export class NotebookParagraphComponent extends
ParagraphBase implements OnInit,
}
}
+ highlightMatches(searchText: string) {
+ if (this.notebookParagraphCodeEditorComponent) {
+ this.notebookParagraphCodeEditorComponent.highlightMatches(searchText);
+ }
Review Comment:
You might simplify this as follow:
```suggestion
this.notebookParagraphCodeEditorComponent?.highlightMatches(searchText);
```
##########
zeppelin-web-angular/src/app/pages/workspace/notebook/action-bar/action-bar.component.html:
##########
@@ -177,9 +177,41 @@
</nz-dropdown-menu>
</nz-button-group>
<nz-button-group nzSize="small">
- <button nz-button nz-tooltip nzTooltipTitle="Search code"
(click)="searchCode()">
+ <button
+ nz-button
+ nz-dropdown
+ [nzDropdownMenu]="searchMenu"
+ nzTrigger="click"
+ nz-tooltip
+ nzTooltipTitle="Search code"
+ nzTooltipTrigger="hover"
+ (nzVisibleChange)="onSearchMenuOpenChange($event)"
+ >
<i nz-icon nzType="search" nzTheme="outline"></i>
</button>
+ <!-- Search Code Dropdown UI -->
+ <nz-dropdown-menu #searchMenu="nzDropdownMenu">
+ <div class="ant-dropdown-menu">
+ <div style="display: flex;">
+ <nz-input-group nzAddOnBefore="Find">
+ <input type="text" nz-input [(ngModel)]="searchText"
(ngModelChange)="searchCode()" />
+ </nz-input-group>
+ <nz-button-group style="display: flex; align-items: center;
flex-wrap: nowrap;">
+ <button nz-button (click)="onFindPrevClick(searchText)"><i
nz-icon nzType="left"></i></button>
+ <button nz-button (click)="onFindNextClick(searchText)"><i
nz-icon nzType="right"></i></button>
+ </nz-button-group>
+ </div>
+ <div style="display: flex;">
+ <nz-input-group nzAddOnBefore="Replace">
+ <input type="text" nz-input [(ngModel)]="replaceText" />
+ </nz-input-group>
+ <nz-button-group style="display: flex; align-items: center;
flex-wrap: nowrap;">
+ <button nz-button (click)="onReplaceClick(searchText,
replaceText)">Replace</button>
+ <button nz-button (click)="onReplaceAllClick(searchText,
replaceText)">All</button>
+ </nz-button-group>
+ </div>
Review Comment:
Could you extract inline styles to `.less` file?
##########
zeppelin-web-angular/src/app/pages/workspace/notebook/paragraph/code-editor/code-editor.component.ts:
##########
@@ -59,6 +59,7 @@ export class NotebookParagraphCodeEditorComponent implements
OnChanges, OnDestro
@Output() readonly toggleEditorShow = new EventEmitter<void>();
private editor?: IStandaloneCodeEditor;
private monacoDisposables: IDisposable[] = [];
+ private highlightDecorations: string[] = [];
Review Comment:
How about defining a type that describes the meaning of the string somewhere
and use it for here?
This way, other developers can more quickly understand how this property is
indended to be used.
e.g.,
```typescript
type DecorationIdentifier =
ReturnType<monaco.editor.ICodeEditor['deltaDecorations']>[number];
@Component({
...
})
export class ...
...
private highlightDecorations: DecorationIdentifier[] = [];
...
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
