Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 6.0.0-beta1
    • Component/s: wicket
    • Labels:

      Description

      this is a proposal to remove the rather annoying requirement of having to call setoutputmarkupid(true) on components.

      when a user calls getmarkupid() they are showing intent of using that id, so if that method is called we should automatically call setoutputmarkupid(true). further we should process renderhead() of components first - before rendering the components. most calls to getmarkupid() come from that method so processing it before rendering the component will toggle the correct bit.

      1. WICKET-4091.patch
        0.7 kB
        Emond Papegaaij

        Activity

        Hide
        Ronald Tetsuo Miura added a comment -

        I don't think I understood the proposal correctly.

        Most of the complaints about setOutputMarkupId() I see are around component re-rendering in ajax requests, which usually doesn't involve calling getMarkupId() (just target.add(component)).

        Even if it did (say, we changed target.add(Component) to target.add(String), which would be a terrible idea, of course), it's called in the event handling time, not in page construction/render time, thus having no effect.

        I don't see a solution to this, besides setting it to true on every component by default (either by a custom listener or by a built-in setting). And then, you'll still have to call setOutputPlaceholderTag() when the components are initially invisible.

        I vote to make both true by default, or add some one-line setting to do it

        Show
        Ronald Tetsuo Miura added a comment - I don't think I understood the proposal correctly. Most of the complaints about setOutputMarkupId() I see are around component re-rendering in ajax requests, which usually doesn't involve calling getMarkupId() (just target.add(component)). Even if it did (say, we changed target.add(Component) to target.add(String), which would be a terrible idea, of course), it's called in the event handling time, not in page construction/render time, thus having no effect. I don't see a solution to this, besides setting it to true on every component by default (either by a custom listener or by a built-in setting). And then, you'll still have to call setOutputPlaceholderTag() when the components are initially invisible. I vote to make both true by default, or add some one-line setting to do it
        Hide
        Sven Meier added a comment -

        This will not cure all cases (as Ronald has pointed out), but it's a first step in the right direction: +1.

        Show
        Sven Meier added a comment - This will not cure all cases (as Ronald has pointed out), but it's a first step in the right direction: +1.
        Hide
        Martin Dilger added a comment -

        I agree with Ronald, I would rather provide a one-line setting to automatically enable/disable both,
        we currently use a ComponentInstanatiationListener to activate both and go well with it.

        Show
        Martin Dilger added a comment - I agree with Ronald, I would rather provide a one-line setting to automatically enable/disable both, we currently use a ComponentInstanatiationListener to activate both and go well with it.
        Hide
        Emond Papegaaij added a comment -

        Enabling outputMarkupId from getMarkupId() will help a great deal with js-integration frameworks, such as WiQuery. WiQuery will render statements such as $("#id123").plugin(options). For this, it calls getMarkupId to get the markup id, and also setOutputMarkupId(true), to enable it in the markup. I think Igor's statement makes sense: if you use the markup id (ie. you call getMarkupId), you want it in the markup. Or the other way around: what good is the markup id if it does not show up in the markup?

        Show
        Emond Papegaaij added a comment - Enabling outputMarkupId from getMarkupId() will help a great deal with js-integration frameworks, such as WiQuery. WiQuery will render statements such as $("#id123").plugin(options). For this, it calls getMarkupId to get the markup id, and also setOutputMarkupId(true), to enable it in the markup. I think Igor's statement makes sense: if you use the markup id (ie. you call getMarkupId), you want it in the markup. Or the other way around: what good is the markup id if it does not show up in the markup?
        Hide
        Emond Papegaaij added a comment -

        As far as I can see, this is all that needs to be changed. OutputMarkupId was already set in setMarkupIdImpl, it only had to be moved up a few lines, to make it also in effect for implicit calls. renderHead is already called before the body is rendered, so no change is required for that.

        Show
        Emond Papegaaij added a comment - As far as I can see, this is all that needs to be changed. OutputMarkupId was already set in setMarkupIdImpl, it only had to be moved up a few lines, to make it also in effect for implicit calls. renderHead is already called before the body is rendered, so no change is required for that.
        Hide
        Martin Grigorov added a comment -

        With the usage of jQuery in wicket-ajax.js I think the writing of the component markup id in the final markup can be optional.
        We can add target.add(componentWithoutMarkupId, "someValidCssSelector") and this will produce <component selector="someValidCssSelector">...</component> and the component(s) which match this CSS selector will be replaced with the markup of componentWithoutMarkupId.

        This will work for any CSS3 selector engine (like Sizzle (jQuery and Dojo), qwery (Ender), ...);

        This way the user will be able to repaint several fragments in the markup with one shot.
        Is this something we want ?
        I can see how this can desynchronize the server state with the client view.

        Show
        Martin Grigorov added a comment - With the usage of jQuery in wicket-ajax.js I think the writing of the component markup id in the final markup can be optional. We can add target.add(componentWithoutMarkupId, "someValidCssSelector") and this will produce <component selector="someValidCssSelector">...</component> and the component(s) which match this CSS selector will be replaced with the markup of componentWithoutMarkupId. This will work for any CSS3 selector engine (like Sizzle (jQuery and Dojo), qwery (Ender), ...); This way the user will be able to repaint several fragments in the markup with one shot. Is this something we want ? I can see how this can desynchronize the server state with the client view.
        Hide
        Igor Vaynberg added a comment - - edited

        imho this is going to be too error prone....as in the dev now needs to keep css in sync with the java side of things in order for the app to work.

        Show
        Igor Vaynberg added a comment - - edited imho this is going to be too error prone....as in the dev now needs to keep css in sync with the java side of things in order for the app to work.
        Hide
        Emond Papegaaij added a comment -

        With the patch for WICKET-4273 applied, I think this issue can be closed. The patch for that issue also included the patch attached here. After porting our application and WiQuery to Wicket 6, this small change made most setOutputMarkupId calls unnecessary.

        Show
        Emond Papegaaij added a comment - With the patch for WICKET-4273 applied, I think this issue can be closed. The patch for that issue also included the patch attached here. After porting our application and WiQuery to Wicket 6, this small change made most setOutputMarkupId calls unnecessary.
        Hide
        Emond Papegaaij added a comment -

        As the patch has been applied quite some time ago, and no problems have been found so far regarding this, I'm closing the ticket. It now works as Igor described.

        Show
        Emond Papegaaij added a comment - As the patch has been applied quite some time ago, and no problems have been found so far regarding this, I'm closing the ticket. It now works as Igor described.

          People

          • Assignee:
            Emond Papegaaij
            Reporter:
            Igor Vaynberg
          • Votes:
            1 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development