rfellows commented on code in PR #11304:
URL: https://github.com/apache/nifi/pull/11304#discussion_r3350975468


##########
nifi-frontend/src/main/frontend/libs/shared/src/components/component-context/component-context.component.html:
##########
@@ -18,7 +18,11 @@
 <div class="component-context flex flex-col">
     <div class="flex gap-x-1 items-center">
         <div class="context-logo text-3xl flex flex-col">
-            <i class="icon tertiary-color" [class]="componentIconClass"></i>
+            @if (componentIcon.iconType === 'font-awesome') {
+                <i [class]="componentIcon.className"></i>
+            } @else {
+                <i class="icon tertiary-color" 
[class]="componentIcon.className"></i>
+            }

Review Comment:
   This is a bit off still vertically and lacks spacing between the icon and 
the text.
   
   **font-awesome:** 
   <img width="276" height="103" alt="Image" 
src="https://github.com/user-attachments/assets/e23669af-2100-4331-aef2-3102f4e4cc6e";
 />
   
   **flowfont:** 
   <img width="272" height="115" alt="Image" 
src="https://github.com/user-attachments/assets/b0925ced-9875-4413-9971-128b542baaac";
 />
   
   Also, the font-awesome icon gets its color from the method adding the class, 
but the others inline it right here in the dom. we should just follow that... 
and for that matter we could add the `fa` here too since we know these are 
font-awesome. Then, the method can just return the class name needed and not 
any extras...
   
   ```suggestion
               @if (componentIcon.iconType === 'font-awesome') {
                   <i class="fa mr-2 tertiary-color" 
[class]="componentIcon.className"></i>
               } @else {
                   <i class="icon tertiary-color" 
[class]="componentIcon.className"></i>
               }
   
   ```



##########
nifi-frontend/src/main/frontend/libs/shared/src/components/component-context/component-context.component.ts:
##########
@@ -42,29 +49,31 @@ export class ComponentContext {
     @Input() id: string | null = null;
     @Input() name: string | undefined = '';
 
-    private getIconClassName(type: ComponentType | string | null) {
+    private getIconMeta(type: ComponentType | string | null): IconMeta {
         if (type === null) {
-            return 'icon-drop';
+            return ComponentContext.DEFAULT_ICON;
         }
         switch (type) {
             case ComponentType.Connection:
-                return 'icon-connect';
+                return { className: 'icon-connect', iconType: 'flowfont' };
             case ComponentType.Processor:
-                return 'icon-processor';
+                return { className: 'icon-processor', iconType: 'flowfont' };
             case ComponentType.OutputPort:
-                return 'icon-port-out';
+                return { className: 'icon-port-out', iconType: 'flowfont' };
             case ComponentType.InputPort:
-                return 'icon-port-in';
+                return { className: 'icon-port-in', iconType: 'flowfont' };
             case ComponentType.ProcessGroup:
-                return 'icon-group';
+                return { className: 'icon-group', iconType: 'flowfont' };
             case ComponentType.Funnel:
-                return 'icon-funnel';
+                return { className: 'icon-funnel', iconType: 'flowfont' };
             case ComponentType.Label:
-                return 'icon-label';
+                return { className: 'icon-label', iconType: 'flowfont' };
             case ComponentType.RemoteProcessGroup:
-                return 'icon-group-remote';
+                return { className: 'icon-group-remote', iconType: 'flowfont' 
};
+            case ComponentType.Connector:
+                return { className: 'fa fa-plug tertiary-color', iconType: 
'font-awesome' };

Review Comment:
   based on my comment above, we can just do this here:
   ```suggestion
                   return { className: 'fa-plug', iconType: 'font-awesome' };
   ```



##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/queue/state/queue-listing/queue-listing.effects.ts:
##########


Review Comment:
   Can we add some tests here? I realize there were no tests prior, but I think 
now would be a good time to add some.



-- 
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]

Reply via email to