Adding APIs for getting UI elements visibility (and maybe reworking toggles ?)

[Inao0]

Currently, the Feature Methods API offers toggle mechanics for annotations, the reader, tours, tools and measurements visibility. This may be practical to implement toggle buttons, but in any other case, this is ambiguous as there is no way to know the current state of these UI elements, if they are visible or not.

Adding getters for the visibility of UI elements

I would like to add some setters corresponding to these :

Name	Parameters	Description
getAnnotationsVisibility()	None	get the visibility of annotations
getReaderVisibility()	None	get the visibility of the article reader
getToursVisibillity()	None	get the visibility of the tour UI
getToolsVisibillity()	None	get the visibility of the article UI
getMeasurementVisibillity()	None	get the visibility of the measurement tool panel UI

Regarding setters for the visibility of UI elements

I see three options :

1. Keeping only the toggle APIs. This leads to the least amount of modification. It is less practical to set the visibility of a given element : It is up to the user of the API to check the current value before toggling it or not.
2. Keeping the toggle APIs and adding setters. This leads to more code, toggle could be rewritten to use the setter method. It is more practical to set the visibility of a given element while maintaining backward compatibility.
3. Replace the toggle API by setters. This is more practical to set the visibility of a given element but breaks backward compatibility.

I can work on any of these options but thought it would be best to tackle it at the same time as adding getters, hence this discussion before proposing a pull request.

[gjcope]

This may be practical to implement toggle buttons, but in any other case

Could you give an example of another case?

[Inao0]

To set the visibility to a given value.
Let's say I want to make sure the annotations are off, with the API, I currently have to rely on external knowledge to know if the annotations are visible are not at this point in time. I cannot just switch it off. I cannot query the current visibility.

[gjcope]

Without context that could still be a toggle. Are you saying there is a use case where you are providing users with custom UI to set annotation visibility as well as maintaining the native annotation UI? Or are you saying the use case is to externally script non-user controlled visibility (while maintaining native UI)?

[gjcope]

Per our off-thread conversation, I think option 2 is best of the above. One note, we might want to avoid using 'visibility' as there has been some confusion in the past with end users (and occasionally in code) over enabling features (annotations on/off, etc.) vs the visibility of the feature UI (annotation button visible, etc.).

[Inao0]

I don't mind using another word, though I have trouble thinking about something clearer than "visibility". I could write some comments to make it clear this is not about UI visibility but whether the feature is enabled.

state could be an option. So for example, for annotations, we would have :

Name	Parameters	Description
toggleAnnotations()	None	Toggle the visibility (unchanged)
getAnnotationState ()	None	Returns if the annotations are visible
setAnnotationState (state)	True, False	Sets whether annotations are visible are not

But without looking at the documentation, I would tend to expect annotations properties such as their positions, colors, etc...

Another idea might be using enable, but then setters and getters don't start by "set..." and "get...." which is not coherent with the rest of the API:

Name	Parameters	Description
toggleAnnotations()	None	Toggle the visibility (unchanged)
isAnnotationEnabled ()	None	Returns if the annotations are enabled
enableAnnotations (state)	True, False	Sets whether annotations are visible are not

Last one could have a default state value of true

[ndejardin425]

Hello,

Jumping into the conversation but I also need a function to be sure that the "visibility" of a feature is set to a specific value. Since that's what I already done for annotations in #445, I'll go with "enable..." functions described by @Inao0.

[ndejardin425]

on second thoughts, I think that doing enableAnnotations(false) seems weird,
so maybe enableAnnotations() and disableAnnotations() ?

[ndejardin425]

started with #448

[gjcope]

@ndejardin425 How about setAnnotationsEnabled(bool)? Maybe that's less awkward for false since it reads more like a property?

[ndejardin425]

setAnnotationsEnabled seems ok to me