tapestry-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Kalle Korhonen <kalle.o.korho...@gmail.com>
Subject Re: Extending the mixin concept (just a bit)
Date Mon, 08 Aug 2011 19:55:44 GMT
Howard, and anybody else interested in this: can you comment on the
@EmbeddedMixin concept? Dragan's tried out several different
approaches and the embedded mixin is the least obtrusive one and ready
for inclusion to tapestry core in my opinion. I instructed Dragan to
create a an issue and a patch separately for it so it'd still make it
into 5.3. The risk is minimal and allows one to implement some
interesting mixins, such as Dragan's context menu that can be applied
to grid cells.

Kalle


On Fri, Aug 5, 2011 at 12:15 PM, Howard Lewis Ship <hlship@gmail.com> wrote:
> I've been at a client site all week doing training and haven't looked
> at this, or many other things in my queue.
>
> On Fri, Aug 5, 2011 at 8:18 AM, dragan.sahpaskix@gmail.com
> <dragan.sahpaskix@gmail.com> wrote:
>> Hi,
>> I have implemented this feature which can be seen in this
>> commit<https://github.com/dragansah/tapestry5/commit/eb1232939025ea4df6359dd06987edbfa17e8a47>on
>> my github fork of tapestry. I will now describe how it is used and
>> would
>> like to get some feedback on weather it is worth supporting it in order to
>> continue working on it (no tests and needs further work and testing but it
>> works for all the examples i tried).
>>
>> A mixin can have embedded mixins defined with an annotation @EmbeddedMixin.
>> Example:
>>
>> usage in a page:
>> <t:grid t:mixins="mixinsForGrid" />
>>
>> 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 phases on gridCell
>>
>> void beginRender()
>> {
>>
>> }
>>
>> }
>>
>> Here are the implementation details:
>>
>> Public api
>> new EmbeddedMixin annotation
>> new method in ComponentModel: List<String> getEmbeddedMixinClassNames()
>> new method in ComponentModel  String getComponentIdForEmbeddedMixin(String
>> mixinClassName)
>> new method in MutableComponentModel  void addEmbeddedMixinClassName(String
>> mixinClassName, String embeddedComponentId, String... order);
>>
>> Private API
>> new EmbeddedMixin worker
>> added method in PageLoaderImpl to add the embedded mixins in the components
>> new method in EmbeddedComponentAssemblerImpl: public
>> List<String> getMixinClassNames()
>> implementation to the new methods in MutableComponentModelImpl
>>
>> I think the feature will give greater power to the mixins, because a mixin
>> will now have insight to child components of the component it applies to.
>> This can be potentially dangerous if the component's structure is meant to
>> be a private implementation of the component, but IMO it rarely is the case
>> where a component once released, has significant changes in the tml
>> template.
>>
>> Cheers,
>> Dragan Sahpaski
>>
>>
>>
>> On Thu, Aug 4, 2011 at 1:01 AM, Kalle Korhonen
>> <kalle.o.korhonen@gmail.com>wrote:
>>
>>> I'm afraid that Dragan might have been a little too exhaustive
>>> describing the issue :) The meat is at the end of his post - what if
>>> you could invoke mixin's methods not only on the invocation of the
>>> component's own render methods but also its child component render
>>> method invocations? You could also think of it as a mixin applying
>>> mixins to the child components of a component. The documentation
>>> clearly states that "mixins may not, themselves, have mixins"
>>> (http://tapestry.apache.org/component-mixins.html), but is this a
>>> technical limitation or a design decision? It could be useful whenever
>>> you have a well-defined composite component, like with Grid,
>>> BeanEditor, etc. Dragan's use case makes sense to me, but I'm
>>> wondering if anybody can reason why this might not be such a good idea
>>> or see any better solutions for the problem.
>>>
>>> Kalle
>>>
>>>
>>> On Sat, Jul 30, 2011 at 2:10 AM, dragan.sahpaskix@gmail.com
>>> <dragan.sahpaskix@gmail.com> wrote:
>>> > Dear Developers,
>>> > As part<
>>> http://www.google-melange.com/gsoc/proposal/review/google/gsoc2011/dragansah/1
>>> >of
>>> > GSOC 2011 I'm doing (as well as some other things) a contextmenu mixin
>>> > that should work with the t5 grid. The mixin works fine on other
>>> components
>>> > but the t5 grid is tricky because I want to apply a context for the menu
>>> for
>>> > each cell (and row) i.e. provide a way to show different content based on
>>> > the cell (and row) value.
>>> > I have read threads on the list where when someone is trying to extend a
>>> t5
>>> > component like the grid with something the grid was "not intended to do",
>>> > the comment on the list can be make a new component etc. but IMO it would
>>> > be awesome if t5 allowed doing things like this.
>>> >
>>> > I have the t5 grid integration with the contextmenu mixin working, but it
>>> > involves some changes in GridCell and AbstractPropertyOutput, and a
>>> change
>>> > that introduces another GridOutputContext service (similar to
>>> > PropertyOutputContext) that don't break anything (tests don't fail) but
>>> are
>>> > only there for components like the contextmenu that want to pull cell
>>> data
>>> > from the grid rendering cycle, to be used outside in a mixin for the grid
>>> > or similar. I want to make a patch for the contextmenu component but IMO
>>> you
>>> > (the developers) won't be so thrilled in commiting such changes, and I
>>> can
>>> > say I would agree with you if this was the case.
>>> >
>>> > So please read on while I make a proposal for extending the t5 concept of
>>> > mixins just a bit.
>>> >
>>> > In t5 the semantics of the components is generally  exposed as @Parameter
>>> s,
>>> > public methods, render cycles and events. While this component model is
>>> > very convenient and powerful it doesn't (out of the box) have a way to
>>> > expose the structure of the component to the outside world. OK you can
>>> argue
>>> > that what's inside the component is a private implementation, and should
>>> not
>>> > be changed by an outside component for example, but please consider this
>>> > example: The t5 grid component has gridRows and gridCells. This simple
>>> fact
>>> > about grid's structure should not change (IMO) and should be possible to
>>> be
>>> > exposed outside the grid, for components or mixins (like the contextmenu)
>>> > can use it.
>>> >
>>> > Not to be too long I don't have some ground breaking suggestion for some
>>> > radical change but I have a simple suggestion, that if I get positive
>>> > comments from the devs would like to work on right away.
>>> >
>>> > I suggest that a mixin applied on a component should be able not only to
>>> > have the component's render phase events, but also have the render phase
>>> > events from embedded components in that component.
>>> > Example:
>>> > Let's have <t:grid t:mixins="contextmenu" />
>>> > ContextMenu.java (mixin)
>>> >
>>> > void afterRender() {
>>> > ...
>>> > }
>>> >
>>> > // not possible now
>>> > void afteRenderFromGridCell(){
>>> > ...
>>> > }
>>> >
>>> > I guess one downside would be that users start making mixins that
>>> > use embedded components that change (or get removed) in the future, but
>>> > maybe we can control this by having something like
>>> > @ExposeRenderEvents(GridCell.class, GridRows.class) or something similar.
>>> >
>>> > Does anyone consider this extension worth having?
>>> >
>>> > Cheers,
>>> > Dragan Sahpaski
>>> >
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
>>> For additional commands, e-mail: dev-help@tapestry.apache.org
>>>
>>>
>>
>
>
>
> --
> Howard M. Lewis Ship
>
> Creator of Apache Tapestry
>
> The source for Tapestry training, mentoring and support. Contact me to
> learn how I can get you up and productive in Tapestry fast!
>
> (971) 678-5210
> http://howardlewisship.com
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
> For additional commands, e-mail: dev-help@tapestry.apache.org
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org


Mime
View raw message