nifiguy commented on code in PR #10097:
URL: https://github.com/apache/nifi/pull/10097#discussion_r2247833460
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/summary/ui/connection-status-listing/connection-status-table/connection-status-table.component.html:
##########
@@ -193,6 +193,16 @@
</td>
</ng-container>
+ <!-- Load Balance Status Column -->
+ <ng-container matColumnDef="loadBalanceStatus">
+ <th mat-header-cell *matHeaderCellDef
mat-sort-header>
+ <div class="overflow-ellipsis
overflow-hidden whitespace-nowrap">Load Balance</div>
Review Comment:
It's a subtle difference, but I prefer "Load Balance" (and if necessary will
add "Status" back in.) By phrasing it "Load Balancing", this is a active
wording and implies active state versus configuration status. It suggest a
status of "Active" means the connection is currently in the state of moving
data as it load balances and that once complete or once no more data is in the
connection's queue, it will update to "Inactive".
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/summary/ui/connection-status-listing/connection-status-table/connection-status-table.component.ts:
##########
@@ -124,12 +126,27 @@ export class ConnectionStatusTable extends
ComponentStatusTable<ConnectionStatus
return `${connection.connectionStatusSnapshot.percentUseCount}% |
${connection.connectionStatusSnapshot.percentUseBytes}%`;
}
+ formatLoadBalanceStatus(connection: ConnectionStatusSnapshotEntity):
string {
+ switch (connection.connectionStatusSnapshot.loadBalanceStatus) {
+ case 'LOAD_BALANCE_NOT_CONFIGURED':
+ return 'Not Configured';
+ case 'LOAD_BALANCE_ACTIVE':
+ return 'Active';
+ case 'LOAD_BALANCE_INACTIVE':
+ return 'Inactive';
+ default:
+ return 'unknown';
+
+ }
Review Comment:
I'm not certain what you mean by "more dynamic". I do not like simply
striking `LOAD_BALANCE_` from the string leaving `NOT_CONFIGURED`, `ACTIVE`,
and `INACTIVE`. This does not render nicely in the UI, i.e. the ALL CAPS
presentation is not appropriate in the UI as it feels over emphasized.
Similarly, to make it all lower case using `.toLowerCase()` isn't as elegant as
a natural reading presentation having the first character capitalized; it also
leaves behind an odd underscore in the case of "not_configured".
Also, in reality, the `unknown` case should never occur by the nature of the
`loadBalanceStatus` variable and its underlying allowable values, but is
necessary for the switch statement completeness. Since it should never occur, I
deliberately left it as all lowercase such that it would stand out as
inconsistent and suggest a "problem" if ever it presents itself in the future.
Bottom line, I'm thinking of appropriate presentation in the UI more than
elegant and minimal lines of code.
--
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]
