[jira] [Commented] (TAP5-1606) EmbeddedMixin concept

["Howard M. Lewis Ship (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/TAP5-1606?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13084434#comment-13084434 ] 

Howard M. Lewis Ship commented on TAP5-1606:
--------------------------------------------

I significantly dislike this.  Component structure is supposed to be a black block; this annotation effectively tears the structure of a component apart to allow the mixins to be applied.

Even your example shows the problems this will cause:  Any change to the Tapestry implementation of Grid (something that's long overdue) will break these embedded mixins, and result in backwards compatibility problems.

The right solution for large components, such as Grid, is to implement them using sub-components that can be reassembled differently for customization purposes. Grid is long overdue for such a makeover, and the template overrides introduced in 5.2 (or maybe 5.1?) were put in place for this reason.  Further, Grid itself needs to do a better job communicating with its embedded components via a Environmental object rather than lots of parameters, which would also faciliate swapping different components into place via template overrides (once Grid.tml is refactored that way ... and perhaps an AbstractGrid base class is introduced).

So I'm -1 on this change. I wouldn't be happy with Java code that downcast from an interface to an implementation class and then used reflection to modify its fields ... that's pretty much what we have here. 

> EmbeddedMixin concept
> ---------------------
>
>                 Key: TAP5-1606
>                 URL: https://issues.apache.org/jira/browse/TAP5-1606
>             Project: Tapestry 5
>          Issue Type: New Feature
>          Components: tapestry-core
>    Affects Versions: 5.3
>            Reporter: Dragan Sahpaski
>            Assignee: Kalle Korhonen
>            Priority: Minor
>              Labels: mixin
>             Fix For: 5.3
>
>         Attachments: EmbeddedMixin patch.diff
>
>
> The EmbeddedMixin concept allows applying mixins on embedded components of an existing component.
> Usage scenario:
> A mixin can have embedded mixins defined with the new @EmbeddedMixin annotation.
> A motivational demo can be found here http://dragansah.com/demoapp/embeddedmixinexamples
> Usage example:
> <t:grid t:mixins="mixinForGrid" />
> public class MixinForGrid
> {
>         // apply the MixinForGridCell mixin to the rows.gridCell subcomponent of the grid
>         @EmbeddedMixin(componentId="rows.gridCell")
>         private MixinForGridCell mixinForGridcell;
>         // render phases on the grid
>         void beginRender()
>         {
>         }
> }
> public class MixinForGridCell
> {
>         // render phase on gridCell
>         void beginRender()
>         {
>         }
> }

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira