Dear Wicket team,
Well, as you are might be close to the RC1 (and it is not sure there will
be a beta2), I send you a little feedback now because I just test it
yesterday. There is nothing extraordinary about the feedback, so do not
worry by the length of this mail.
As I ported the whole jQuery UI library to Wicket components, I was really
excited about the implementation of jQuery as the backing library! So, I
migrated to the 6.0.0-beta1... and with a few changes, about 95% of the API
worked as expected at the first try. For the 5% left, I had to think a
little bit, but it has been done. Thanks for the very good job (I mean, as
usual) !!
I only have 2 comments:
First: About the JSONObject
By using plugins (oh yes, I forgot: thanks for the
JQueryPluginResourceReference class! :)), we often need to have an
"options" object, which is a json object.
For my implementation using wicket 1.5, I wrote my own Options object, but
as we now have a JSONObject, I thought it might be a good idea to use this
one...
A plugin is based on a Behavior, so it can be bring to live using the
IHeaderResponse (in Behavior#renderHead). But, if the plugin is reattached
to the ajax request target, the behavior needs to be re-executed. That
means that the option values should still be known (ie, serialized). But
the problem: the JSONObject is not serializable. So it can not be used in
this case.
Another aspect of the options object, is that it *seems* that it cannot
contains event definitions, like "{ select: function() { ... } }." because
the callback will be written as a string (and then, never executed). I am
so surprised that I do not exclude I just did not found how to do it...
Moreover, a JSONObject object could be construct from a map, which is good,
but it is not possible to merge a map to an existing object (like #put(Map
map) or best: #merge(JSONObject jo)).
And to conclude this first point: the map in the JSONObject is not generic..
Second: AbstractDefaultAjaxBehavior / DynamicExtraParameters
In some widget, we have to provide ajax behavior(s). I'm using a small
custom class extended from AbstractDefaultAjaxBehavior, and, thanks to
updateAjaxAttributes, we can achieve many things really easily.
But, back to my "select" event just a few line above... As many plugins,
the callback definition is of the form: function(event, ui) { ... }. We
could want to supply an extra parameter to the ajax object, ie: "index",
which should have the value of "ui.item.id" for instance.
Option #1:
I tried to use something like "function(event, ui) {" +
behavior.getCallbackScript() + "}", where I set the DynamicExtraParameters
in the behavior's updateAjaxAttributes like:
attributes.getDynamicExtraParameters().add("return { index: ui.item.id }");
It was a great idea, but the problem is that "ui" is not knows in the
generated context.
Option #2:
AbstractDefaultAjaxBehavior provides a getCallbackFunction() method. Great!
I can write something like "{ select: " +
behavior.getCallbackFunction("ui", "event") + "}"
But, the result of getCallbackFunctionBody will generate something like:
var params = { "event": event, "ui": ui }, whereas I needed: var params = {
"index": ui.item.id }.
Question: would it be possible to provide another signature to
getCallbackFunction / getCallbackFunctionBody, so it can renders something
like: var params = { index: ui.item.id };?
The user can still override getCallbackFunctionBody to manage this (and it
works) but if it can be a built-in method, it would be better :)
Last detail, getCallbackFunction is protected. So, does it make sense to
change getCallbackFunction access modifier to public? In my opinion, as
getCallbackFunction takes parameters, which are not transmitted by another
method of the API, it is not especially designed to be restricted and can
therefore be invoked from outside the class (ie, public). Sure, users can
still promote the visibility in a child class...
Thanks & best regards,
Sebastien.
On Sun, May 6, 2012 at 12:37 PM, Martin Grigorov <[email protected]>wrote:
> On Fri, May 4, 2012 at 4:27 PM, tetsuo <[email protected]> wrote:
> > -1 RCx should only be used if you are pretty sure it could be released,
> > since it means *Release* Candidate.
>
> Looking at Jira I don't see any reason why not to release it as 6.0.0
> There are no planned API changes, new features, etc.
> Everything that I upgraded to 6.0-SNAPSHOT seems to work OK. I know of
> at least 15 users working with 6.0.0-beta1 and they don't experience
> any problems as well.
>
> >
> >
> >
> > On Fri, May 4, 2012 at 9:53 AM, Martijn Dashorst <
> [email protected]
> >> wrote:
> >
> >> I would opt for a beta2. IMO we should not make release candidates
> >> available for wider use, but really use release candidates as a way to
> >> push a release to its conclusion. I.E. each drop of a beta2 is a rc.
> >>
> >> Martijn
> >>
> >> On Thu, May 3, 2012 at 11:43 AM, Martijn Dashorst
> >> <[email protected]> wrote:
> >> > On Thu, May 3, 2012 at 10:44 AM, Emond Papegaaij
> >> > <[email protected]> wrote:
> >> >> I agree, it should be RC1, but first I want to merge my
> >> >> sandbox/atmosphere branch back in master and I think we should add
> >> >> wicket-cdi as experimental module as well, just as it is now.
> >> >
> >> > +1
> >> >
> >> > Martijn
> >>
> >>
> >>
> >> --
> >> Become a Wicket expert, learn from the best: http://wicketinaction.com
> >>
>
>
>
> --
> Martin Grigorov
> jWeekend
> Training, Consulting, Development
> http://jWeekend.com
>
