search

icfnext / cq-component-maven-plugin

Other
22 stars 35 forks source link

TouchUI Checkbox Annotation Fixes #22

Closed alexlockhart7 closed 8 years ago

alexlockhart7 commented 8 years ago

Changed the TouchUI CheckBox annotation in two ways:

  1. Added the "value" attribute to the annotation so that the touch ui dialog xml will have the attribute. This way it is possible to choose the "checked" value of the checkbox, defaulting to "{Boolean}true".
  2. Removed the "checked" attribute from the annotation so that the touch ui dialog xml does not have the attribute. This attribute being set to either true or false implies that "ignoreData" is true. I do not believe that this is what we want from the checkbox fields.

The usage of the different attributes in the touch ui checkbox xml is a bit awkward and unexpected. So if you feel that I might be wrong about this, read the documentation carefully. However I think that we want to set the "value" attribute instead of the "checked" attribute, which is what this pull request does.

Relevant documentation:

Granite Checkbox Documentation CRX path to checkbox JSP: /libs/granite/ui/components/foundation/form/checkbox/checkbox.jsp

pmichelotti commented 8 years ago

Two questions / update requests and then I can merge this in:

1) Is there a reason we need to add value as an attribute to the Checkbox annotation? Can the one tied to DialogField not be used for this purpose? 2) While I agree that it would be rarely used in the Touch UI it would be nice to keep the checked option. Can we change the annotation such that it takes a list of booleans? That way we can actually differentiate between the user not setting the attribute and setting it to false. An empty list should cause the property not to be added to the dialog output. A list with one or more elements should cause the property to be added to the dialog output and its value should be the value of the first element in the list. If the list contains more than one element a warning should be logged indicating we only care about the first element.

alexlockhart7 commented 8 years ago

@pmichelotti Both of your suggestions have been added.