Re: [PR] [NIFI-13095] surface and contrast improvements [nifi]

2024-04-30 Thread via GitHub 


rfellows merged PR #8707:
URL: https://github.com/apache/nifi/pull/8707


-- 
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: issues-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] [NIFI-13095] surface and contrast improvements [nifi]

2024-04-29 Thread via GitHub 


scottyaslan commented on PR #8707:
URL: https://github.com/apache/nifi/pull/8707#issuecomment-2083768159

   @rfellows thanks for the feedback. I have addressed your comments. Let me 
know if you find anything else.


-- 
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: issues-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] [NIFI-13095] surface and contrast improvements [nifi]

2024-04-29 Thread via GitHub 


scottyaslan commented on code in PR #8707:
URL: https://github.com/apache/nifi/pull/8707#discussion_r1583802012


##
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/assets/styles/_listing-table.scss:
##
@@ -110,15 +120,15 @@
 }
 
 tr:hover {
-background-color: $surface-highlight !important;
+background-color: $highlight-surface;
 }
 
 .selected {
-background-color: $selected-row-color !important;
+background-color: $highlight-surface !important;

Review Comment:
   Ok yea, James also just pointed this out. It is especially tricky since we 
have the `unset` values in the properties table as well. His suggestion was to 
remove the even styles for the `.listing-table`'s and just have the lighter or 
darker nifi surface palette be the selected style depending on light/dark mode. 



-- 
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: issues-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] [NIFI-13095] surface and contrast improvements [nifi]

2024-04-29 Thread via GitHub 


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


##
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/assets/styles/_listing-table.scss:
##
@@ -110,15 +120,15 @@
 }
 
 tr:hover {
-background-color: $surface-highlight !important;
+background-color: $highlight-surface;
 }
 
 .selected {
-background-color: $selected-row-color !important;
+background-color: $highlight-surface !important;

Review Comment:
   selected and highlight are now the same color? this makes for odd tables:
   https://github.com/apache/nifi/assets/713866/9a841948-b995-43ae-9974-56312fabdbc5;>
   https://github.com/apache/nifi/assets/713866/c4ef2d5a-1814-4a97-8801-a16e7e00d58d;>
   
   
   



##
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/flow-designer/service/manager/processor-manager.service.ts:
##
@@ -226,7 +226,7 @@ export class ProcessorManager {
 // in
 details
 .append('rect')
-.attr('class', 'processor-stats-in-out')
+.attr('class', 'processor-stats-in-out odd')

Review Comment:
   if we specify `odd`, we should also specify `even`



##
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/flow-designer/service/manager/remote-process-group-manager.service.ts:
##
@@ -215,7 +215,7 @@ export class RemoteProcessGroupManager {
 // sent
 details
 .append('rect')
-.attr('class', 'remote-process-group-sent-stats')
+.attr('class', 'remote-process-group-sent-stats odd')

Review Comment:
   if we specify `odd`, we should also specify `even`



##
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/flow-designer/service/manager/process-group-manager.service.ts:
##
@@ -454,7 +454,7 @@ export class ProcessGroupManager {
 // queued
 details
 .append('rect')
-.attr('class', 'process-group-queued-stats')
+.attr('class', 'process-group-queued-stats odd')

Review Comment:
   if this is `odd`, shouldn't the next one be `even`? currently it displays 
the right color since the background is the same color (`even`). However, if 
for some reason we change the color for `even`, these would get missed.



##
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/flow-designer/ui/canvas/graph-controls/operation-control/operation-control.component.ts:
##
@@ -77,7 +78,8 @@ export class OperationControl {
 public canvasUtils: CanvasUtils,
 private canvasView: CanvasView,
 private client: Client,
-private storage: Storage
+private storage: Storage,
+protected themingService: ThemingService

Review Comment:
   `themingService` doesn't appear to be used. can it be removed?



-- 
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: issues-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] [NIFI-13095] surface and contrast improvements [nifi]

2024-04-29 Thread via GitHub 


rfellows commented on PR #8707:
URL: https://github.com/apache/nifi/pull/8707#issuecomment-2083232134

   reviewing...


-- 
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: issues-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org