Skip to content
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

Automatically detect mismatched banners on VIs part of LVLIBs #40

Open
svelderrainruiz opened this issue Oct 8, 2024 · 10 comments
Open
Assignees
Labels
enhancement New feature or request

Comments

@svelderrainruiz
Copy link
Collaborator

This issue stems from a comment contained on #19

Quote:
_Optional feature extension:
During its loading/start-up procedure the Icon Editor could detect if the VI/CTL has the "wrong" banner (banner does not match current owner banner). If that is the case, the Icon Editor could display a two button dialogue containing a message such as:

"This VI does not use the banner of its owner, "name of owner here". Would you like the banner to be applied?"

If the user selects "Yes" (or hits Enter), the Icon Editor would execute the "Import Icon from Owning Library..." action for the user.

Benefits:

This functionality would act like a mini VI analyser. It would help catch this issue.
Clicking Yes (or hitting Enter) would be even quicker than executing the keyboard shortcut.
Downsides:
Performing the check may slow down the start-up sequence. This may be a significant drawback.

Also enable a checkbox to "Always Apply".
_

@svelderrainruiz svelderrainruiz self-assigned this Oct 8, 2024
@svelderrainruiz
Copy link
Collaborator Author

svelderrainruiz commented Oct 9, 2024

image

I am thinking on having 2 checkboxes for this.

  • Enable automatic banner mismatch detection: Enables or disables the feature that automatically checks mismatches between owning library and VI when VI Icon Editor loads. Having this option enabled and having a mismatch between library banner and VI banner would trigger a popup that gives you the choice to update the banner with the banner from the owning library.

  • Always apply owning banner: This option remains hidden if "Enable automatic banner mismatch detection" is disabled. Enabling this option would automatically apply the owning library banner when you open the icon editor.

@stagg54
Copy link

stagg54 commented Oct 9, 2024

sounds reasonable.

@crossrulz
Copy link

I think it would nice to have the Icon Editor detect if the owning library's icon was updated and put a warning in a window status text or similar. This would be not in your face and force you to update the icon. To remove the notice, one just needs to update the NI_Library layer via the menu or shortcut.

@svelderrainruiz
Copy link
Collaborator Author

@gregr-ni @dnattinger do you believe this is a change that can be brought back into the product if i do it?

@gregr-ni
Copy link
Collaborator

gregr-ni commented Oct 9, 2024

The goal is to have the shipping icon editor be built from github, so everything that happens here should be pulled back into the product. We don't have that process automated yet though.

@svelderrainruiz
Copy link
Collaborator Author

svelderrainruiz commented Oct 9, 2024

These are the 4 functions that i am going to develop:

1 - Enable automatic banner mismatch detection:

  • Enables or disables an automatic check between the VI Icon layer called "NI_Library" and the icon of the owning library on icon editor load.

image

  • If there is a mismatch, a 2 button dialog gives the user the choice to automatically update layer "NI_Library" of the current VI to the icon of the owning library.

2 - Auto import icon from owning library:

  • This option is disabled and grayed out if "Enable automatic banner mismatch detection" is set as FALSE.

image

  • Enabling this option automatically applies the owning library icon to layer "NI_Library" on Icon Editor load.
  • If applied, a small transparent text indicator with the default value "Import icon from owning library has been automatically called. Save the VI to apply these changes"

image

3 - Auto save VI when importing owning library:

  • Saves the VI after automatically after applying the owning library icon to layer "NI_Library" from the current VI.

image

  • If the VI was automatically saved by this function, a small transparent text indicator with the default value "Import icon from owning library has been automatically called. This VI has been saved automatically" gets shown.

image

4 - VIs with any of the following criteria will not be able to use this feature, by automatically disabling it for the course of the lifespan of that icon editor execution

  • VI is located on the LabVIEW install folder.
  • VI is not a part of a library or a project.
  • VI is listed as a dependency.

@svelderrainruiz svelderrainruiz added the enhancement New feature or request label Oct 9, 2024
@chrisb2244
Copy link

chrisb2244 commented Oct 10, 2024

