Skip to content

NIFI-12099 Repurpose Template icon in top left bar#7811

Merged
rfellows merged 1 commit intoapache:mainfrom
elcsiga:CFM-3177-Repurpose-Template-icon-in-top-left-bar
Oct 19, 2023
Merged

NIFI-12099 Repurpose Template icon in top left bar#7811
rfellows merged 1 commit intoapache:mainfrom
elcsiga:CFM-3177-Repurpose-Template-icon-in-top-left-bar

Conversation

@elcsiga
Copy link
Contributor

@elcsiga elcsiga commented Sep 29, 2023

Summary

NIFI-12099

import-ftom-registry.mp4

Tracking

Please complete the following tracking steps prior to pull request creation.

Issue Tracking

Pull Request Tracking

  • Pull Request title starts with Apache NiFi Jira issue number, such as NIFI-00000
  • Pull Request commit message starts with Apache NiFi Jira issue number, as such NIFI-00000

Pull Request Formatting

  • Pull Request based on current revision of the main branch
  • 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

  • New dependencies are compatible with the Apache License 2.0 according to the License Policy
  • New dependencies are documented in applicable LICENSE and NOTICE files

Documentation

  • Documentation formatting appears as expected in rendered files

@elcsiga elcsiga force-pushed the CFM-3177-Repurpose-Template-icon-in-top-left-bar branch from a06cae4 to 1f80a5c Compare September 29, 2023 14:00
@sardell
Copy link
Contributor

sardell commented Oct 2, 2023

Reviewing now...

@sardell
Copy link
Contributor

sardell commented Oct 2, 2023

It looks like the draggable icons are no longer centered in their container, which is more obvious when hovering over an icon.

Before your changes:

before

After:
after

@elcsiga elcsiga force-pushed the CFM-3177-Repurpose-Template-icon-in-top-left-bar branch from 1f80a5c to ecf0a20 Compare October 18, 2023 16:44
@rfellows
Copy link
Contributor

reviewing...

@rfellows
Copy link
Contributor

rfellows commented Oct 18, 2023

This is good and works pretty well. The only improvement I could recommend would be to refresh the current-user after configuring or deleting a registry client. As it stands now, we are relying on the background polling of the current-user endpoint to update the current user which is where it is determined if canVersionFlows is true or false. It is this property that determines if we show the No Registry Client available dialog or the Import Version dialog.
If you delete the registry client and then try to drag on a new import before the polling happens, you can be shown import dialog with no registries:
Screenshot 2023-10-18 at 3 35 34 PM

Similarly, if you do the same too soon after configuring a registry client, you can get a stale No Registry Client available dialog even though you have one configured.

Maybe @scottyaslan can provide some more insight into if this is consistent with other areas of the application.

@elcsiga elcsiga force-pushed the CFM-3177-Repurpose-Template-icon-in-top-left-bar branch from ecf0a20 to 59aaf21 Compare October 19, 2023 13:06
@elcsiga
Copy link
Contributor Author

elcsiga commented Oct 19, 2023

@rfellows good point, now adding or removing a registry client also reloads /current-user

Copy link
Contributor

@rfellows rfellows left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good to me 👍
Thanks @elcsiga

@rfellows rfellows merged commit d4014c7 into apache:main Oct 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants