Hi Sebastien, Thanks for the feedback. I've changed the 2 methods (getCallbackFunction and getCallbackFunctionBody) to public. While porting WiQuery to wicket 6, I ran into the same issue.
For the callback parameters, please take a look at the following file: https://github.com/papegaaij/wiquery/blob/master/wiquery-core/src/main/java/org/odlabs/wiquery/core/behavior/AbstractAjaxEventCallback.java It solves exactly the problem you are trying to solve. Subclasses can specify additional parameters by putting name and javascript code in the map. With regards to the json, WiQuery has exactly the same problem. So far, we've tried several solutions, and the one I like best is serializing java beans using jackson (or gson) from the model of the component. The problem with the approach however is that it would add another dependency to wicket and it might impose a significant performance overhead. For example, on a page with thousands of these components, I reduced the server time by about 99% by replacing jackson with an hand-built stripped down json parser. Best regards, Emond 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 >>
