You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by "scottyaslan (via GitHub)" <gi...@apache.org> on 2024/04/30 21:23:33 UTC
[PR] [NIFI-13093] use warn palette for all errors and introduce caution palette [nifi]
scottyaslan opened a new pull request, #8721:
URL: https://github.com/apache/nifi/pull/8721
(no comment)
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [NIFI-13093] use warn palette for all errors and introduce caution palette [nifi]
Posted by "scottyaslan (via GitHub)" <gi...@apache.org>.
scottyaslan commented on code in PR #8721:
URL: https://github.com/apache/nifi/pull/8721#discussion_r1586895561
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/bulletins/ui/bulletin-board/bulletin-board-list/bulletin-board-list.component.ts:
##########
@@ -148,7 +148,7 @@ export class BulletinBoardList implements AfterViewInit {
case 'warning':
return 'bulletin-warn';
Review Comment:
No. Bulletins have a `.bulletin-error` state that is red. This is correct. I updated it to use the semantic `.caution-color` class.
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [NIFI-13093] use warn palette for all errors and introduce caution palette [nifi]
Posted by "scottyaslan (via GitHub)" <gi...@apache.org>.
scottyaslan commented on code in PR #8721:
URL: https://github.com/apache/nifi/pull/8721#discussion_r1586964629
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/assets/themes/material.scss:
##########
@@ -50,8 +50,33 @@ $material-primary-light-palette: (
A700: #ffffff,
)
);
+
$material-accent-light-palette: mat.$brown-palette;
-$warn-light-palette: mat.$red-palette;
+
+$warn-light-palette: (
+ 50: #ffebee,
+ 100: #ffccd2,
+ 200: #f49999,
+ 300: #eb7071,
+ 400: #f64e4c,
+ 500: #fa3b30,
+ 600: #ec3030,
+ 700: #ff1507,
+ 800: #ff0000,
+ 900: #ba554a, // g.component rect.border.unauthorized, #connection-full-drop-shadow feFlood, text.restricted, g.connection rect.border.unauthorized, g.connection path.connection-path.full, g.connection path.connection-path.unauthorized, .controller-bulletins.has-bulletins, path.link.selected, g.event circle.selected
+ contrast: (
+ 50: rgba(#000000, 0.87),
+ 100: rgba(#000000, 0.87),
+ 200: rgba(#000000, 0.87),
+ 300: rgba(#000000, 0.87),
+ 400: #ffffff,
+ 500: #ffffff,
+ 600: #ffffff,
+ 700: #ffffff,
+ 800: #ffffff,
+ 900: #ffffff
+ )
+);
Review Comment:
Good catch! I have updated this.
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [NIFI-13093] use warn palette for all errors and introduce caution palette [nifi]
Posted by "rfellows (via GitHub)" <gi...@apache.org>.
rfellows merged PR #8721:
URL: https://github.com/apache/nifi/pull/8721
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [NIFI-13093] use warn palette for all errors and introduce caution palette [nifi]
Posted by "scottyaslan (via GitHub)" <gi...@apache.org>.
scottyaslan commented on code in PR #8721:
URL: https://github.com/apache/nifi/pull/8721#discussion_r1586964545
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/assets/styles/_app.scss:
##########
@@ -261,56 +281,43 @@
//
// NOTE: nifi surface palette lighter/darker is purposefully omitted here. Any application of nifi surface colors should be as a background-color to a surface DOM element. The surface color should switch from light to dark depending on the mode for the theme.
- .nifi-surface-default {
+ .surface-color {
color: $nifi-theme-surface-palette-default;
fill: $nifi-theme-surface-palette-default;
}
- .nifi-success-default {
+ .success-color {
color: $nifi-theme-success-palette-default;
fill: $nifi-theme-success-palette-default;
}
- .nifi-success-lighter {
+ .success-color-lighter {
color: $nifi-theme-success-palette-lighter;
fill: $nifi-theme-success-palette-lighter;
}
- .nifi-success-darker {
+ .success-color-darker {
color: $nifi-theme-success-palette-darker;
fill: $nifi-theme-success-palette-darker;
}
- .nifi-warn-default {
- color: $nifi-theme-warn-palette-default;
- fill: $nifi-theme-warn-palette-default;
+ .caution-color {
+ color: $nifi-theme-caution-palette-default;
+ fill: $nifi-theme-caution-palette-default;
Review Comment:
Pretty much any time we need a yellow color it should be the `.caution-color`. Currently this is used extensively for any `.fa .fa-warning` icons which are used in the `.has-errors` or `.invalid` status scenarios. The darker caution color is used for things like back pressure, when a connection is selected on the canvas, and midpoints, bend points, and the label resize handle.
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [NIFI-13093] use warn palette for all errors and introduce caution palette [nifi]
Posted by "scottyaslan (via GitHub)" <gi...@apache.org>.
scottyaslan commented on code in PR #8721:
URL: https://github.com/apache/nifi/pull/8721#discussion_r1586964545
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/assets/styles/_app.scss:
##########
@@ -261,56 +281,43 @@
//
// NOTE: nifi surface palette lighter/darker is purposefully omitted here. Any application of nifi surface colors should be as a background-color to a surface DOM element. The surface color should switch from light to dark depending on the mode for the theme.
- .nifi-surface-default {
+ .surface-color {
color: $nifi-theme-surface-palette-default;
fill: $nifi-theme-surface-palette-default;
}
- .nifi-success-default {
+ .success-color {
color: $nifi-theme-success-palette-default;
fill: $nifi-theme-success-palette-default;
}
- .nifi-success-lighter {
+ .success-color-lighter {
color: $nifi-theme-success-palette-lighter;
fill: $nifi-theme-success-palette-lighter;
}
- .nifi-success-darker {
+ .success-color-darker {
color: $nifi-theme-success-palette-darker;
fill: $nifi-theme-success-palette-darker;
}
- .nifi-warn-default {
- color: $nifi-theme-warn-palette-default;
- fill: $nifi-theme-warn-palette-default;
+ .caution-color {
+ color: $nifi-theme-caution-palette-default;
+ fill: $nifi-theme-caution-palette-default;
Review Comment:
Pretty much any time we need a yellow color it should be the `.caution-color`. Currently this is used for the `.bulletin-warn` color and extensively for any `.fa .fa-warning` icons which are used in the `.has-errors` or `.invalid` status scenarios. The `.caution-color-darker` is used for things like back pressure, when a connection is selected on the canvas, and midpoints, bend points, and the label resize handle.
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [NIFI-13093] use warn palette for all errors and introduce caution palette [nifi]
Posted by "scottyaslan (via GitHub)" <gi...@apache.org>.
scottyaslan commented on PR #8721:
URL: https://github.com/apache/nifi/pull/8721#issuecomment-2090483016
> Overall this looks great. I did find one thing that we might want to clean up but I'm also happy if we just want to log it and fix as part of another pull request...
>
> Essentially, in dark mode, the checkbox we use to support select all, select none, or indicate some are selected isn't clearly defined when checked or indicating some are selected. It isn't immediately apparent that you can interact with it. It is fine when nothing is selected because it has an outline:
>
> <img alt="Screenshot 2024-05-02 at 8 30 28 AM" width="1036" src="https://private-user-images.githubusercontent.com/713866/327430128-18a9644b-079e-4c17-8966-51f0fac87880.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MTQ2NTYxOTUsIm5iZiI6MTcxNDY1NTg5NSwicGF0aCI6Ii83MTM4NjYvMzI3NDMwMTI4LTE4YTk2NDRiLTA3OWUtNGMxNy04OTY2LTUxZjBmYWM4Nzg4MC5wbmc_WC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTYmWC1BbXotQ3JlZGVudGlhbD1BS0lBVkNPRFlMU0E1M1BRSzRaQSUyRjIwMjQwNTAyJTJGdXMtZWFzdC0xJTJGczMlMkZhd3M0X3JlcXVlc3QmWC1BbXotRGF0ZT0yMDI0MDUwMlQxMzE4MTVaJlgtQW16LUV4cGlyZXM9MzAwJlgtQW16LVNpZ25hdHVyZT0yNDU2NjZjNTMwNmE4OWZmZDMzZDhmYTY0NzI1NzM3OGI5ZmU2MGVkZTkzYmMzNWNhYjU0ZGZhNmNkYmRmMzYzJlgtQW16LVNpZ25lZEhlYWRlcnM9aG9zdCZhY3Rvcl9pZD0wJmtleV9pZD0wJnJlcG9faWQ9MCJ9.ypA42Ik4DLHAMzsbO69Qcb93Sn0NbTRfiTRR3SWiFpI"> <img alt="Screenshot 2024-05-02 at 8 31 21 AM" width="1036" src="https://private-user-images.githubuserconte
nt.com/713866/327430171-dc15d0e5-74f2-486a-a370-35b57925ed02.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MTQ2NTYxOTUsIm5iZiI6MTcxNDY1NTg5NSwicGF0aCI6Ii83MTM4NjYvMzI3NDMwMTcxLWRjMTVkMGU1LTc0ZjItNDg2YS1hMzcwLTM1YjU3OTI1ZWQwMi5wbmc_WC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTYmWC1BbXotQ3JlZGVudGlhbD1BS0lBVkNPRFlMU0E1M1BRSzRaQSUyRjIwMjQwNTAyJTJGdXMtZWFzdC0xJTJGczMlMkZhd3M0X3JlcXVlc3QmWC1BbXotRGF0ZT0yMDI0MDUwMlQxMzE4MTVaJlgtQW16LUV4cGlyZXM9MzAwJlgtQW16LVNpZ25hdHVyZT1iNjdiYmM0MmNhZTExZDhiNGY1M2Y2MTU0Yzk1NDRlNTUyMDAwY2Y5MjRkNzI0ZjQxYmNkMzYzMDVkMzM0OTMxJlgtQW16LVNpZ25lZEhlYWRlcnM9aG9zdCZhY3Rvcl9pZD0wJmtleV9pZD0wJnJlcG9faWQ9MCJ9.pBzeNEpPZIqFXJoJyWrzdwx23eVwZ3LMnngvxJxb3BQ"> <img alt="Screenshot 2024-05-02 at 8 31 34 AM" width="1040" src="https://private-user-images.githubusercontent.com/713866/327430190-d2778102-9feb-4073-9b10-fdaafbc03bb6.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3Mi
OiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MTQ2NTYxOTUsIm5iZiI6MTcxNDY1NTg5NSwicGF0aCI6Ii83MTM4NjYvMzI3NDMwMTkwLWQyNzc4MTAyLTlmZWItNDA3My05YjEwLWZkYWFmYmMwM2JiNi5wbmc_WC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTYmWC1BbXotQ3JlZGVudGlhbD1BS0lBVkNPRFlMU0E1M1BRSzRaQSUyRjIwMjQwNTAyJTJGdXMtZWFzdC0xJTJGczMlMkZhd3M0X3JlcXVlc3QmWC1BbXotRGF0ZT0yMDI0MDUwMlQxMzE4MTVaJlgtQW16LUV4cGlyZXM9MzAwJlgtQW16LVNpZ25hdHVyZT03ZDliM2MyYWNkNGI5ZmY4NjlhYWVjNDIzY2JjMTZmMTA0YzJlYjMxMzJkOWZhMzI5MmE0NzNiOWVkNWQ2ZjBhJlgtQW16LVNpZ25lZEhlYWRlcnM9aG9zdCZhY3Rvcl9pZD0wJmtleV9pZD0wJnJlcG9faWQ9MCJ9.7XVxE_ECAQT3bBRzwjVEhbRPusDvfRY18xeoeV689J8">
> @scottyaslan let me know if you want to include a fix for this here or if we should log it separately.
Ooooooo good eye! Let's log it as a separate jira. Thanks!
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [NIFI-13093] use warn palette for all errors and introduce caution palette [nifi]
Posted by "rfellows (via GitHub)" <gi...@apache.org>.
rfellows commented on code in PR #8721:
URL: https://github.com/apache/nifi/pull/8721#discussion_r1586818369
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/assets/styles/_app.scss:
##########
@@ -261,56 +281,43 @@
//
// NOTE: nifi surface palette lighter/darker is purposefully omitted here. Any application of nifi surface colors should be as a background-color to a surface DOM element. The surface color should switch from light to dark depending on the mode for the theme.
- .nifi-surface-default {
+ .surface-color {
color: $nifi-theme-surface-palette-default;
fill: $nifi-theme-surface-palette-default;
}
- .nifi-success-default {
+ .success-color {
color: $nifi-theme-success-palette-default;
fill: $nifi-theme-success-palette-default;
}
- .nifi-success-lighter {
+ .success-color-lighter {
color: $nifi-theme-success-palette-lighter;
fill: $nifi-theme-success-palette-lighter;
}
- .nifi-success-darker {
+ .success-color-darker {
color: $nifi-theme-success-palette-darker;
fill: $nifi-theme-success-palette-darker;
}
- .nifi-warn-default {
- color: $nifi-theme-warn-palette-default;
- fill: $nifi-theme-warn-palette-default;
+ .caution-color {
+ color: $nifi-theme-caution-palette-default;
+ fill: $nifi-theme-caution-palette-default;
Review Comment:
what is the guidance for using the warn vs the caution colors?
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/bulletins/ui/bulletin-board/bulletin-board-list/bulletin-board-list.component.ts:
##########
@@ -148,7 +148,7 @@ export class BulletinBoardList implements AfterViewInit {
case 'warning':
return 'bulletin-warn';
Review Comment:
should this one use the `caution-color`
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/assets/themes/material.scss:
##########
@@ -50,8 +50,33 @@ $material-primary-light-palette: (
A700: #ffffff,
)
);
+
$material-accent-light-palette: mat.$brown-palette;
-$warn-light-palette: mat.$red-palette;
+
+$warn-light-palette: (
+ 50: #ffebee,
+ 100: #ffccd2,
+ 200: #f49999,
+ 300: #eb7071,
+ 400: #f64e4c,
+ 500: #fa3b30,
+ 600: #ec3030,
+ 700: #ff1507,
+ 800: #ff0000,
+ 900: #ba554a, // g.component rect.border.unauthorized, #connection-full-drop-shadow feFlood, text.restricted, g.connection rect.border.unauthorized, g.connection path.connection-path.full, g.connection path.connection-path.unauthorized, .controller-bulletins.has-bulletins, path.link.selected, g.event circle.selected
+ contrast: (
+ 50: rgba(#000000, 0.87),
+ 100: rgba(#000000, 0.87),
+ 200: rgba(#000000, 0.87),
+ 300: rgba(#000000, 0.87),
+ 400: #ffffff,
+ 500: #ffffff,
+ 600: #ffffff,
+ 700: #ffffff,
+ 800: #ffffff,
+ 900: #ffffff
+ )
+);
Review Comment:
The red is very bright in 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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [NIFI-13093] use warn palette for all errors and introduce caution palette [nifi]
Posted by "scottyaslan (via GitHub)" <gi...@apache.org>.
scottyaslan commented on code in PR #8721:
URL: https://github.com/apache/nifi/pull/8721#discussion_r1586895561
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/bulletins/ui/bulletin-board/bulletin-board-list/bulletin-board-list.component.ts:
##########
@@ -148,7 +148,7 @@ export class BulletinBoardList implements AfterViewInit {
case 'warning':
return 'bulletin-warn';
Review Comment:
No. Bulletins have a `.bulletin-error` state that is red. This is correct.
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [NIFI-13093] use warn palette for all errors and introduce caution palette [nifi]
Posted by "scottyaslan (via GitHub)" <gi...@apache.org>.
scottyaslan commented on code in PR #8721:
URL: https://github.com/apache/nifi/pull/8721#discussion_r1586964545
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/assets/styles/_app.scss:
##########
@@ -261,56 +281,43 @@
//
// NOTE: nifi surface palette lighter/darker is purposefully omitted here. Any application of nifi surface colors should be as a background-color to a surface DOM element. The surface color should switch from light to dark depending on the mode for the theme.
- .nifi-surface-default {
+ .surface-color {
color: $nifi-theme-surface-palette-default;
fill: $nifi-theme-surface-palette-default;
}
- .nifi-success-default {
+ .success-color {
color: $nifi-theme-success-palette-default;
fill: $nifi-theme-success-palette-default;
}
- .nifi-success-lighter {
+ .success-color-lighter {
color: $nifi-theme-success-palette-lighter;
fill: $nifi-theme-success-palette-lighter;
}
- .nifi-success-darker {
+ .success-color-darker {
color: $nifi-theme-success-palette-darker;
fill: $nifi-theme-success-palette-darker;
}
- .nifi-warn-default {
- color: $nifi-theme-warn-palette-default;
- fill: $nifi-theme-warn-palette-default;
+ .caution-color {
+ color: $nifi-theme-caution-palette-default;
+ fill: $nifi-theme-caution-palette-default;
Review Comment:
Pretty much any time we need a yellow color it should be the caution darker color. Currently this is used extensively for any `.fa .fa-warning` icons but it is also used for things like back pressure, when a connection is selected on the canvas, and midpoint/bendpoints/label resize.
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [NIFI-13093] use warn palette for all errors and introduce caution palette [nifi]
Posted by "rfellows (via GitHub)" <gi...@apache.org>.
rfellows commented on PR #8721:
URL: https://github.com/apache/nifi/pull/8721#issuecomment-2089085260
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org