On Tuesday 08 May 2012 10:28:16 Martin Grigorov wrote: > 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.
In this case, the use case is quite clear to me :) However, I'm not sure about the solution. What he is asking for, is an easy way to pass parameters from JavaScript to Java via a function that takes parameters. In this case, the function takes two parameters: event and ui. But, these aren't the parameters you want to pass to Java. A conversion is needed. The Map<String, String> solution is not going to solve this, because it only specifies the conversion. I was thinking about the following: let getCallbackFunction take a CallbackParameter vararg, with 4 implementations for CallbackParameter: ContextParameter, ExplicitParameter, ResolvedParameter and ConvertedParameter. A context parmater, only provides context to the function, it is not added to the Ajax call. Both event and ui are context parameter. Explicit parameters are context parameters that are also passed to the Ajax call. Resolved parameters are resolved using some javascript code. The last adds a context parameter and a conversion script to add it to the Ajax call. I've created a JIRA issue for this: WICKET-4540. Please let me know what you think of the proposed solution. > > > 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/ja > >> va/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.
