search

rasyahadlinugraha / wiquery

Automatically exported from code.google.com/p/wiquery
MIT License
0 stars 0 forks source link

Adding a WiQueryPlugin to target.addComponent(plugin) fails to render it as a jQuery component #143

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
What steps will reproduce the problem?
1.  add a Dialog.
2.  Open the Dialog from an AjaxLink onClick via dialog.open(target);
3.  ALSO, add the dialog or its parent to target.addComponet(dialog) in order 
to update the contents of the dialog.

What is the expected output? What do you see instead?
I expect the Dialog to open with the new content.  However, the dialog fails to 
render at all if you add it or its parent to the target.  We don't always have 
access to the content div of the dialog to add that component directly, so what 
is the workaround here or am I doing something wrong?

What version of the product are you using? On what operating system?
1.1.1 Windows 7

Please provide any additional information below.
It seems this is not unique to dialog, but any WiQueryPlugin.

Original issue reported on code.google.com by gregory....@gmail.com on 1 Dec 2010 at 12:32

GoogleCodeExporter commented 9 years ago 
Additionally, it seems if you add ANY component to the target, it breaks any 
appended jQuery javascript.  for instance, I tried step 1 and 2, and then added 
a random OTHER component to the target.  THAT component's jQuery functionality 
disappeared, and the dialog did not open.

Original comment by gregory....@gmail.com on 1 Dec 2010 at 12:48

GoogleCodeExporter commented 9 years ago 
More information:  Looking at the wicket ajax debug, it appears that it is 
placing the javascript to open the dialog ahead of the 
$(#component).dialog(...)  Basically, the plugin rendering javascript is being 
executed AFTER dialog.open(target), and therefore you never see it.   Anything 
you add to target.appendJavascript (including dialog.open(target) which does 
this under the covers) will be executed prior to the plugin.render javascript.  
At least my understanding is that you can't open a dialog that isn't yet a 
dialog!

from code:
                @Override
                public void onClick(AjaxRequestTarget target)
                {
                    viewReqModal.setModelObject(requirementService.find(66L));
                    target.addComponent(viewReqModal);
                    viewReqModal.open(target);
                }

Wicket AJAX Debug (the dialog has an accordion in it):

<?xml version="1.0" 
encoding="UTF-8"?><ajax-response><header-contribution><![CDATA[<head 
xmlns:wicket="http://wicket.apache.org"><script type="text/javascript" 
src="resources/org.odlabs.wiquery.core.commons.CoreJavaScriptResourceReference/j
query/jquery-1.4.4.js"></script>
<link rel="stylesheet" type="text/css" 
href="resources/org.odlabs.wiquery.ui.themes.WiQueryCoreThemeResourceReference/f
usion/jquery-ui-1.8.6.custom.css" />
<script type="text/javascript" 
src="resources/org.odlabs.wiquery.ui.core.CoreUIJavaScriptResourceReference/jque
ry.ui.core.js"></script>
<script type="text/javascript" 
src="resources/org.odlabs.wiquery.ui.widget.WidgetJavascriptResourceReference/jq
uery.ui.widget.js"></script>
<script type="text/javascript" 
src="resources/org.odlabs.wiquery.ui.mouse.MouseJavascriptResourceReference/jque
ry.ui.mouse.js"></script>
<script type="text/javascript" 
src="resources/org.odlabs.wiquery.ui.position.PositionJavascriptResourceReferenc
e/jquery.ui.position.js"></script>
<script type="text/javascript" 
src="resources/org.odlabs.wiquery.ui.dialog.DialogJavaScriptResourceReference/jq
uery.ui.dialog.js"></script>
<script type="text/javascript" 
src="resources/org.odlabs.wiquery.ui.draggable.DraggableJavaScriptResourceRefere
nce/jquery.ui.draggable.js"></script>
<script type="text/javascript" 
src="resources/org.odlabs.wiquery.ui.resizable.ResizableJavaScriptResourceRefere
nce/jquery.ui.resizable.js"></script>
<script type="text/javascript" 
src="resources/org.odlabs.wiquery.ui.accordion.AccordionJavaScriptResourceRefere
nce/jquery.ui.accordion.js"></script>
</head>]]></header-contribution><component id="viewRequirement10" 
><![CDATA[<!-- the content is here but I have ommitted it to slim things 
down-->]]></component><evaluate><![CDATA[$('#viewRequirementDialog4').dialog('op
en');]]></evaluate><evaluate><![CDATA[$('#viewRequirementDialog4').dialog({autoO
pen: false, position: 'center', height: 700, width: 1000, modal: true, 
resizable: false})
;]]></evaluate><evaluate><![CDATA[$('#requirementViewAccordion5').accordion({aut
oHeight: false})
;]]></evaluate></ajax-response>

