Thanks Sebastien! We need this kind of feedback to make sure we are on the right track.
Martijn On Sun, May 6, 2012 at 3:20 PM, Sebastien <[email protected]> wrote: > 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 >> -- Become a Wicket expert, learn from the best: http://wicketinaction.com
