dev@javaserverfaces.java.net

Re: Please review changebundle for focus tag

From: Jason Lee <jason_at_steeplesoft.com>
Date: Tue, 8 Apr 2008 08:39:59 -0500

On Mon, Apr 7, 2008 at 8:47 PM, Jim Driscoll <Jim.Driscoll_at_sun.com> wrote:

> This is the tag that we were talking about on the list. This is the
> simple version, you just use <mj:focus for="componetID"/>. Added it as part
> of the Mojarra Extensions library.
>

I saw a couple of things. My first thought is a bit of a tangent, though.
:P I noticed you're adding these to a Mojarra Extensions directory. Ryan
and I started the sandbox some time ago for this sort of experimentation.
Since then, we moved just about everything from the sandbox to Mojarra
Scales to make it easier to manage, especially with regard to non-Mojarra
dev participation. You're more than welcome to put this there if you want,
though you by no means have to.

(Looking at the diff, it seems there's more to mojarra_ext than I thought.
I'll have to look at that :)

With that out of the way, here are my suggestions:

   - Move the Strings for type and family to public static final String
   attributes of the class. That will make it easier and less error-prone for
   people wanting to instantiate the component programmtically.
   - You're using ValueBinding in FocusTag. I'd suggest using
   ValueExpression, as VB is deprecated (unless you're wanting to make this
   work in 1.1, something I've given up on with Scales. :)
   - I'd also suggest adding a Facelets taglib file. They're dirt simple
   to make, and, since most people seem to be using Facelets (or JSFTemplating,
   which will now consume a Facelets tablig file), the component will be more
   usable.

Other than those little nits, it looks pretty good to me. :)

-- 
Jason Lee, SCJP
Software Architect -- Objectstream, Inc.
Mojarra and Mojarra Scales Dev Team
https://mojarra.dev.java.net
https://scales.dev.java.net
http://blogs.steeplesoft.com