As you can see with the EVALUATE statements at the end, the javascript you 
manually append to the target (dialog.open()) occurs before the 
WiQueryCoreHeaderContributor.renderPlugin jQuery statements.

Original comment by gregory....@gmail.com on 3 Dec 2010 at 4:14

GoogleCodeExporter commented 9 years ago 
Hi,

Do you use Wicket-1.4.14 ? Because it seems this release contains some problems 
with resources and Ajax.

By the way, can you provide us an eclipse project to illustrate your problem 
please ?

Thank you very much

Julien Roche

Original comment by roche....@gmail.com on 4 Dec 2010 at 12:35

GoogleCodeExporter commented 9 years ago 
No, using WiQuery 1.1.2 with wicket 1.4.13 (also had same problem with WiQuery 
1.1.1 and Wicket 1.4.12).  I'm working on the project.

Original comment by gregory....@gmail.com on 4 Dec 2010 at 12:46

GoogleCodeExporter commented 9 years ago 
Attached is a sample project that shows the issue.  Please let me know how high 
priority this is to the wiquery team.  My deadlines are getting closer and I 
may need to look into other options if I can't get this working.  Thanks again 
for the help.

Original comment by gregory....@gmail.com on 4 Dec 2010 at 2:57

Attachments:

GoogleCodeExporter commented 9 years ago 
Hi Greg,

Usually, I put a wicket element into the dialog that I render to refresh the 
content of the dialog. But your approach raises a problem: we try to open the 
dialog before the jquery regeneration:

<evaluate><![CDATA[$('#testDialogBad1').dialog('open');]]></evaluate><evaluate><
![CDATA[$('#testDialogBad1').dialog({autoOpen: false, position: 'center', 
height: 700, width: 1000, modal: true, resizable: false})
;]]></evaluate>

So, I try to found a solution to execute the javascript before the open 
statement. I found a solution with the use of the AjaxRequestTarget.IListener 
interface where we can execute javascript after the HTML insertion and before 
the "append" javascripts.

Before:
AjaxRequestTarget ajaxRequestTarget = (AjaxRequestTarget) requestTarget;
ajaxRequestTarget.appendJavascript(js);

After:
AjaxRequestTarget ajaxRequestTarget = (AjaxRequestTarget) requestTarget;
                ajaxRequestTarget.addListener(new IListener() {

                    /**
                     * {@inheritDoc}
                     * @see org.apache.wicket.ajax.AjaxRequestTarget.IListener#onAfterRespond(java.util.Map, org.apache.wicket.ajax.AjaxRequestTarget.IJavascriptResponse)
                     */
                    public void onAfterRespond(Map<String, Component> map,
                            IJavascriptResponse response) {
                        response.addJavascript(js);
                    }

                    /**
                     * {@inheritDoc}
                     * @see org.apache.wicket.ajax.AjaxRequestTarget.IListener#onBeforeRespond(java.util.Map, org.apache.wicket.ajax.AjaxRequestTarget)
                     */
                    public void onBeforeRespond(Map<String, Component> map, AjaxRequestTarget target) {
                        // Do nothing
                    }
                });

For the moment, maybe you can override the JsQuery class and applying the 
attached patch. I will ask members of the wiQuery teams if they see problems 
with the new approach and if we can apply it for the next release.

Regards

Julien Roche

Original comment by roche....@gmail.com on 5 Dec 2010 at 9:06

Attachments:

GoogleCodeExporter commented 9 years ago 
New patch

Original comment by roche....@gmail.com on 5 Dec 2010 at 9:08

Attachments:

