Panel: Dynamically created close elements do not close the panel

[tschettler]

After dynamically creating a close button within a panel after the panel is created, the panel doesn't close when the button is clicked.

Test Page: http://jsbin.com/oqahez/1

Steps to reproduce:

1. Click "Open Panel"
2. Click "Dynamic Close"

Expected outcome: The panel should close
Actual outcome: The panel remains open

Platforms/browsers tested:

• Windows 7 (64-bit)
  • Chrome 26.0.1450.0
  • Firefox 20.0.1
  • Opera 12.15
  • Safari 5.1.7
  • IE10

jQuery Mobile 1.3.0
jQuery 1.9.1

The issue is in the _bindCloseEvents function:

_bindCloseEvents: function() {
    var self = this;

    self._closeLink.on( "click.panel" , function( e ) {
        e.preventDefault();
        self.close();
        return false;
    });

I would propose removing _closeLink (it's only used for binding the click event) and use its selector instead:

_bindCloseEvents: function() {
    var self = this;

    self.element.on( "click.panel" , ":jqmData(rel='close')" , function ( e ) {
        e.preventDefault();
        self.close();
        return false;
    });

This resolved the issue for me.

[jaspermdegroot]

@tschettler

Thanks for reporting the issue. This wouldn't be a problem if the panel widget had a refresh method. After dynamically creating the close button you could call .panel( "refresh" ) and it would work. There is already a ticket for this: #5659.

About your PR #5928. First of all, thanks a lot for proposing a solution! This would indeed resolve the problem, but has a negative effect on performance.

Closing this ticket as duplicate of #5659

[tschettler]

@uGoMobi

Thanks for the cordial response, I would agree that a refresh method would be helpful, but it also seems that approaches similar to what I have suggested have already been implemented in this and other jQuery Mobile widgets. For example, the very next event binding:

self.element.on( "click.panel" , "a:jqmData(ajax='false')", function( e ) {
    self.close();
});     

Also, the popup widget uses a closeLinkSelector ("a:jqmData(rel='back')") to bind the close events, as:

self.element
    .delegate( opts.closeLinkSelector, opts.closeLinkEvents, function( e ) {
        self.close();
        e.preventDefault();
    });

By your estimation, would these not also have a negative impact on performance? If they do, are they slated to be addressed in the future? From what I've seen, it seems that there's not really a standard way to treat close links in jQuery Mobile widgets, should there be?

Thanks again!

[tschettler]

@uGoMobi : Please respond when you get a chance. I have been using this without noticing any performance issues. Could you also expound on why the jQuery "on" method with a selector vs a jQuery object has a negative impact on performance? From what I understand of the "on" function, the exact opposite should be true, since calling it with a selector results in a single delegated event handler.

Thanks very much!

[jaspermdegroot]

@tschettler

Here is a jsPerf test that shows the difference (thanks @gabrielschulhof for setting up the test!).

What we test here is the processing time; the responsiveness. The test shows that our current code is much faster than the code from your PR.
It's not very likely that there are many close links in a panel, otherwise we should also consider the difference in attaching time. Your code probably results in a faster startup.

Re: a standard way to treat close links. For 1.4/1.5 we are focussing on improving performance and reviewing all widgets. In widgets where we treat close links differently we can look into doing it the same way as we do for panels.
However, performance is not the only thing we have to look at. For example, if the issue you reported couldn't be fixed with a refresh method we should have merged your PR. So there might never be one standard way.

Good point about the next event binding in the _bindCloseEvents function in the panel widget. We should look into improving that when we review the panel widget.

Thanks!

[tschettler]

Thanks for the response and for posting the benchmarks @uGoMobi! This benchmark does cover one dimension of the performance, but the other would be to consider using a refresh method on the panel to rebind the event when an item is added. Here's a benchmark for that as well:
http://jsperf.com/delegated-vs-object/7

The performance would obviously degrade for the selector method, as it would require rebinding. This does not include the overhead to perform the other functions in a panel refresh. In my case, I'm using a template to build a list of items within the panel, each with a data-rel=close attribute. This list is updated based on other elements in the UI. I would face a performance penalty if I were to call a refresh on the panel every time the list gets updated. I can see how it would be helpful if there were UI enhancements that needed to be made, but it seems like overkill to call it just to rebind the close event.

The only other option for me would be to not use close buttons and call close on the panel after any of the items are clicked. This is a mediocre workaround at best, I'd prefer to use the semantics of JQM to indicate that these items should close the panel. It seems the best recourse for me may be to use my own custom version of jQuery Mobile for now. I'll revisit it once there is a refresh method to test.

Thanks for indulging me!

[jaspermdegroot]

@tschettler

In my previous comment I wrote: "It's not very likely that there are many close links in a panel, otherwise we should also consider the difference in attaching time. Your code probably results in a faster startup." But in your use case you do have many close links so I understand that you are looking for the code that provides the fastest startup (/refresh).

Just to understand your use case better, why do you add data-rel="close" to each of the list items? Why not adding it to the ul?

[tschettler]

I'm adding it to anchor tags within each li, I hadn't thought of adding it to the ul, but that does make more sense. In the project I'm working on, I switched about half of the pages out for panels after I upgraded to 1.3.0; I probably just changed the hrefs to data-rels without giving it a second thought. I'll give that a try.

Thanks for the suggestion!

[gabrielschulhof]

http://jsperf.com/delegated-vs-object/3 shows the performance hit when using delegated handling and http://jsperf.com/delegated-vs-object/4 shows the performance hit when the selector is not a native selector. Thus, the correct way forward performance-wise is to not use delegated handlers at all, but instead to provide a method that informs about the changes in close-links.

[gabrielschulhof]

Whoops :) Should've refreshed :)