-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
NIFI-12425: Controller Service Listing #8091
Conversation
Will review. |
- Controller Service Listing. - Adding lazy loading to the Canvas with the introduction of the Controller Service listing. - Reorganizing existing components in the Flow Designer. - Allowing the current Process Group to be configured. - Inline Service creation.
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.
Just a couple of minor things and something to think about in the future.
.../nifi-web-frontend/src/main/nifi/src/app/pages/flow-designer/feature/flow-designer.module.ts
Outdated
Show resolved
Hide resolved
.../nifi-web-frontend/src/main/nifi/src/app/pages/flow-designer/feature/flow-designer.module.ts
Outdated
Show resolved
Hide resolved
<breadcrumbs | ||
[entity]="serviceState.breadcrumb" | ||
[currentProcessGroupId]="serviceState.processGroupId"></breadcrumbs> |
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.
probably for a future discussion, but i was thinking that based on the context this breadcrumb is used in... one could argue that it shouldn't navigate you to the canvas with the selected pg loaded but rather to the controller services listing for the selected breadcrumb pg.
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.
Good question. Happy to discuss it further. One thing to consider is that the breadcrumbs list all ancestor Process Groups. The listing for the current Process Group already contains all Controller Services for the current Process Group and all ancestor Groups. The user can also navigate to a desired service directly.
That said, I agree that it may feel more intuitive for the breadcrumb link to keep the user in the service listing. Let's evaluate when we get around to improving the layout and breadcrumb styling. Will be handled in a child task of [1].
- Removing unused imports.
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.
Great work, @mcgilman.
NIFI-12425: