Refactoring Proposals

[crossrulz]

I have a working branch with the following changes as a refactoring effort.

1. Move the front panel controls so that the origin is (0,0). This would allow for using the "Scroll panes to origin at run time" feature in the Window Run-Time Position of the VI Properties. It would also help fix accidental scrolling as the runtime would not care anymore. Other than the moving of the controls, the Updated Coordinates.vi needed updating to account for the picture controls being moved.
   
   

2. I found the references to the front panel's BW Icon In and Color Icon In clusters are not actually used in any code for the Icon Editor other than initialization. The proposal here is to remove these two references from the References cluster, simplifying the mother cluster at the same time, and pass in the Color Icon In value into the initialization VI.
   
   

My current branch also includes moving the unit tests into a Test folder as I suggested in #210

[RobustoSystems]

1. "Move the front panel controls so that the origin is (0,0)." - Fully agree. FWIW I do this with all user-facing GUIs I create in my day-to-day work.
2. Suggestion: The first screenshot under point number 2 shows a Bundle node. For increased code readability it might be best to use a Bundle By Name node. FWIW I always try to use BBN nodes rather than Bundle nodes, due to their increased readability.
3. Suggestion: The second screenshot under point number 2 shows that the error out terminal of the FP.Open invoke node is NOT wired. Any/all error out terminals SHOULD BE WIRED. For example, a Merge Errors node could be used. Any/all errors should propagate down the error wire until they reach the relevant error handling code.

[crossrulz]

Re point 2: I also much prefer Bundle By Name. Unfortunately, it greatly increased the diagram space required. I do have a goal of reducing what is required to go into that cluster. Once it is shrunken enough, I would be more than happy to switch to a BBN.

[crossrulz]

Re point 3: I would arrange it to just have the FP.Open error be in series, therefore not needing a Merge Errors.

[crossrulz]

I went ahead and used the Bundle By Name. It wasn't as bad as I remembered when I tried previously. It will definitely help readability. It already lead me down a rabbit hole of a few more things I think I can eliminate from this cluster. But that can be saved for another effort.

[j-medland]

Can co-ordinates correction take its offset from some reference/programmatically to support any future positioning changes?

[RobustoSystems]

I agree that this solution would increase the flexibility of the code. And it can be achieved relatively easily.

My only concern is that this part of the code executes as part of launching the Icon Editor. Adding this functionality may slow down the launching of the Icon Editor by perhaps 100 ms or more. It may be worth sacrificing flexibility (keeping the current implementation) in favour of increased launch speed.

[RobustoSystems]

4. Suggestion: It might be best to display the label (Visible Items >> Label) of the typedef cluster constant whose header is "ICON". In the first screenshot of point number 2.
5. Suggestion: The icon of the aforementioned typedef cluster could be improved. It could contain three lines of text that represent the name of the typedef cluster.

[crossrulz]

I am more inclined to update the icon over adding a label. I am not immediately seeing a good reason to do both of these suggestions.

[svelderrainruiz]

@eyesonvis we are planning on refactoring some portions of the icon editor, this is the first from a series of discussions, can you please comment on this specific discussion? we would like to open for contribution some of the action item from this discussion.

[eyesonvis]

I don't think anything here affects the end user functionality, so I don't have much to contribute. I don't object to any of the suggested changes.

[crossrulz]

This effort was completed with #229