On Fri, Jul 10, 2015 at 1:27 AM, Sven Meier <[email protected]> wrote: > Yes, an overriden method with ART parameter will be something to be > migrated :/. > > > We can try though! > > I've greped the Wicket's souce code once again, and only on*()- and > respond()-methods kept their ART. > All other 'active' methods (e.g. ModalWindow#close()) take a > IPartialPageRequestHandler now. >
I've missed that ModalWindow#close() is changed too. I'll do a grep as well. Since this change is done for Wicket then it should be done for other more important UI libraries like Wicket jQuery UI, Wicket Bootstrap and WicketStuff Foundation. I suggest to postpone the release of Wicket 7.0.0 with few weeks until we make the changes and test them for few days. WDYT? > > If it helps, I could create a pull-request for the relevant methods in > wicket-jquery-ui. > > Sven > > > > On 09.07.2015 22:32, Martin Grigorov wrote: > >> On Thu, Jul 9, 2015 at 9:23 PM, Sven Meier <[email protected]> wrote: >> >> Ok. you're partially right :/ >>> >>> The compilation error would happen if an application extends ModalWindow >> and overrides #close(AjaxRequestTarget) method to do some extra tasks. I >> have such code for Wicket Bootstrap's Modal component in one of my >> applications. >> >> >> #close(AjaxRequestTarget) to #close(IPartialPageRequestHandler) >>>> >>> Every application has to be recompiled as the method argument type has >>> changed. But there won't be any compilation error. >>> But for a migration to Wicket 7 each application will have to be >>> recompiled anyway. >>> >> >> The problem is that we probably won't find and change all possible >> candidates for this change. And once 7.0.0 is out it will be too late. We >> can try though! >> >> >> >>> Sven >>> >>> >>> >>> On 09.07.2015 20:11, Sven Meier wrote: >>> >>> Hi Martin, >>>> >>>> This is what I meant as too disruptive change. If we change the type of >>>>> >>>> the parameter from ART >>>> >>>>> to IPPRH then every application that uses ModalWindow will have to fix >>>>> >>>> the compilation error. >>>> >>>> but ART *is* an IPPRH, so there will be no compilation error. >>>> >>>> If we broaden the API from #close(AjaxRequestTarget) to >>>> >>>>> #close(IPartialPageRequestHandler) then this will be an API break. >>>>> Every application will both at compile time and runtime. >>>>> >>>>> Nope, see above. >>>> >>>> Regards >>>> Sven >>>> >>>> On 09.07.2015 16:35, Martin Grigorov wrote: >>>> >>>> Hi Sven, >>>>> >>>>> On Thu, Jul 9, 2015 at 4:31 PM, Sven Meier <[email protected]> wrote: >>>>> >>>>> Hi, >>>>> >>>>>> I've just changed all relevant places in Wicket from AjaxRequestTarget >>>>>> to >>>>>> IPartialPageRequestHandler. >>>>>> >>>>>> I don't think this change is too disruptive: >>>>>> >>>>>> - client code can still call the existing methods with an ART, e.g. >>>>> >>>>> modalWindow.close(ajaxRequestTarget) >>>>>> >>>>>> What Maxim wanted as changes in Wicket jQuery UI are actually >>>>>> changes >>>>>> >>>>> exactly like ModalWindow#close(). >>>>> He needs to be able to close jQuery UI dialogs (non-modal window) from >>>>> WebSocket response. >>>>> This is what I meant as too disruptive change. If we change the type of >>>>> the >>>>> parameter from ART to IPPRH then every application that uses >>>>> ModalWindow >>>>> will have to fix the compilation error. >>>>> >>>>> >>>>> - all places needing an ART remain unchanged, e.g. >>>>> >>>>>> AjaxLink#onClick(AjaxRequestTarget) >>>>>> >>>>>> It is not possible to click a link with WebSocket request so this is >>>>>> >>>>> OK. >>>>> The application developer can roll WebSocketLink if something like this >>>>> is >>>>> needed. >>>>> >>>>> >>>>> - a dozen Wicket components had to be changed to cooperate with >>>>> >>>>>> IPartialPageRequestHandler instead of just ART: >>>>>> find(IPartialPageRequestHandler.class) >>>>>> >>>>>> Those are good changes! >>>>>> >>>>> >>>>> If we identify other methods later, we can still broaden their >>>>> signature >>>>> >>>>>> to use IPartialPageRequestHandler >>>>>> without breaking the API. >>>>>> >>>>>> No. If we broaden the API from #close(AjaxRequestTarget) to >>>>> #close(IPartialPageRequestHandler) then this will be an API break. >>>>> Every application will both at compile time and runtime. >>>>> >>>>> >>>>> >>>>> we are very (very) close to release 7.0.0, that's why I am a little >>>>>> bit >>>>>> >>>>>> concerned >>>>>>> >>>>>>> So am I, but for now I think it's simpler to go one step further >>>>>>> >>>>>> instead >>>>>> of 2 steps back. >>>>>> >>>>>> IMO, it is OK-ish to change APIs in Wicket jQuery UI (Sebastien does >>>>>> >>>>> this >>>>> from time to time). >>>>> Even if we decide to not break the API of some component then as I >>>>> suggested it should be possible to create an adapter from IPPRH to ART: >>>>> >>>>> MyComponent.java: >>>>> >>>>> add(new WebSocketBehavior() { >>>>> protected void onPush(WebSocketRequestHandler handler, >>>>> IWebSocketPushMessage message) >>>>> { >>>>> AjaxRequestTarget target = >>>>> >>>>> WebApplication.get().getAjaxRequestTargetProvider().get(handler.getPage()); >>>>> >>>>> modalWindow.close(target); >>>>> } >>>>> }); >>>>> >>>>> >>>>> Regards >>>>> >>>>>> Sven >>>>>> >>>>>> >>>>>> >>>>>> On 09.07.2015 00:31, Sebastien wrote: >>>>>> >>>>>> Hi Sven, >>>>>> >>>>>>> >>>>>>> But it seems in wicket-jquery-ui there are more methods of this >>>>>>> kind? >>>>>>> >>>>>>> True, about 85 methods (taking an ART without starting with >>>>>>>> >>>>>>>> "on"-prefix). >>>>>>> In these, I should identify the ones that should be changed (like >>>>>>> #open, >>>>>>> #close, #show, #hide, #refresh, etc), the others, plus 2 calls to >>>>>>> #find(ART.class) to be taken into account... >>>>>>> This is a new change since -M6 and we are very (very) close to >>>>>>> release >>>>>>> 7.0.0, that's why I am a little bit concerned (did I wrote "worried" >>>>>>> previously? ;)). I hope I/we do not miss/forget something here... >>>>>>> >>>>>>> Thanks & best regards, >>>>>>> Sebastien. >>>>>>> >>>>>>> >>>>>>> >>>>>>> --------------------------------------------------------------------- >>>>>>> >>>>>> To unsubscribe, e-mail: [email protected] >>>>>> For additional commands, e-mail: [email protected] >>>>>> >>>>>> >>>>>> >>>>>> --------------------------------------------------------------------- >>>> To unsubscribe, e-mail: [email protected] >>>> For additional commands, e-mail: [email protected] >>>> >>>> >>>> --------------------------------------------------------------------- >>> To unsubscribe, e-mail: [email protected] >>> For additional commands, e-mail: [email protected] >>> >>> >>> > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > >
