mcgilman commented on PR #7591: URL: https://github.com/apache/nifi/pull/7591#issuecomment-1747434037
@markobean Thanks for the PR! @exceptionfactory @mtien-apache Thanks for the reviews! I just wanted to chime in here to provide some additional background that may help the direction we proceed here. The Process Group dialog used to be similar to other dialogs in terms of size and input field placement. However, when we introduced fine grain access controls we also added support for Process Group scoped Controller Services. Previous to that there was only a single collection of Controller Services that were available for all components. Because we introduced Process Group scoped Controller Services, we duplicated this table in the Process Group dialog. The previous size of the dialog didn't accommodate the full table and we moved this to a full-screen dialog. This Controller Service table is something that I believe we should probably consider now when it comes to the Comments tab and the Apply button. The Apply button was only on the General tab because changes to the Controller Services are not saved when the user presses Apply. They are saved as the user Adds, Edits, Enables, Disables, and Deletes the Services directly in the Controller Service tab. This is also why the dialog doesn't close automatically after Applying changes to the General tab as the user may still want/need to make changes on the Controller Service tab. Because of this, I would avoid placing the Apply or Cancel buttons on the Controller Service tab. Personally, I would vote for not having a separate Comments tab. While it's nice that there is some alignment with other component configurations, I worry it would leave the Process Group configuration a little confusing. We'd have 3 tabs where only two have an Apply button and pressing the Apply button would save changes made on both but not the other. Leaving the basic behavior in place where there are two tabs, General (with the Apply button) and Controller Services (where changes are saved automatically), would be the least surprising to users and would also eliminate a lot of the scope creep mentioned earlier. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
