Mailing-List: contact dev-help@myfaces.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "MyFaces Development" <dev@myfaces.apache.org>
Received-SPF: pass (athena.apache.org: local policy)
Subject: Re: myfaces-builder-plugin notes
From: simon <simon.kitching@chello.at>
To: MyFaces Development <dev@myfaces.apache.org>
In-Reply-To: <f6c92360806160721u3a6ab0ebr27cedb56ff688560@mail.gmail.com>
References: <1213128345.6360.30.camel@simon-laptop>
	 <f6c92360806101439l861ad34s34951f456ce955e5@mail.gmail.com>
	 <1213339936.6956.27.camel@simon-laptop>
	 <f6c92360806131036k47afd895vceaacb69b8100dc7@mail.gmail.com>
	 <1213559516.6403.40.camel@simon-laptop>
	 <f6c92360806160721u3a6ab0ebr27cedb56ff688560@mail.gmail.com>
Content-Type: text/plain
Date: Mon, 16 Jun 2008 20:41:36 +0200
Message-Id: <1213641696.6225.29.camel@simon-laptop>
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit


On Mon, 2008-06-16 at 09:21 -0500, Leonardo Uribe wrote:
> 
> 
> On Sun, Jun 15, 2008 at 2:51 PM, simon <simon.kitching@chello.at>
> wrote:
>         Or we could use approach (c) and model both. I actually like
>         this idea
>         quite a lot. A class that is annotated in a way that triggers
>         code-generation would create TWO ComponentMeta objects: one
>         for itself,
>         and one for the class to be generated. I think that would make
>         things
>         much clearer...
> 
> I don't know if this concept could break some functionality

You're right. After some more thought, it is a bad idea.

I think I've finally got my head around the current implementation, and
agree that it's fine as is. It just needs better documentation (which
I'm working on) and IMO a few minor property renames for clarity.

I'm working on the documentation, but will check with you before doing
anything other than trivial renames. Hope that's ok.


>         
>         [1] This unit test seems to have also found a bug in the
>         creation of the
>         "parent" field, which currently looks like ".Foo" rather than
>         "packagename.Foo".
> 
> I can see the problem. The solution could be use:

Hmm.. the problem seems to have gone away now.

>         
>         
>         I've updated componentClass11.vm to output the original
>         sourceCource as
>         a comment on generated classes so it's easy to find the
>         template class.
>         
> 
> Good, Theorically, myfaces-builder plugin should use tomahawk core and
> core12 velocity
> templates as default templates (actually the default templates are
> never used).
> The idea is create an archetype to allow people start coding
> components with
> myfaces-builder-plugin easily.

Ok. I will update the builder version to match core11 for clarity - but
also add a comment indicating that projects using the builder plugin are
expected to make a copy of that template and modify it to suit
themselves.

>         > Yeap, I see the changes on myfaces api 1.1 and 1.2
>         yesterday. There is
>         > a probem with some tomahawk component that extends from
>         UISelectItem
>         > and use rendered property (obviously throw Unsupported.....,
>         but the
>         > solution is very easy.
>         
>         
>         Is this just a theoretical problem, or is there really a
>         component that
>         does this?
> 
> The test that fails is this:
> 
>  myfaces-example-simple/panelnavigation_2.jsf
> 
> The stack trace is this:
> 
> Caused by: java.lang.UnsupportedOperationException
> 	at javax.faces.component.UISelectItem.setRendered(UISelectItem.java:79)
> 
> 	at org.apache.myfaces.custom.navmenu.htmlnavmenu.HtmlNavigationMenuRenderer.addUINavigationMenuItems(HtmlNavigationMenuRenderer.java:291)
> 	at org.apache.myfaces.custom.navmenu.htmlnavmenu.HtmlNavigationMenuRenderer.preprocessNavigationItems(HtmlNavigationMenuRenderer.java:267)
> 
> 	at org.apache.myfaces.custom.navmenu.htmlnavmenu.HtmlNavigationMenuRenderer.encodeEnd(HtmlNavigationMenuRenderer.java:129)
> 	at javax.faces.component.UIComponentBase.encodeEnd(UIComponentBase.java:762)
> We also have problems with UIViewRoot (see MYFACES-1881). For
> MYFACES-1881 in my opinion the best is do not touch
> UIComponentClassicTagBase. For UINavigationMenuItems the best is
> custom redefinition of rendered property. But I want to hear your
> opinion before take any decision.

I'll take a look at that and see.

For UINavigationMenuItem, I think the best solution is probably to
remove that call from the renderer. The code is creating a
UINavigationMenuItem for each "model" menu item, then trying to disable
some of them by setting the rendered flag. Probably it should just skip
creation of the UINavigationMenuItem instance in that case. But that is
complicated and would require testing, so as a hack adding a "rendered"
property on UINavigationMenuItem sounds ok to me.

But I'm convinced that disabling rendered on UISelectItem is right, and
the navigation component is wrong to try to use that property. A
UISelectItem is a logical options that is rendered *by* something else
(a UISelectMany or UISelectOne) but is not renderable itself.


Is the UIComponentClassicTagBase code this bit?
                if (_id != null)
                {
                    _componentInstance.setId(_id);
                }
                else
                {
                    _componentInstance.setId(getFacesJspId());
                }

I'd be tempted to just add 
   else if (!_componentInstance instanceof UIViewRoot)
but I haven't given this a lot of thought.

I'm pretty sure that there are no side-effects from UIViewRoot not
having an id. Ids are for:
* mapping tags in the jsp file to existing components in the tree
  on postbacks. But this is not relevant for UIViewRoot; see 
  restoreView method
* finding components in the tree. But there exists a dedicated method
  to find the unique view root component
* mapping POST data into components. But UIViewRoot is not an input.

Regards,
Simon
>