GoogleCodeExporter commented 9 years ago 
Do I need 1.2-SNAPSHOT to apply this patch?  If so, how stable is this compared 
to 1.1.2?   Thanks for the help!

Original comment by gregory....@gmail.com on 5 Dec 2010 at 9:41

GoogleCodeExporter commented 9 years ago 
This seems to work great to solve the original problem of the dialog not 
showing.  However, this may warrant a new issue number, but if you watch your 
firebug display, each time you try to open the "bad" dialog in my sample 
project by clicking the link, it creates a NEW div for the dialog instead of 
re-rendering the existing div.  This issues exists in 1.1.2 before your patch - 
it is not introduced by it.  I noticed when the Dialog wasn't opening before 
your patch it kept creating new dialog divs instead of re-rendering the 
existing one.  Take a look, click the first link in my test page and then close 
the dialog 3 times.  You'll see three separate divs in firebug.

Original comment by gregory....@gmail.com on 5 Dec 2010 at 10:22

GoogleCodeExporter commented 9 years ago 
One other thing:  Since it is adding new dialog divs instead of replacing the 
original one, I thought I should mention that the dialog div that is SHOWN is 
the original, not the NEW CONTENT provided by the ajax call.  That new ajax 
content is added as a new dialog div, hidden, and the old dialog div is then 
shown.

Original comment by gregory....@gmail.com on 5 Dec 2010 at 10:45

GoogleCodeExporter commented 9 years ago 
Hi,

The 1.1.2 release is the mirror of the trunk (for the moment, no changes were 
done).

For the multiple divs, we cannot considerer this is a wiQuery issue, because 
this is the normal jquery behavior of the dialog: it removes the html content 
from the original position and put it at the end of the dom (very important if 
we want to display as a modal window). So, if you want to clean the dom, you 
should do:

target.preprend(dialog.destroy().chain("remove")); // Destroy the jquery dialog 
and remove all unused HTML elements
// Refresh dialog

That's why I prefer use a wicket element inside the dialog to refresh its 
content instead of refresh the entire dialog.

Regards

Julien Roche

Original comment by roche....@gmail.com on 6 Dec 2010 at 8:09

GoogleCodeExporter commented 9 years ago 
Hi,

I think the first way to issue Dialog is wrong because u=you are updating the 
component and at the same time executing the JavaScript that creates it. For 
such use cases maybe it would be good to create a new component:

public class CanBeOpenViaAJAXDialog extends Dialog implements 
IHeaderContributor {

    private static final long serialVersionUID = 1L;

    /**
     * @param id
     */
    public CanBeOpenViaAJAXDialog(String id) {
        super(id);
    }

    public void renderHead(IHeaderResponse response) {
        response.renderOnDomReadyJavascript(this.open().render().toString());
    }

}

But again you will have the problem of multiple divs I guess.

About the patch: would it make sense to make it local? That is make Dialog.open

/**Method to open the dialog within the ajax request
     * @param ajaxRequestTarget
     */
    public void open(AjaxRequestTarget ajaxRequestTarget) {     
        ajaxRequestTarget.addListener(new AjaxRequestTarget.IListener() {

            public void onBeforeRespond(Map<String, Component> map,
                    AjaxRequestTarget target) {             
            }

            public void onAfterRespond(Map<String, Component> map,
                    IJavascriptResponse response) {
                response.addJavascript(Dialog.this.open().render().toString());
            }
        });
    }

Regards,

Ernesto

Original comment by reier...@gmail.com on 6 Dec 2010 at 8:31

GoogleCodeExporter commented 9 years ago 
Hi Ernesto,

With this approach, the open statement will be called again before the 
generation statement (and so, after the html generation).

I really think we have to use the IListener into the JsQuery class to force the 
generation to be executed first and the other actions after

Cheers

Julien

Original comment by roche....@gmail.com on 6 Dec 2010 at 9:00

GoogleCodeExporter commented 9 years ago 
Julien,

What about 

public class CanBeOpenViaAJAXDialog extends Dialog implements 
IHeaderContributor {

    private static final long serialVersionUID = 1L;

    /**
     * @param id
     */
    public CanBeOpenViaAJAXDialog(String id) {
        super(id);
    }

    @Override
    public JsStatement statement() {
        return null;
    }

    public void renderHead(IHeaderResponse response) {
        response.renderOnDomReadyJavascript(new JsQuery(this).$().chain("dialog",
                getOptions().getJavaScriptOptions()).render().toString());
        response.renderOnDomReadyJavascript(this.open().render().toString());
    }

}

