search

icfnext / cq-component-maven-plugin

Other
22 stars 34 forks source link

Property annotation change to select touch, classic or both #40

Closed alexlockhart7 closed 8 years ago

alexlockhart7 commented 8 years ago

The Property annotation will now be able to select whether it will be generated for touch, classic or both. It will default to both.

alexlockhart7 commented 8 years ago

@pmichelotti This is ready to be reviewed.

michaelhodgdon commented 8 years ago

The Property class is used elsewhere where this option doesn't make sense. We should either split property into 2 classes (one the is generic and one with the touchui/classicui choice) or document in the javadoc where the choice works.

alexlockhart7 commented 8 years ago

The Property annotation javadoc says:

Defines a property to be written to a dialog widget's XML node in the Component's dialog.xml.

@michaelhodgdon Would an example of it being "used elsewhere where this option doesn't make sense" be the renderConditionProperties field in the Tab Annotation, where this is already labeled for touch ui only?

Expanding on the two options you stated, we could extend the Property annotation with an annotation specific to the additionalProperties field. This would give a little bit of normalcy by avoiding a property that is sometimes irrelevant. However as far as I see it this would require a major version update.

Specifying the use case of an annotation field is prevalent in the component maven plugin, and no matter what we do it is the class that uses the annotation that has to enforce the functionality (the Property annotation never knows it's context). Because of this I think the better option is to simply specify in a comment that the renderIn field is only used in the context of the additionalProperties field of the DialogField annotation.

alexlockhart7 commented 8 years ago

I went ahead and added information to comment for the renderIn field. This is ready for review again.