Hi - a few thoughts/comments:

  • What do you mean by "VI is listed as a dependency" (project dependency? I guess to avoid changing code that isn't part of the current project/focused area?)
  • Please don't detect a difference if the layer is the same but hidden - I don't want to be repeatedly prompted just because it's not visible or something (not sure how you plan to code this, or if detecting similarity would be affected by visibility toggles, but just in case) (Likewise, do you plan to address at all the layer height, or just leave it where it is? I guess if there is no NI_Library layer, it would go on top, but if there is a different layer, it would be replaced in the same 'height'?)
  • If possible, a way to say no and not be repeatedly prompted would be nice, but stashing extra info into the VI (e.g. via tags) might be a pain for SCC and potentially confuse other developers who didn't set the 'remember no' tag and then didn't expect a 'wrong' icon without a prompt. Not sure what I'd suggest here, but repeated prompts for the same VI is probably annoying (though maybe pretty niche, if you're detecting specifically a layer titled "NI_Library" like the current implementation).
  • Although it's a different direction to the suggestion here, and I'm not particularly opposed to your proposed change, have you considered instead of a 2 button dialog on open using a separate button to "Apply Class/Library Icon" or similar (probably on the layer page?). This would I think solve the same problem and perhaps not involve too many more clicks (i.e. the same number maybe), but I guess the focus in the modal would make hitting Enter faster...

@crossrulz
Copy link

* Although it's a different direction to the suggestion here, and I'm not particularly opposed to your proposed change, have you considered instead of a 2 button dialog on open using a separate button to "Apply Class/Library Icon" or similar (probably on the layer page?). This would I think solve the same problem and perhaps not involve too many more clicks (i.e. the same number maybe), but I guess the focus in the modal would make hitting Enter faster...

There is already a menu item to apply the library icon and @svelderrainruiz added a shortcut for that item.

Again, I don't like the dialog. A warning status text would be sufficient, in my opinion.

@svelderrainruiz
Copy link
Collaborator Author

Hi - a few thoughts/comments:

> * What do you mean by "VI is listed as a dependency" (project dependency? I guess to avoid changing code that isn't part of the current project/focused area?)

Correct, project dependency. This feature would only work for VIs that are part of a project. This is to avoid accidentally triggering a change on a VI that may already been modified previous to opening the icon editor

> * Please don't detect a difference if the layer is the same but hidden - I don't want to be repeatedly prompted just because it's not visible or something (not sure how you plan to code this, or if detecting similarity would be affected by visibility toggles, but just in case) (Likewise, do you plan to address at all the layer height, or just leave it where it is? I guess if there is no NI_Library layer, it would go on top, but if there is a different layer, it would be replaced in the same 'height'?)

Noted, i will create a unit test that will be able to catch that

> * If possible, a way to say no and not be repeatedly prompted would be nice, but stashing extra info into the VI (e.g. via tags) might be a pain for SCC and potentially confuse other developers who didn't set the 'remember no' tag and then didn't expect a 'wrong' icon without a prompt. Not sure what I'd suggest here, but repeated prompts for the same VI is probably annoying (though maybe pretty niche, if you're detecting specifically a layer titled "NI_Library" like the current implementation).

This is something i would definitely use... ill add this

  • Although it's a different direction to the suggestion here, and I'm not particularly opposed to your proposed change, have you considered instead of a 2 button dialog on open using a separate button to "Apply Class/Library Icon" or similar (probably on the layer page?). This would I think solve the same problem and perhaps not involve too many more clicks (i.e. the same number maybe), but I guess the focus in the modal would make hitting Enter faster...

** As @crossrulz mentioned, i made a new feature that is pending approval from @gregr-ni that allows you to Import owning library icon to current VI by pressing CTRL+I**

@svelderrainruiz
Copy link
Collaborator Author

* Although it's a different direction to the suggestion here, and I'm not particularly opposed to your proposed change, have you considered instead of a 2 button dialog on open using a separate button to "Apply Class/Library Icon" or similar (probably on the layer page?). This would I think solve the same problem and perhaps not involve too many more clicks (i.e. the same number maybe), but I guess the focus in the modal would make hitting Enter faster...

There is already a menu item to apply the library icon and @svelderrainruiz added a shortcut for that item.

Again, I don't like the dialog. A warning status text would be sufficient, in my opinion.

@crossrulz , enabling this option would skip the dialog, and would give you only a warning on the icon editor if the icon editor detects a mismatch between NI_Library layer from the VI Icon and the icon from the lvlib.

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: In progress
Development

When branches are created from issues, their pull requests are automatically linked.

5 participants