Cheers,

Ernesto

Original comment by reier...@gmail.com on 6 Dec 2010 at 2:07

GoogleCodeExporter commented 9 years ago 
Ernesto,

We will have two jquery generation statements if we refresh the dialog with the 
AjaxRequestTarget. The IListener can prevent it.

For the renderOnDomReadyJavascript, I have to check again into the code, but I 
see that it can be used only "one time". If another behavior / component had 
already called it, the second called will not inject it into the "domready" 
statement (I'm not sure, I saw it very fast yesterday).

I don"t know what is the best approach.

Cheers

Julien

Original comment by roche....@gmail.com on 6 Dec 2010 at 2:20

GoogleCodeExporter commented 9 years ago 
Julien,

That approach works perfectly.  I extended Dialog with our own custom Dialog 
subclass, and overrode open(ajaxRequestTarget) with:

    /*
     * (non-Javadoc)
     * 
     * @see org.odlabs.wiquery.ui.dialog.Dialog#open(org.apache.wicket.ajax.
     * AjaxRequestTarget)
     */
    @Override
    public void open(AjaxRequestTarget ajaxRequestTarget)
    {
        if (checkInTargetHierarchy(ajaxRequestTarget.getComponents()))
        {
            ajaxRequestTarget.prependJavascript(destroy().chain("remove").render().toString());
        }
        super.open(ajaxRequestTarget);
    }

    private boolean checkInTargetHierarchy(Collection<? extends Component> components)
    {
        boolean found = false;
        Iterator<? extends Component> iter = components.iterator();
        while (iter.hasNext() && !found)
        {
            Component component = iter.next();
            found = (component == this);
            if (!found)
            {
                // component is not this one, check its kids
                if (component instanceof MarkupContainer)
                {

                    Object result = ((MarkupContainer) component).visitChildren(OscarDialog.class, new IVisitor<Component>()
                        {

                            @Override
                            public Object component(Component component)
                            {
                                return (component == OscarDialog.this) ? component : CONTINUE_TRAVERSAL;
                            }
                        });
                    if (result == this)
                    {
                        found = true;
                    }
                }
            }
        }
        return found;

    }

Original comment by gregory....@gmail.com on 6 Dec 2010 at 9:45

GoogleCodeExporter commented 9 years ago 
Hi Greg,

I think we will not apply the patch. Maybe we can supply for strategics methods 
(like the open method from the dialog) an option to clean the DOM (destroy the 
instance, HTML ...)

What do you think ?

Regards

Julien Roche

Original comment by roche....@gmail.com on 7 Dec 2010 at 7:46

GoogleCodeExporter commented 9 years ago 
Julien,

Which issue, the first or the second?  I think your fix (patch you included) is 
important.  What is a use case where you would NOT want do what you patch does? 
 I'm trying to think of an example where you would WANT appendJavascripts to be 
executed before WiQuery plugin render javascripts?

Original comment by gregory....@gmail.com on 7 Dec 2010 at 7:48

GoogleCodeExporter commented 9 years ago 
Greg,

Want you want to achieve is easily achievable with your second approach 

1-add dialog contents to request target 
2-and do dialog open.

IMHO what we need is something like a dialog panel with markup

<wicket:panel>
    <div wicket:id="dialog">
       <div wicket:id="contents"></div>
    </div>
<wicket:panel>

with a setContents method and a show or open method that does both things 

1- add contents to AjaxRequestTarget 
2- and issue an open JavaScript statement.

This class will be very similar to  the ModalWindow component found as part of 
wicket-extensions.

Regards,

Ernesto

Original comment by reier...@gmail.com on 8 Dec 2010 at 6:54

GoogleCodeExporter commented 9 years ago 
Fix in commit r607

Original comment by roche....@gmail.com on 8 Dec 2010 at 8:42

GoogleCodeExporter commented 9 years ago 
Great news, thanks Julien.

Original comment by gregory....@gmail.com on 10 Dec 2010 at 8:31