rfellows commented on code in PR #8766:
URL: https://github.com/apache/nifi/pull/8766#discussion_r1593057709
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/ui/common/controller-service/controller-service-table/controller-service-table.component.html:
##########
@@ -116,62 +116,73 @@
<ng-container matColumnDef="actions">
<th mat-header-cell *matHeaderCellDef></th>
<td mat-cell *matCellDef="let item">
- @if (definedByCurrentGroup(item)) {
- <div class="flex items-center justify-end gap-x-2">
- <div
- class="pointer fa fa-cog primary-color"
- (click)="configureClicked(item, $event)"
- [title]="canConfigure(item) ? 'Edit' : 'View
Configuration'"></div>
- @if (hasAdvancedUi(item)) {
- <div
- class="pointer fa fa-cogs primary-color"
- (click)="advancedClicked(item, $event)"
- title="Advanced"></div>
+ <div class="flex items-center justify-end gap-x-2">
+ <button
+ mat-icon-button
+ type="button"
+ [matMenuTriggerFor]="actionMenu"
+ class="h-16 w-16 flex items-center justify-center
icon global-menu">
+ <i class="fa fa-ellipsis-v"></i>
+ </button>
+ <mat-menu #actionMenu="matMenu" xPosition="before">
+ @if (definedByCurrentGroup(item)) {
+ <button mat-menu-item
(click)="configureClicked(item, $event)">
+ <i class="fa fa-cog primary-color
mr-2"></i>
+ {{ canConfigure(item) ? 'Edit' : 'View
Configuration' }}
+ </button>
+ @if (hasAdvancedUi(item)) {
+ <button mat-menu-item
(click)="advancedClicked(item, $event)">
+ <i class="fa fa-cogs primary-color
mr-2"></i>
+ Advanced
+ </button>
+ }
+ @if (canDisable(item)) {
+ <button mat-menu-item
(click)="disableClicked(item)">
+ <i class="icon icon-enable-false
primary-color mr-2"></i>
+ Disable
+ </button>
+ }
+ @if (canEnable(item)) {
+ <button mat-menu-item
(click)="enabledClicked(item)">
+ <i class="fa fa-flash primary-color
mr-2"></i>
+ Enable
+ </button>
+ }
+ @if (canChangeVersion(item)) {
+ <button mat-menu-item
(click)="changeVersionClicked(item)">
+ <i class="fa fa-exchange primary-color
mr-2"></i>
+ Change Version
+ </button>
+ }
+ @if (canDelete(item)) {
+ <button mat-menu-item
(click)="deleteClicked(item, $event)">
+ <i class="fa fa-trash primary-color
mr-2"></i>
+ Delete
+ </button>
+ }
+ @if (canViewState(item)) {
+ <button mat-menu-item
(click)="viewStateClicked(item)">
+ <i class="fa fa-tasks primary-color
mr-2"></i>
+ View State
+ </button>
+ }
+ @if (canManageAccessPolicies()) {
+ <button
+ mat-menu-item
+ (click)="$event.stopPropagation()"
+ [routerLink]="getPolicyLink(item)">
+ <i class="fa fa-key primary-color
mr-2"></i>
+ Manage Access Policies
Review Comment:
The other tabs on this page all offer "Access Policies" as options, we
should be consistent. We have 3 different phrases now for this action "Access
Policies" (the other tabs in Controller Settings), "Manage Access Policies"
(canvas context menu and here), "Policies" (main menu). I guess I prefer the
shorter phrases so the dropdown width doesn't have to be so wide.
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/users/ui/user-listing/user-table/user-table.component.html:
##########
@@ -79,24 +79,37 @@
<th mat-header-cell *matHeaderCellDef></th>
<td mat-cell *matCellDef="let item">
<div class="flex items-center justify-end gap-x-2">
- @if (canEditOrDelete(currentUser, item)) {
- <div
- class="pointer fa fa-cog primary-color"
- title="Edit"
- (click)="editClicked(item, $event)"></div>
- }
- @if (canEditOrDelete(currentUser, item)) {
- <div
- class="pointer fa fa-trash primary-color"
- title="Remove"
- (click)="deleteClicked(item)"></div>
- }
- @if (hasAccessPolicies(item)) {
- <div
- class="pointer fa fa-key primary-color"
- title="View User Policies"
- (click)="viewAccessPoliciesClicked(item,
$event)"></div>
+ @if (
+ canEditOrDelete(currentUser, item) ||
hasAccessPolicies(item) || hasAccessPolicies(item)
+ ) {
+ <button
+ mat-icon-button
+ type="button"
+ [matMenuTriggerFor]="actionMenu"
+ class="h-16 w-16 flex items-center
justify-center icon global-menu">
+ <i class="fa fa-ellipsis-v"></i>
+ </button>
}
+ <mat-menu #actionMenu="matMenu" xPosition="before">
Review Comment:
This menu doesn't close when the edit or user policies dialogs pop up in
front:
<img width="1307" alt="Screenshot 2024-05-07 at 5 02 17 PM"
src="https://github.com/apache/nifi/assets/713866/70056aec-c345-4b8e-bcda-e4bfb8abeca7";>
It does hide if the delete confirmation dialog opens though.
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/summary/ui/processor-status-listing/processor-status-table/processor-status-table.component.html:
##########
@@ -248,25 +248,36 @@
<th mat-header-cell *matHeaderCellDef></th>
<td mat-cell *matCellDef="let item">
<div class="flex items-center justify-end
gap-x-2">
- <div
- class="pointer fa
fa-long-arrow-right primary-color"
-
[routerLink]="getProcessorLink(item)"
- (click)="$event.stopPropagation()"
- title="Go to Processor in {{
-
item?.processorStatusSnapshot?.processGroupNamePath
- }}"></div>
+ <button
+ mat-icon-button
+ type="button"
+ [matMenuTriggerFor]="actionMenu"
+ class="h-16 w-16 flex items-center
justify-center icon global-menu">
+ <i class="fa fa-ellipsis-v"></i>
+ </button>
+ <mat-menu #actionMenu="matMenu"
xPosition="before">
Review Comment:
This menu doesn't close when a dialog opens in front of it:
<img width="1183" alt="Screenshot 2024-05-07 at 4 57 39 PM"
src="https://github.com/apache/nifi/assets/713866/84cb2fc9-1e5d-427c-93f8-b6219f970563";>
_This comment applies to all summary tables, just going to leave comments
here for brevity_
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/summary/ui/processor-status-listing/processor-status-table/processor-status-table.component.html:
##########
@@ -248,25 +248,36 @@
<th mat-header-cell *matHeaderCellDef></th>
<td mat-cell *matCellDef="let item">
<div class="flex items-center justify-end
gap-x-2">
- <div
- class="pointer fa
fa-long-arrow-right primary-color"
-
[routerLink]="getProcessorLink(item)"
- (click)="$event.stopPropagation()"
- title="Go to Processor in {{
-
item?.processorStatusSnapshot?.processGroupNamePath
- }}"></div>
+ <button
+ mat-icon-button
+ type="button"
+ [matMenuTriggerFor]="actionMenu"
+ class="h-16 w-16 flex items-center
justify-center icon global-menu">
+ <i class="fa fa-ellipsis-v"></i>
+ </button>
+ <mat-menu #actionMenu="matMenu"
xPosition="before">
+ <button
+ mat-menu-item
+
[routerLink]="getProcessorLink(item)"
+
(click)="$event.stopPropagation()">
+ <i class="fa
fa-long-arrow-right primary-color mr-2"></i>
+ Go to Processor in
+ {{
item?.processorStatusSnapshot?.processGroupNamePath }}
+ </button>
+ <button mat-menu-item
(click)="viewStatusHistoryClicked($event, item)">
+ <i class="fa fa-area-chart
primary-color mr-2"></i>
+ View Status History
Review Comment:
We could probably just use "Status History" here to shorten the phrase if we
decide to shorted the "View Clustered Processor Details" option as well.
_This comment applies to all summary tables, just going to leave comments
here for brevity_
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/summary/ui/processor-status-listing/processor-status-table/processor-status-table.component.html:
##########
@@ -248,25 +248,36 @@
<th mat-header-cell *matHeaderCellDef></th>
<td mat-cell *matCellDef="let item">
<div class="flex items-center justify-end
gap-x-2">
- <div
- class="pointer fa
fa-long-arrow-right primary-color"
-
[routerLink]="getProcessorLink(item)"
- (click)="$event.stopPropagation()"
- title="Go to Processor in {{
-
item?.processorStatusSnapshot?.processGroupNamePath
- }}"></div>
+ <button
+ mat-icon-button
+ type="button"
+ [matMenuTriggerFor]="actionMenu"
+ class="h-16 w-16 flex items-center
justify-center icon global-menu">
+ <i class="fa fa-ellipsis-v"></i>
+ </button>
+ <mat-menu #actionMenu="matMenu"
xPosition="before">
+ <button
+ mat-menu-item
+
[routerLink]="getProcessorLink(item)"
+
(click)="$event.stopPropagation()">
+ <i class="fa
fa-long-arrow-right primary-color mr-2"></i>
+ Go to Processor in
+ {{
item?.processorStatusSnapshot?.processGroupNamePath }}
+ </button>
+ <button mat-menu-item
(click)="viewStatusHistoryClicked($event, item)">
+ <i class="fa fa-area-chart
primary-color mr-2"></i>
+ View Status History
+ </button>
- <div
- class="pointer fa fa-area-chart
primary-color"
- title="View Status History"
-
(click)="viewStatusHistoryClicked($event, item)"></div>
-
- @if (connectedToCluster) {
- <div
- class="pointer fa fa-cubes
primary-color"
- title="View Clustered
Processor Details"
-
(click)="viewClusteredDetailsClicked($event, item)"></div>
- }
+ @if (connectedToCluster) {
+ <button
+ mat-menu-item
+
(click)="viewClusteredDetailsClicked($event, item)">
+ <i class="fa fa-cubes
primary-color mr-2"></i>
+ View Clustered Processor
Details
Review Comment:
We could shorten this to "View Clustered Details" or "Clustered Details" to
avoid wrapping
<img width="353" alt="Screenshot 2024-05-07 at 4 41 26 PM"
src="https://github.com/apache/nifi/assets/713866/97a5968b-f711-44aa-a799-47356914827d";>
_This comment applies to all summary tables, just going to leave comments
here for brevity_
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/parameter-contexts/ui/parameter-context-listing/parameter-context-table/parameter-context-table.component.html:
##########
@@ -58,32 +58,52 @@
<th mat-header-cell *matHeaderCellDef></th>
<td mat-cell *matCellDef="let item">
<div class="flex items-center justify-end gap-x-2">
- @if (canRead(item)) {
- <div
- class="pointer fa fa-cog primary-color"
- (click)="editClicked(item, $event)"
- [title]="canWrite(item) ? 'Edit' : 'View
Configuration'"></div>
- }
- @if (canDelete(item)) {
- <div
- class="pointer fa fa-trash primary-color"
- (click)="deleteClicked(item, $event)"
- title="Delete"></div>
- }
- @if (canManageAccessPolicies()) {
- <div
- class="pointer fa fa-key primary-color"
- (click)="$event.stopPropagation()"
- [routerLink]="getPolicyLink(item)"
- title="Access Policies"></div>
- }
- @if (canGoToParameterProvider(item)) {
- <div
- class="pointer fa fa-long-arrow-right
primary-color"
- (click)="$event.stopPropagation()"
- [routerLink]="getParameterProviderLink(item)"
- title="Go to Parameter Provider"></div>
+ @if (
+ canRead(item) ||
+ canDelete(item) ||
+ canManageAccessPolicies() ||
+ canGoToParameterProvider(item)
+ ) {
+ <button
+ mat-icon-button
+ type="button"
+ [matMenuTriggerFor]="actionMenu"
+ class="h-16 w-16 flex items-center
justify-center icon global-menu">
+ <i class="fa fa-ellipsis-v"></i>
+ </button>
}
+ <mat-menu #actionMenu="matMenu" xPosition="before">
+ @if (canRead(item)) {
+ <button mat-menu-item
(click)="editClicked(item, $event)">
+ <i class="fa fa-cog primary-color
mr-2"></i>
+ {{ canWrite(item) ? 'Edit' : 'View
Configuration' }}
+ </button>
Review Comment:
This menu doesn't close when selecting Edit.
<img width="1214" alt="Screenshot 2024-05-07 at 4 55 48 PM"
src="https://github.com/apache/nifi/assets/713866/1b8694d9-030a-4f22-9916-6d763f3d1b64";>
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/summary/ui/processor-status-listing/processor-status-table/processor-status-table.component.html:
##########
@@ -248,25 +248,36 @@
<th mat-header-cell *matHeaderCellDef></th>
<td mat-cell *matCellDef="let item">
<div class="flex items-center justify-end
gap-x-2">
- <div
- class="pointer fa
fa-long-arrow-right primary-color"
-
[routerLink]="getProcessorLink(item)"
- (click)="$event.stopPropagation()"
- title="Go to Processor in {{
-
item?.processorStatusSnapshot?.processGroupNamePath
- }}"></div>
+ <button
+ mat-icon-button
+ type="button"
+ [matMenuTriggerFor]="actionMenu"
+ class="h-16 w-16 flex items-center
justify-center icon global-menu">
+ <i class="fa fa-ellipsis-v"></i>
+ </button>
+ <mat-menu #actionMenu="matMenu"
xPosition="before">
+ <button
+ mat-menu-item
+
[routerLink]="getProcessorLink(item)"
+
(click)="$event.stopPropagation()">
+ <i class="fa
fa-long-arrow-right primary-color mr-2"></i>
+ Go to Processor in
+ {{
item?.processorStatusSnapshot?.processGroupNamePath }}
Review Comment:
This translates into some long and wrapped options in the menu. We should
just show "Go To [component type]" to be consistent with other usages.
<img width="353" alt="Screenshot 2024-05-07 at 4 41 26 PM"
src="https://github.com/apache/nifi/assets/713866/ee5139ec-11f4-4594-b7ae-0c338c944f4a";>
_This comment applies to all summary tables, just going to leave comments
here for brevity_
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/ui/common/controller-service/controller-service-table/controller-service-table.component.html:
##########
@@ -116,62 +116,73 @@
<ng-container matColumnDef="actions">
<th mat-header-cell *matHeaderCellDef></th>
<td mat-cell *matCellDef="let item">
- @if (definedByCurrentGroup(item)) {
- <div class="flex items-center justify-end gap-x-2">
- <div
- class="pointer fa fa-cog primary-color"
- (click)="configureClicked(item, $event)"
- [title]="canConfigure(item) ? 'Edit' : 'View
Configuration'"></div>
- @if (hasAdvancedUi(item)) {
- <div
- class="pointer fa fa-cogs primary-color"
- (click)="advancedClicked(item, $event)"
- title="Advanced"></div>
+ <div class="flex items-center justify-end gap-x-2">
+ <button
+ mat-icon-button
+ type="button"
+ [matMenuTriggerFor]="actionMenu"
+ class="h-16 w-16 flex items-center justify-center
icon global-menu">
+ <i class="fa fa-ellipsis-v"></i>
+ </button>
+ <mat-menu #actionMenu="matMenu" xPosition="before">
+ @if (definedByCurrentGroup(item)) {
+ <button mat-menu-item
(click)="configureClicked(item, $event)">
+ <i class="fa fa-cog primary-color
mr-2"></i>
+ {{ canConfigure(item) ? 'Edit' : 'View
Configuration' }}
+ </button>
+ @if (hasAdvancedUi(item)) {
+ <button mat-menu-item
(click)="advancedClicked(item, $event)">
+ <i class="fa fa-cogs primary-color
mr-2"></i>
+ Advanced
+ </button>
+ }
+ @if (canDisable(item)) {
+ <button mat-menu-item
(click)="disableClicked(item)">
+ <i class="icon icon-enable-false
primary-color mr-2"></i>
+ Disable
+ </button>
+ }
+ @if (canEnable(item)) {
+ <button mat-menu-item
(click)="enabledClicked(item)">
+ <i class="fa fa-flash primary-color
mr-2"></i>
+ Enable
+ </button>
+ }
+ @if (canChangeVersion(item)) {
+ <button mat-menu-item
(click)="changeVersionClicked(item)">
+ <i class="fa fa-exchange primary-color
mr-2"></i>
+ Change Version
+ </button>
+ }
+ @if (canDelete(item)) {
+ <button mat-menu-item
(click)="deleteClicked(item, $event)">
+ <i class="fa fa-trash primary-color
mr-2"></i>
+ Delete
Review Comment:
Clicking the delete or edit action does not close the menu:
<img width="863" alt="Screenshot 2024-05-07 at 5 09 18 PM"
src="https://github.com/apache/nifi/assets/713866/7f2fcc7f-b493-4cab-8474-efa4582364de";>
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/users/ui/user-listing/user-table/user-table.component.html:
##########
@@ -79,24 +79,37 @@
<th mat-header-cell *matHeaderCellDef></th>
<td mat-cell *matCellDef="let item">
<div class="flex items-center justify-end gap-x-2">
- @if (canEditOrDelete(currentUser, item)) {
- <div
- class="pointer fa fa-cog primary-color"
- title="Edit"
- (click)="editClicked(item, $event)"></div>
- }
- @if (canEditOrDelete(currentUser, item)) {
- <div
- class="pointer fa fa-trash primary-color"
- title="Remove"
- (click)="deleteClicked(item)"></div>
- }
- @if (hasAccessPolicies(item)) {
- <div
- class="pointer fa fa-key primary-color"
- title="View User Policies"
- (click)="viewAccessPoliciesClicked(item,
$event)"></div>
+ @if (
+ canEditOrDelete(currentUser, item) ||
hasAccessPolicies(item) || hasAccessPolicies(item)
+ ) {
+ <button
+ mat-icon-button
+ type="button"
+ [matMenuTriggerFor]="actionMenu"
+ class="h-16 w-16 flex items-center
justify-center icon global-menu">
+ <i class="fa fa-ellipsis-v"></i>
+ </button>
}
+ <mat-menu #actionMenu="matMenu" xPosition="before">
+ @if (canEditOrDelete(currentUser, item)) {
+ <button mat-menu-item
(click)="editClicked(item, $event)">
+ <i class="fa fa-cog primary-color
mr-2"></i>
+ Edit
+ </button>
+ }
+ @if (canEditOrDelete(currentUser, item)) {
+ <button mat-menu-item
(click)="deleteClicked(item)">
+ <i class="fa fa-trash primary-color
mr-2"></i>
+ Remove
+ </button>
+ }
+ @if (hasAccessPolicies(item)) {
+ <button mat-menu-item
(click)="viewAccessPoliciesClicked(item, $event)">
+ <i class="fa fa-key primary-color
mr-2"></i>
+ View User Policies
Review Comment:
We should be consistent in our menu options for policies. This is slightly
different than the "Access Policies" option, but however we decide to phrase
that one should inform this as well so they are similar
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/settings/ui/reporting-tasks/reporting-task-table/reporting-task-table.component.html:
##########
@@ -111,53 +111,64 @@
<th mat-header-cell *matHeaderCellDef></th>
<td mat-cell *matCellDef="let item">
<div class="flex items-center justify-end gap-x-2">
- <div
- class="pointer fa fa-cog primary-color"
- (click)="configureClicked(item, $event)"
- [title]="canEdit(item) ? 'Edit' : 'View
Configuration'"></div>
- @if (hasAdvancedUi(item)) {
- <div
- class="pointer fa fa-cogs primary-color"
- (click)="advancedClicked(item, $event)"
- title="Advanced"></div>
- }
- @if (canStop(item)) {
- <div
- class="pointer fa fa-stop primary-color"
- (click)="stopClicked(item)"
- title="Stop"></div>
- }
- @if (canStart(item)) {
- <div
- class="pointer fa fa-play primary-color"
- (click)="startClicked(item)"
- title="Start"></div>
- }
- @if (canChangeVersion(item)) {
- <div
- class="pointer fa fa-exchange primary-color"
- (click)="changeVersionClicked(item)"
- title="Change Version"></div>
- }
- @if (canDelete(item)) {
- <div
- class="pointer fa fa-trash primary-color"
- (click)="deleteClicked(item)"
- title="Delete"></div>
- }
- @if (canViewState(item)) {
- <div
- class="pointer fa fa-tasks primary-color"
- (click)="viewStateClicked(item)"
- title="View State"></div>
- }
- @if (canManageAccessPolicies()) {
- <div
- class="pointer fa fa-key primary-color"
- (click)="$event.stopPropagation()"
- [routerLink]="getPolicyLink(item)"
- title="Access Policies"></div>
- }
+ <button
+ mat-icon-button
+ type="button"
+ [matMenuTriggerFor]="actionMenu"
+ class="h-16 w-16 flex items-center justify-center
icon global-menu">
+ <i class="fa fa-ellipsis-v"></i>
+ </button>
+ <mat-menu #actionMenu="matMenu" xPosition="before">
+ <button mat-menu-item
(click)="configureClicked(item, $event)">
+ <i class="fa fa-cog primary-color mr-2"></i>
+ {{ canEdit(item) ? 'Edit' : 'View
Configuration' }}
Review Comment:
Edit opens the dialog but does not close the menu:
<img width="1058" alt="Screenshot 2024-05-07 at 5 11 35 PM"
src="https://github.com/apache/nifi/assets/713866/ddcf1a4b-2725-4c87-a3b1-c104cecbd840";>
--
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]
