icfnext / cq-component-maven-plugin

Other
22 stars 35 forks source link

Feature: Tab Title Association Support #34

Closed eppsjt closed 8 years ago

eppsjt commented 8 years ago

This is a new feature allowing for dialog fields to be associated to a dialog tab by tab title.

Currently dialog field to tab association can only be done via the tab index (number 1-N).

The purpose of this feature is to allow for an inheritance model where parent interfaces or classes define dialog fields that are meant to be shared between multiple components. Using tab index for this purpose does not scale well and is not possible when different components have different numbers of tabs.

Example Use Case We currently use this functionality to share 'Analytics' and 'Background' fields with components. Each component is free to declare as many of its own tabs as necessary (thus making tab index unreliable) yet can also inherit dialog fields for 'Analytics' and 'Background' by declaring the tabs in the @Component annotation and extending/implementing the appropriate Analytics and Background interface/class.

Implementation notes:

michaelhodgdon commented 8 years ago

This is actually very similar to the functionality that we removed in favor of using the index. Is there a reason DialogFieldOverride can't be used for your use case?

eppsjt commented 8 years ago

If my understanding of the DialogFieldOverride annotation is correct, these are slightly different use cases. Though the DialogFieldOverride annotation could be used to accomplish the same end, it is not well suited for this scenario.

For instance, the most cumbersome example is an Image tab. An Image tab may have the following fields:

An ImageMixin (or ImageTrait) interface contains these 7 fields and a Component class implements the interface. The goal is to place all of these fields in the same 'Image' tab for every component that makes use of a primary image.

Using the DialogFieldOverride annotation would require every Component class using an image to override/re-declare all 7 methods and for each re-define the appropriate tab() index.

The DialogFieldOverride approach is feasible when dealing with a small number of inherited fields, however, when the same component inherits fields and tabs for Image, Background, Rich Text, Analytics, and so on, the number of DialogFieldOverride annotations per component could be 20+ and becomes error prone and difficult to maintain.

Tab title provides a very clean approach in that a Component class simply declares a tab with the same name (ideally stored in a public static final String constant).

What are your thoughts on this?

michaelhodgdon commented 8 years ago

This appears to be a use case for DialogFieldSet. Using that you would put all the fields in a single object (which seem to be how you are logically grouping them now).

We went away from the using tab titles to define fields after version 1 and I am opposed to going back to it as it was somewhat error prone.

drewglass commented 8 years ago

@michaelhodgdon Why are tab titles prone to error?

eppsjt commented 8 years ago

After doing some experimentation, the DialogFieldSet approach is a good intermediate approach.

For Touch UI, this requires adding support for the new Touch UI Collapsible widget, but this can be done outside of the core plugin code.

Thanks for the suggestion :+1: