dididy commented on code in PR #5079:
URL: https://github.com/apache/zeppelin/pull/5079#discussion_r2367749675
##########
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();
+ }
+ }
Review Comment:
How about making the search input automatically focused when the search
button is clicked and the dropdown opens? In Monaco Editor, the input is
focused right away when performing a search.
##########
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>
Review Comment:
It would be better to disable the button when no search is triggered or when
there are no matching results.
Also, each button currently has its own border radius, but it would be
preferable to apply the radius only to the overall corners of the dropdown. You
can achieve this by using border-top-*-radius and border-bottom-*-radius
appropriately.
##########
zeppelin-web-angular/src/app/pages/workspace/notebook/notebook.component.html:
##########
@@ -23,6 +23,7 @@
[currentRevision]="currentRevision"
(tableHideChange)="setAllParagraphTableHide($event)"
(editorHideChange)="setAllParagraphEditorHide($event)"
+ (search)="onParagraphSearch($event)"
Review Comment:
Is there a particular reason why you used the name search here? What do you
think about renaming it to handleSearch instead?
##########
zeppelin-web-angular/src/app/pages/workspace/notebook/action-bar/action-bar.component.less:
##########
@@ -113,3 +113,12 @@
}
}
});
+
+.ant-dropdown-menu {
+ padding: 0;
+
+ .ant-input-group,
+ .ant-input {
+ height: 32px;
+ }
+}
Review Comment:
It looks like EOF handling is needed.
--
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]
