Hi Sebastien, On Tue, May 8, 2012 at 3:03 AM, Sebastien <[email protected]> wrote: > Hi Emond, > > Thanks for your reply (and thanks to Martijn's one too). > > I am glad to notice that you changed the visibility of getCallbackFunction > and getCallbackFunctionBody. Thanks for this! > > I looked carefully at the class you provides as an example for the use case > of callback parameters. Then, we both have a different solutions to answer > the need: where I'm overriding the method in a simple manner, but which > need to be done for each case, you provide a more generic way, but by > adding a layer of complexity. In fact, we both have a "complex" manner to > answer a need... that is still a common need. You have to figure out - and > I'm telling that with all the respect due to you and to the work you've > done - that you will have to explain this how-to to all people that will > encounter the use case; whereas it would have been sooooo simple to provide > another signature like getCallbackFunctionBody(Map<String, String> > parameters) either by replacing the actual method's signature or by > supplying an overloaded method, the one calling the other... > But it is not too late (I think) ! :)
Don't be afraid to create tickets and attach patches to them! ;-) It is much more easier to understand the use case with a patch and a test case next to it. > > About the json object, I agree with your investigation... So, I will > continue dealing with my custom Options class, which "manually" serialize > to json, that's perfect for me !... > > Thanks again & best regards, > Sebastien. > > On Sun, May 6, 2012 at 10:08 PM, Emond Papegaaij > <[email protected]>wrote: > >> 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. >> > >> -- Martin Grigorov jWeekend Training, Consulting, Development http://jWeekend.com
