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 2025/02/19 22:06:16 UTC
[PR] [NIFI-14258] tokenize theme [nifi]
scottyaslan opened a new pull request, #9733:
URL: https://github.com/apache/nifi/pull/9733
# Summary
[NIFI-14258](https://issues.apache.org/jira/browse/NIFI-14258)
This theming upgrade aligns with best practices as outlined in Angular 19 guides:
- https://material.angular.io/guide/theming
- https://material.angular.io/guide/system-variables
As such all color palettes have been mapped into material design system tokens so the entire application theming has been affected in this PR. When reviewing the reviewer should definitely compare both light and dark modes to the current main branch.
Step 1:
Build the latest main branch of NiFi and visit https://localhost:8443/nifi. Now start the dev server using `nx run nifi:serve:development` Next, open the browser to localhost:4200/nifi. You will want to compare both the default apache theme and the included purple theme from the main branch with the upgraded tokenized default apache theme as well as the upgrade tokenized purple theme. You will want to build some flows and exercise configuring the flow, connections, parameter contexts, controller services, permissions, etc.
There are a few contrast ratio improvements that should be noted here and are expected improvements. In dark mode for the default apache nifi theme the text on the processor details on the canvas as well as in the operation palette, the PG name in the Flow Analysis drawer, the selected border on canvas components, and the bottom bread crumb are all intentional improvements. In the screen shot you can see the improved readability:
<img width="3199" alt="Screenshot 2025-02-19 at 5 02 46 PM" src="https://github.com/user-attachments/assets/c424b59f-3338-4622-b8e8-90f2bb056779" />
Step 2:
There are two motivations to these changes. The first and main motivation was to align with the latest guidance from Angular Material 19 theming. The second motivation of this effort was to leverage system variables/Css properties/design tokens such that when the NiFi UI loads a custom UI in an iframe the theme from the parent application will be set into the iframe application to ensure consistent user experiences when custom UI's are loaded in this manner.
You can test that this is working with the latest changes in this PR by building this PR and starting nifi with the default apache nifi theme. Visit https://localhost:8443/nifi and on the nifi canvas add the JoltTransformJSON and an UpdateAttribute processors to your root PG on the canvas.
For example, the JoltTransformJSON Advanced UI default apache nifi dark mode theme on port `:8443` should look like:
<img width="1746" alt="Screenshot 2025-02-19 at 4 16 38 PM" src="https://github.com/user-attachments/assets/9124a26a-61d5-477b-b39c-844ad086b5a3" />
Notice that the JoltTransformJSON advanced UI is also using the Inter Font as well as the default apache nifi dark mode theme.
Then, using `nx run nifi:serve:development` the reviewer should be able to start the dev server. Next, open `nifi-frontend/src/main/frontend/apps/nifi/src/styles.scss` and on line 67 change the dev server to the included purple theme. Open the browser to localhost:4200/nifi. and again open either of the advanced UI's for the JoltTransformJSON or the UpdateAttribute processors.
For example the JoltTransformJSON Advanced UI purple dark mode theme on port `:4200` should now look like:
<img width="1752" alt="Screenshot 2025-02-19 at 4 20 48 PM" src="https://github.com/user-attachments/assets/828c7be1-e19c-456c-9244-8e528f3908cf" />
Notice that the purple them is now displayed for these custom UI's without requiring a full rebuild of the extension.
Winning! :) This same experiment with current main branch and without system variables/Css Properties/Design Tokens would not work!!!
# Tracking
Please complete the following tracking steps prior to pull request creation.
### Issue Tracking
- [X] [Apache NiFi Jira](https://issues.apache.org/jira/browse/NIFI) issue created
### Pull Request Tracking
- [X] Pull Request title starts with Apache NiFi Jira issue number, such as `NIFI-14258`
- [X] Pull Request commit message starts with Apache NiFi Jira issue number, as such `NIFI-14258`
### Pull Request Formatting
- [X] Pull Request based on current revision of the `main` branch
- [X] Pull Request refers to a feature branch with one commit containing changes
# Verification
Please indicate the verification steps performed prior to pull request creation.
### Build
- [ ] Build completed using `mvn clean install -P contrib-check`
- [ ] JDK 21
### Licensing
- [X] New dependencies are compatible with the [Apache License 2.0](https://apache.org/licenses/LICENSE-2.0) according to the [License Policy](https://www.apache.org/legal/resolved.html)
- [X] New dependencies are documented in applicable `LICENSE` and `NOTICE` files
### Documentation
- [X] Documentation formatting appears as expected in rendered files
--
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-14258] tokenize theme [nifi]
Posted by "rfellows (via GitHub)" <gi...@apache.org>.
rfellows commented on code in PR #9733:
URL: https://github.com/apache/nifi/pull/9733#discussion_r1964015193
##########
nifi-frontend/src/main/frontend/libs/shared/src/assets/themes/apache-nifi.scss:
##########
Review Comment:
The date picker is different now than it was prior to the change. The background color of the date range in particular. Not sure if this was intentional or not, just calling out the difference.
Before:
<img width="307" alt="Screenshot 2025-02-20 at 11 51 07" src="https://github.com/user-attachments/assets/7de51469-b30b-4635-9c7e-b37deb023285" />
After:
<img width="318" alt="Screenshot 2025-02-20 at 11 51 14" src="https://github.com/user-attachments/assets/e572fe6e-ae43-4b21-9268-498c69435ff8" />
Looks like in dark mode this can be solved by:
```
--mat-datepicker-calendar-date-in-range-state-background-color: var(--md-ref-palette-secondary-30);
```
Light mode has the same problem. I didn't experiment with trying to solve that one.
--
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-14258] tokenize theme [nifi]
Posted by "rfellows (via GitHub)" <gi...@apache.org>.
rfellows commented on PR #9733:
URL: https://github.com/apache/nifi/pull/9733#issuecomment-2671520018
Will review.
--
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-14258] tokenize theme [nifi]
Posted by "rfellows (via GitHub)" <gi...@apache.org>.
rfellows merged PR #9733:
URL: https://github.com/apache/nifi/pull/9733
--
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-14258] tokenize theme [nifi]
Posted by "mcgilman (via GitHub)" <gi...@apache.org>.
mcgilman commented on code in PR #9733:
URL: https://github.com/apache/nifi/pull/9733#discussion_r1971946124
##########
nifi-frontend/src/main/frontend/apps/nifi-jolt-transform-ui/src/styles.scss:
##########
@@ -28,13 +28,7 @@
@use 'codemirror/addon/lint/lint.css';
@use 'font-awesome';
-@use 'libs/shared/src/assets/themes/material';
-
-// Include the common styles for Angular Material. We include this here so that you only
-// have to load a single css file for Angular Material in your app.
-// Be sure that you only ever include this mixin once!
-@include mat.elevation-classes();
-@include mat.app-background();
+@use '../../../libs/shared/src/assets/themes/material';
Review Comment:
Should we keep the non-relative path here?
##########
nifi-frontend/src/main/frontend/apps/standard-content-viewer/src/styles.scss:
##########
@@ -28,13 +28,7 @@
@use 'codemirror/addon/fold/foldgutter.css';
@use 'font-awesome';
-@use 'libs/shared/src/assets/themes/material';
-
-// Include the common styles for Angular Material. We include this here so that you only
-// have to load a single css file for Angular Material in your app.
-// Be sure that you only ever include this mixin once!
-@include mat.elevation-classes();
-@include mat.app-background();
+@use '../../../libs/shared/src/assets/themes/material';
Review Comment:
Should we keep the non-relative path here?
##########
nifi-frontend/src/main/frontend/apps/update-attribute/src/styles.scss:
##########
@@ -33,13 +33,7 @@
@use 'codemirror/addon/hint/show-hint.css';
@use 'font-awesome';
-@use 'libs/shared/src/assets/themes/material';
-
-// Include the common styles for Angular Material. We include this here so that you only
-// have to load a single css file for Angular Material in your app.
-// Be sure that you only ever include this mixin once!
-@include mat.elevation-classes();
-@include mat.app-background();
+@use '../../../libs/shared/src/assets/themes/material';
Review Comment:
Should we keep the non-relative path here?
##########
nifi-frontend/src/main/frontend/apps/nifi/src/styles.scss:
##########
@@ -64,13 +64,7 @@
@use 'codemirror/addon/hint/show-hint.css';
@use 'font-awesome';
-@use 'libs/shared/src/assets/themes/material';
-
-// Include the common styles for Angular Material. We include this here so that you only
-// have to load a single css file for Angular Material in your app.
-// Be sure that you only ever include this mixin once!
-@include mat.elevation-classes();
-@include mat.app-background();
+@use '../../../libs/shared/src/assets/themes/material';
Review Comment:
Should we keep the non-relative path here?
--
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-14258] tokenize theme [nifi]
Posted by "rfellows (via GitHub)" <gi...@apache.org>.
rfellows commented on code in PR #9733:
URL: https://github.com/apache/nifi/pull/9733#discussion_r1966035575
##########
nifi-frontend/src/main/frontend/libs/shared/src/assets/themes/apache-nifi.scss:
##########
Review Comment:
Ok, thanks for clarifying.
--
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-14258] tokenize theme [nifi]
Posted by "scottyaslan (via GitHub)" <gi...@apache.org>.
scottyaslan commented on code in PR #9733:
URL: https://github.com/apache/nifi/pull/9733#discussion_r1965660396
##########
nifi-frontend/src/main/frontend/libs/shared/src/assets/themes/apache-nifi.scss:
##########
Review Comment:
I dont think we want to override this token. I believe you and I have discussed the styles for this datepicker a few times on previous PRs but in this case I think less customization of the date picker widget theme the easier it is to maintain. You are correct that previous to this PR the date picker had a strange blue green hue colorize background but that seems to have been a misconfiguration of the color hues in the theme palettes. That strange color doesn't seem to be used in any other widgets. Using the standard material design style here is more in line with how selection is displayed in other places in our app like context menu, row hover state in the all of the listing... so yea this is an intentional change here.
--
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