Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
NIFI-14319: Create initial project structure for new NiFi Registry UI #9855
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
NIFI-14319: Create initial project structure for new NiFi Registry UI #9855
Changes from 13 commits
96bb176
c9f9879
df16191
cb1e628
cd163ae
c888d1c
9ded513
f96a040
063e999
2ad3b38
74df539
a821539
cecaa80
c163d72
992ea4c
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are no styles here. I think you can delete this file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems odd to me that we would need two different header components (one for NiFi and now one for NiFi Registry). It would be nice if the fd-header could be generalized and moved into the shared lib. This is not a requirement for this PR but really should be considered.
I also see some theming issues:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll be sure to update the text color to use secondary-contrast. In regard to the svg, I think it might be time for a logo refresh? Perhaps it could include the name like NiFi's logo and I can remove this not-so-great-looking header. As a temporary solution, I could override the theme to utilize the old header color in light mode. What do you think?
With regard to using a shared component for the header, I'm very much in support of this. I think we could create a very simplistic component which leverages Angular's content projection, but I'm open to different ideas. The approach I wanted to take with this big piece of work is utilize shared components that currently exist, and make note of components that should be created and shared to be addressed after migrating the Registry UI. How does that sit with you? I can track it as a separate epic in Jira, or I can create tickets in this epic and address them once I have most of the application migrated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The point here is that the text color needs to pass contrast ratio with the background color in both light and dark mode.
One option here for the NiFi Registry logo is that we could wrap it in a div or span and give it the legacy Registry header background color and some rounded border radius:
This would work in both light and dark mode and not require a new logo.
As for you thoughts on a shared component: Yes I think leveraging Angular's content projection is a good path to explore. I was thinking you could just move the header from nifi into the shared lib. If you want to do this in a follow on effort I am supportive of that but we need a way to track this work. We need an epic or at least a parent jira with sub-tasks. This way would make it easier to break the work up so others may contribute and it should also lead to smaller more manageable PRs. Big PRs like this are difficult and time consuming to review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the logo suggestion! I'll give that a try.
I have a Jira task open with subtasks (NIFI-14319 is one of them). The tickets are broken down by views. I don't believe the remaining PRs will be nearly as large as this one initializing the project, but I also now know that breaking down by view is not granular enough. I'll work on breaking these down further asap.
I'd rather address shared components separately to avoid having to deal with rebase conflicts in a PR like this one. Having a dedicated PR for promoting a handful of components from NiFi into the shared lib should be a very straightforward refactor and shouldn't take nearly as long to review.