NIFI-15546 correct active load balance glyph location in UI - #11481
Conversation
exceptionfactory
left a comment
There was a problem hiding this comment.
Thanks for looking at this issue @markobean.
On a quick review, the referenced component has a load-balance-icon-active class applied, which is not defined. Defining that class to apply the transform might be the better way forward, as opposed to changing the nested definition of fa-rotate-90. There also be a better transform class that should be applied. I did not see analogous approaches, so tagging @mcgilman and @rfellows for review on the optimal approach.
Thanks for the feedback @exceptionfactory. However, the style needs to be in One thing I could do is rename |
| text.load-balance-icon-active.fa-rotate-90 { | ||
| transform-box: fill-box; | ||
| transform-origin: center; | ||
| } |
There was a problem hiding this comment.
I agree with @exceptionfactory, I just tried it out and was able to make his suggestion work.
Basically this becomes
| text.load-balance-icon-active.fa-rotate-90 { | |
| transform-box: fill-box; | |
| transform-origin: center; | |
| } | |
| text.load-balance-icon.load-balance-icon-active { | |
| transform: rotate(90deg); | |
| transform-box: fill-box; | |
| transform-origin: center; | |
| } |
and in connection-manager.service.ts this line:
.classed('load-balance-icon-active fa-rotate-90 success-color-variant', (d: any) => {
can just drop the fa-rotate-90
There was a problem hiding this comment.
I agree with @exceptionfactory, I just tried it out and was able to make his suggestion work.
Basically this becomes
and in
connection-manager.service.tsthis line:.classed('load-balance-icon-active fa-rotate-90 success-color-variant', (d: any) => {can just drop the
fa-rotate-90
Ah, ok, so you added the actual rotation (transform: rotate(90deg)) to the style. Got it. Will give this a go.
There was a problem hiding this comment.
@rfellows Tested proposed changes. Worked well. Pushed a new commit which eliminates fa-rotate-90 in favor of the localized load-balance-icon-active style.
Thanks for the recommendation.
There was a problem hiding this comment.
Since we are getting the rotation from the css class load-balance-icon-active, can you remove the fa-rotate-90 class from being added to the element in the first place? I suggested it above, but maybe you missed it.
There was a problem hiding this comment.
Sorry, @rfellows. My latest commit didn't take correctly.. missed the "-m" on the command and didn't notice. It's there now.
|
@markobean unfortunately this is failing CI since the commits are not signed. the original one was however. can you squash your commits and sign the new one, then force push it up? it looks good and i'd like to merge it. |
|
@rfellows I squashed, signed and pushed. Thanks. |
rfellows
left a comment
There was a problem hiding this comment.
Thanks for the contribution, @markobean 👍
Summary
Fixed the issue in the UI where the active load balancing glyph appears in the wrong location. The glyph is the same as the inactive load balancing glyph (two-tone circle split horizontally), but it is rotated and color changed. The rotation - but same location - makes it appear like the icon has changed while load balancing is active. The problem was that the center of rotation was not at the center of the icon; it rotated the icon to an undesirable location no longer within the connection label.
This PR adjusts the center of rotation to be the center of the glyph keeping its location consistent whether it is the active or inactive load balance icon.
NIFI-15546
Tracking
Please complete the following tracking steps prior to pull request creation.
Issue Tracking
Pull Request Tracking
NIFI-00000NIFI-00000VerifiedstatusPull Request Formatting
mainbranchVerification
Testing requires a Cluster; it's the only way to activate load balancing. Queue up a relatively large number of FlowFiles (or few FlowFiles of large size.) Doing so will allow a refresh to display the active load balancing icon. Observe the active versus inactive state. The circle icon will appear to rotate (and change color) but will remain in the exact same location.
Build
./mvnw clean install -P contrib-checkLicensing
LICENSEandNOTICEfilesDocumentation