usnistgov / iheos-toolkit2

XDS Toolkit
https://ihexds.nist.gov
45 stars 25 forks source link

Cosmetic Issue: Labels for Actors in Conformance Tests #545

Closed stl-steve-moore closed 2 years ago

stl-steve-moore commented 2 years ago

When you load the Conformance Tests tab, you see a list of Actors with profiles and options under the actors. You can control the labels on the profiles and actors by modifying ConfTestsTabs.xml, but you cannot change the label on the actor. Any change made in ConfTestsTabs.xml is ignored, and a value that is compiled is used instead.

It would be a nice feature to be able to use the "label" that is in ConfTestsTabs.xml for the display. I understand that the tcCode still needs to refer to something that is defined in java code and compiled. The most recent Sequoia work uses different names for two IHE actors, but the behavior is the same.

I spent some time on this. If I understood the front end stuff better, I would be able to make the change myself. This really is a "nice to have". The software still works as is, and having the ability to label the profiles and options already provides a great deal.

skbhaskarla commented 2 years ago

There is some documentation about this behavior in ConfTestsTabs.xml: Temporary labels: At the actor level, the Tab Label attribute value is for tab organization purposes only. Before this Tab file was introduced, the value of testkit\actorcollections[tcCode].txt is used as the tab name. To preserve this feature, the actor label is reset to the value that of the text file. See "tabConfig.setLabel(tcd.getCollectionTitle());".

It seems like commenting off the setLabel method call as shown above in ToolkitServiceImpl#getPrunedToolTabConfig will achieve the desired effect. This also means that ConfTestsTab.xml will always be the source of the actor label.

Another more flexible approach would be to remove the XML label attribute on the actor tab. In this case if no label is present, we pull the actor description from the compiled code. Otherwise, if the label attribute is present, we use that instead. How does this sound?

stl-steve-moore commented 2 years ago

The proposal to remove the label as default and rely on the compiled code and then use the label if present sounds like a great idea!

skbhaskarla commented 2 years ago

Fix is in branch fix/confTestsActorLabel

stl-steve-moore commented 2 years ago

I did a test, and it operates like I expect. Thank you for putting this in.