[jsr-314-open] composite:insertFacet target facet name

[Ken Paulsen]

Andy Schwartz wrote:
> Ken Paulsen wrote:
>>  
>> Perhaps it's best to explain visually:
>>
>> Page author defines:
>> * Page A
>>    * compComp
>>       * facet "myfacet" : component X  <-- not a child of panelGrid
>>
>> Component author defines:
>> * panelGrid
>>    * facet "caption" : *reference* facet "myfacet"  <-- non-child facet
>>
>> So instead of re-parenting "component X" from "compComp" to 
>> "panelGrid", panelGrid maintains it's relationship that the page 
>> author defines.  Today's behavior does the re-parenting as soon as 
>> you add it to the facet Map.  A proxy-component might be possible 
>> that acts like a "symbolic link" to the page author's component so 
>> that the page author's original component doesn't get moved.
>
> Got it.  The symbolic link analogy is good.
>
>>>>
>>>> If we were to explore not re-parenting, we'd probably need to 
>>>> create a ProxyComponent which could be added to the child and 
>>>> delegate everything to the real component which would remain on the 
>>>> parent.  That might work nicely.
>>>
>>> This sounds fairly close to what composite:renderFacet is doing, right?
>> Yes, but afaik, only for the "rendering" of the component.
>
> Right.
>
>>   For example, if you ask for the "children" of the component put in 
>> as a place holder for rendering, I don't think you'll get the 
>> children defined by the page author... you'll get an empty list of 
>> children b/c there are no children of the placeholder itself.
>
> Yep.
>
>>>
>>> Yep.  So just to make sure I understand where you stand... Are you 
>>> saying that you agree that we could handle these two cases:
>>>
>>> <!-- Insert "foo" facet into the component tree as a facet -->
>>> <composite:implementation>
>>> <h:panelGrid>
>>>   <f:facet name="caption">
>>>     <composite:insertFacet name="foo"/>
>>>   </f:facet>
>>> </h:panelGrid>
>>> </composite:implementation>
>>>
>>>
>>> <!-- Insert "foo" facet into the component tree as a (non-facet) 
>>> child -->
>>> <composite:implementation>
>>> <h:panelGroup>
>>>   <composite:insertFacet name="foo"/>
>>> </h:panelGroup>
>>> </composite:implementation>
>>>
>>>
>>> With a single tag?  But that you prefer that the tag does not 
>>> actually perform re-parenting, ie. that the parent for the inserted 
>>> component is the composite component itself in both cases?
>> Yes, that is what I was trying to say.
>
> Cool.  I feel better now that we've sorted through this a bit. :-)
>
> It seems to me that there are two parts of this problem:
>
> - What the appropriate set of tags are to expose to composite 
> component authors.
> - What the appropriate implementation is for inserting facets 
> components into composite implementation components.
Yes.  Although I believe the 2nd (the behavior) effects what should be 
exposed in the first (interface).  If the behavior (namely timing 
restore-view phase vs. rendering phase; and the re-parenting behavior) 
is so drastically different between these two, the tags should reflect 
that.  If the behavior is the same, there should only be 1 tag.
> As we've seen, the choice of #2 has impact on the page author.
>
> Regarding #1... It seems like we agree that it would be desirable to 
> have a single tag that can perform three operations:
>
> 1. Insert a facet into the composite implementation as a facet of an 
> arbitrary component using the same facet name.
> 2. Insert a facet into the composite implementation as a facet of an 
> arbitrary component using a different facet name.
> 3. Insert a facet into the composite implementation as a direct child 
> of an arbitrary component.
>
> Our current solution supports #1 via <composite:insertFacet>.  We 
> support #3 at least to some degree via <composite:renderFacet>, though 
> there are subtle differences in behavior between this an 
> <composite:renderFacet>.
I agree, we currently support #1 with insertFacet.  However, I disagree 
that we support #3 at all.  This is the omitted use-case which we don't 
support.  I believe <composite:renderFacet> supports #4: "Rendering a 
facet at the specified location".  This is very different as #3 effects 
tree walks, component creation, state saving, clientId's, and more.  #4 
simply tells the "renderer" of the composite component "when" to render 
the user-supplied facet -- which is *exactly* how virtually every 
Java-based component I have seen handles a facet supplied by a page author.
> I believe that #2 is also somewhat covered, though fails in certain 
> cases (such as passing a facet into a nested composite component).  
> These subtle differences are causing confusion about what should and 
> shouldn't work, and I think are a general cause for concern with our 
> current approach.
I don't feel #2 is sufficiently covered at all.
>
> Ideally, we should support #1, #2 and #3 with a single tag, and we 
> would use the same insertion strategy for all three operations.  So, 
> we want an tag contract that is similar to ui:insert (single tag 
> covers all cases), but actual insertion behavior is still open to debate.
I think the insertion behavior needs to be nailed down first.  Then #1, 
#2, #3 can be addressed, or #1, #2, and #4 (and maybe #3?) can be 
addressed.  Otherwise we don't know what the use cases are that we're 
trying to solve.
>
> This is getting somewhat repetitive I know (sorry), but I'll ask one 
> more time...  Can anyone shed some light on why the multi-tag approach 
> is preferable to the single tag approach - ie. is there a reason why 
> we preferred not to follow the ui:insert model of having a single tag 
> address all three of the above operations?  It is possible that there 
> is a good reason, and if so, I would feel more comfortable with our 
> current solution if I knew what that reason was. :-)
As I mentioned above, 1 is a rendering tag which does not effect the 
component tree.  The other effects the component tree.  If that behavior 
stays the same, I am in favor of 2 tags.  If we can change that behavior 
so we essentially have "symbolic links" from the facet map and the child 
list, then we have the same behavior and I'm in favor of 1 tag.
> Regarding the appropriate implementation for inserting facets into the 
> composite implementation (whether as facets or as direct children)...  
> This is a tough call... I understand Ken's concerns about breaking the 
> abstraction for composite component users, though with the 
> introduction of composite:insertFacet it seems that we were willing to 
> accept this behavior (or, perhaps, were unaware of the side effects).
The more I think about the "side-effects" the more I am unhappy with 
what we have... I firmly believe we made a mistake in allowing 
re-parenting to happen.  This diverges composite components from 
Java-based components, and there should be no distinction between the 
two (from the page author's perspective).

While I agree with your "advantages" below, I think the right answer 
just hasn't been proposed.  If you look at the table component, it has 
the same "id" problem -- and solutions exist.  I think repeated id's 
need to be treated in a similar way for composite components when they 
are inserted more than 1 time (now we're getting into the "fun" impl 
details that I said Ryan or someone else would enjoy ;) ).  This would 
negate the "advantages" of a re-parenting solution, while retaining the 
opaqueness of the component.
> One advantage that re-parenting has over referencing (symbolic link 
> approach) is that re-parenting works with repeated component trees.  
> That is...
>
> This works:
>
> <composite:implementation>
>  <ui:repeat>
>    <h:panelGrid>
>      <composite:insertFacet name="caption"/>
>    </h:panelGrid>
>  </ui:repeat>
> </composite:implementation>
>
>
> But this fails in subtle ways:
>
> <composite:implementation>
>  <ui:repeat>
>    <h:panelGrid>
>      <composite:renderFacet name="caption"/>
>    </h:panelGrid>
>  </ui:repeat>
> </composite:implementation>
>
>
> In particular, since the facet component does not end up as a child of 
> the repeated component in the second case, the client ids will not be 
> modified properly to reflect the current row.  If the facet happened 
> to be an input component, we would be out of luck.
>
> If I could make a single API change at this point, the one change that 
> I would make would be to spec that <composite:insertFacet> determines 
> the target facet name in the same way this is determined everywhere 
> else - ie. by the presence of a wrapping <f:facet> tag.  This would 
> allow us to address all three of the above cases with a single tag 
> with consistent behavior, which in my mind would be a huge 
> improvement.  (Presumably we could deprecate <composite:renderFacet> 
> if we made this change.)
>
> However, this is a big change.  It would mean that any existing cases 
> that look like this:
>
> <composite:implementation>
>  <h:panelGrid>
>    <composite:insertFacet name="caption"/>
>  </h:panelGrid>
> </composite:implementation>
>
> Would need to be changed to this:
>
> <composite:implementation>
>  <h:panelGrid>
>    <f:facet name="caption">
>      <composite:insertFacet name="caption"/>
>    </f:facet>
>  </h:panelGrid>
> </composite:implementation>
>
> I am doubtful that we have the ability to make such a large change, 
> but think it is at least worth considering.
I agree, considering the issues raised, I think we need to consider 
deprecating some of this and replacing it with a better solution for 2.1.
> If we cannot do this now, things that we should think about:
>
> - Is there anything that we can do now that would limit the impact of 
> the inconsistencies between our two composite facet tags?
> - Is there anything that we can do now to better support cases like #2 
> above (insert facet using different name)?  (This was the reason for 
> my original email.)
I am in favor of adding an attribute name for the target facet name in 
the short term (pre-2.1).
> - Where do we want to end up in 2.1?
My preference: 1 behavior and 1 tag (if we don't change the behavior, I 
don't support 1 tag.)

Thanks Andy!!

Ken
> - Is there anything that we can do now that will help us get to where 
> we want to be in 2.1?
>
>>   I understand that may be much easier to *say* than implement, 
>> though, as it poses several challenges -- many of which I probably 
>> haven't thought through yet (such as how id's are resolved if it is 
>> inserted 2x or more).
>
> Yep, I sense the implementation team cringing with every character 
> that I type. ;-)
>
>
>>
>>>
>>> FWIW, even if we do not have the ability to make changes along these 
>>> lines now, I am still interested in understanding/working through 
>>> these problems so that we have a firm grasp of any 
>>> limitations/complications present in our current APIs.
>> I agree, I like to think through these things as well for the sake of 
>> understanding it better.
>
> Thanks Ken.  It's been helpful working through these problems.
>
> Andy
>