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