JangoSteve / jQuery-EasyTabs

Easy and flexible jQuery tabbed functionality without all the styling.
http://www.alfajango.com/blog/jquery-easytabs-plugin/
549 stars 203 forks source link

Activate tab fires twice with AJAX tabs, cache set to false #35

Open bknoles opened 12 years ago

bknoles commented 12 years ago

I've got AJAX'd tabs up and running in my project, but I'm having a weird issue where the tab loads twice when the link is clicked. Using the chrome developer tools, it looks like the second call is due to a hash change.

Here is the original call stack:

jQuery.ajaxTransport.send                                        jquery.js:8103
jQuery.extend.ajax                                               jquery.js:7581
jQuery.fn.extend.load                                            jquery.js:7024
$.easytabs.activateTab                                   jquery.easytabs.js:546
$.easytabs.plugin.selectTab                              jquery.easytabs.js:213
$.easytabs.bindToTabClicks                               jquery.easytabs.js:470
jQuery.event.dispatch                                            jquery.js:3257
jQuery.event.add.elemData.handle.eventHandle                     jquery.js:2876

The second call stack is this:

jQuery.ajaxTransport.send                                        jquery.js:8103
jQuery.extend.ajax                                               jquery.js:7581
jQuery.fn.extend.load                                            jquery.js:7024
$.easytabs.activateTab                                   jquery.easytabs.js:546
$.easytabs.plugin.selectTab                              jquery.easytabs.js:217
$.easytabs.plugin.selectTabFromHashChange                jquery.easytabs.js:297
$.easytabs.initHashChange                                jquery.easytabs.js:637
jQuery.event.dispatch                                            jquery.js:3257
jQuery.event.add.elemData.handle.eventHandle                     jquery.js:2876

My js to initialize the easytabs is just

$('#maintabs').easytabs({
  panelContext: $('div#content'),
  cache: false
});

To load the html content, my backend is RoR, and each tab has a link to a controller action that renders an html file without a layout. I can post more details if necessary.

I have the haschange library also, and the back-forward behavior works fine. It reloads the tab once, as expected.

Any idea why this would happen?

JangoSteve commented 12 years ago

You're not using :remote => true on the ajax tab links, are you?

bknoles commented 12 years ago

No, no :remote => true option on the links. I set updateHash: false in the easytabs options and now the load only happens once. It seems that easytabs selects the tabs on click and then again once it detects the history hash change.

For me this solution is ok, but it does come at the cost of not being able to bookmark any of the content tabs. Here's my rails link for one of the tabs:

<%= link_to "Today", {:action => 'showtoday'}, 'data-target' => "#maintabs-1", title: "Today" %>

Maybe it is because my href routes dynamically through the controller instead of a static page?

JangoSteve commented 12 years ago

That's weird, there is a variable called skipUpdateToHash, which prevents this. The controller vs. static page shouldn't change anything.

Can you bind to the easytabs:before event hook of the tab container to see if that is being triggered once or twice?

$('#maintabs').bind('easytabs:before', function() {
  console.log('before hook fired!');
});
bknoles commented 12 years ago

It triggers twice when updateHash: true is in the options, according to the console

JangoSteve commented 12 years ago

Hmm, that's not right. And it's definitely not happening for any of the tabs on the demo/docs page. The only thing I could think of that would cause that is if the hashchange plugin is being included in the page twice.

bknoles commented 12 years ago

The source for the page only shows it included once. I searched through the other js files in the stack to make sure that they don't have the hashchange defined, and I couldn't find anything

I thought for a second that maybe my main js file was included twice (calling easytabs twice on the container), but logging shows it is only called once.

Any other things I can check?

JangoSteve commented 12 years ago

Yeah I wouldn't think that'd it'd be due to easytabs being included twice, because if that were the case, then you'd still see it firing twice even when updateHash was turned off. You're not including both the hash-change plugin and the address plugin are you? Or either of the above along with backbbq?

bknoles commented 12 years ago

I don't have the address plugin... here's the list of what I have

jquery.js jquery_ujs.js jquery.purr.js best_in_place.js jquery.jeditable.mini.js jquery.jeditable.checkbox.js on_the_spot_code.js on_the_spot.js jquery-ui-1.8.16.custom.min.js jquery.ba-hashchange.js jquery.easytabs.js jquery.iframe-transport.js jquery.remotipart.js jquery.spin.js mainpage.js spin.js myapplication.js

myapplication.js is the application.js file

JangoSteve commented 12 years ago

Remotipart, nice :-)

But yeah, I honestly have no idea what's going on.

bknoles commented 12 years ago

haha yea you've made my life pretty easy.

I'm going to just turn off the updatehash. If I decide i really need the back button functionality, I'll revisit.

Thanks for your help!

JangoSteve commented 12 years ago

You don't have a live demo, do you?

bknoles commented 12 years ago

No live demo right now... Everything is running on my local machine

AnrietteC commented 12 years ago

Can I just say, I had EXACTLY the same issue and I solved it by commenting the following line out:

296: plugin.selectTab( $tab );

After that, it seemed to load much smoother and not blink and the hash still changes.

joebernard commented 12 years ago

I found the issue. On line 466, make the following change:

skipUpdateToHash = true;

It was previously set to false. This will prevent EasyTabs from reloading after a hash change.

joebernard commented 12 years ago

Also note that this issue only occurs when cache = false and updateHash = true. All your demos seem to be cached, which is why they don't display the error.

JangoSteve commented 12 years ago

Ah, that's very helpful. Thanks @jebsilver, I'll take a look.

joebernard commented 12 years ago

@JangoSteve I committed 078b63f which should resolve this issue. See comments for more details. I haven't fully tested this yet, so it may act funky with other cache / updateHash setting combinations. Seems to work with cache = false / updateHash = true.

liyo commented 12 years ago

@jebsilver Thanks! Your commit fixed the issue :)

drpwh commented 12 years ago

hmm, i experienced the double load as well, and then tried using the changes @jebsilver committed. these changes did fix the double load, but broke the back and forward buttons. Any ideas? (am using hashchange)

DaAwesomeP commented 11 years ago

First of all, the fix by @jebsilver seems to be on line 480 in version 3.2.0.

@JangoSteve, the current version (3.2.0) does not seem to include the fix. I would like to suggest this plugin to some friends who have been annoyed with the JqueryUI tabs. They have less experience with code and probably have no clue how to fix the issue.

Will the next version include this fix by default?

JangoSteve commented 11 years ago

@DaAwesomeP The fix wasn't included due to the fact that it breaks the forward and back button functionality. I'll have to take a close look when I can get some time.

DaAwesomeP commented 11 years ago

@JangoSteve, thank you for reminding me. I just noticed that the problem occurred to me too. Though, the problem seems more serious. I noticed that the fix completely disables the hash function of easytabs. The tabs work and you can go to a tab by typing in the hash in the URL box of your browser, but the hash is not updated automatically when switching tabs. It occurs in both JQuery 1.10.0 and 2.0.2. Tested in Firefox 21.0 and Chrome 27.0.1453.110 m.

Also, the Jquery Hashchange plugin has not been updated since 2010 and was last tested with JQuery 1.4.2. In fact, the plugin uses

$.browser

which was removed in JQuery 1.9.1. However, I found a fix by adding

$.browser = "navigator.userAgent"

before loading the plugin.

The date of that comment is 9/12/2012, which means that @drpwh must have been using a version around 1.7.2, which is very old. That might explain why the problem got worse.

Take a look at http://benalman.com/projects/jquery-hashchange-plugin/. I know there really isn't really an alternative, but there might be a known bug or some issues on GitHub with the fix.

JangoSteve commented 11 years ago

To be clear, I don't think it's a bug in the hashchange plugin, it seems to be doing as it should. Rather, the line that @jebsilver changed (skipUpdateToHash = false => skipUpdateToHash = true) changes intentional functionality. The original skipUpdateToHash = false is what causes the hash to update (i.e. it's not being skipped). So, changing it to true would disable it, that's expected.

Rather, the bug is the fact that that line is somehow getting called when cache = false but not when cache = true in the config settings. I think the correct answer is to fix this issue further up the call stack than where @jebsilver changed it. I think @AnrietteC's solution may be closer to the right place to fix this, but I'd have to go back and look through it before knowing for sure.

If someone else wants to look through and try to figure it out, that'd be awesome. Otherwise, I'll have to find some time to go through it myself.

DaAwesomeP commented 11 years ago

@AnrietteC 's solution didn't seem to work. However, I found that commenting out

activateTab($clicked, $targetPanel, ajaxUrl, callback);
on line 221 fixes the double-load, but when you click on the current tab, it does not refresh it anymore.

I think I can put an if statement on line 221 that would make sure to only call

activateTab($clicked, $targetPanel, ajaxUrl, callback);
when the current tab is clicked again. How would I check to see if the current tab was reclicked? The new code would be something like:

// Cache is disabled => reload (e.g reload an ajax tab).
} else if ( ! settings.cache ){
        if (CODE FOR DETECTING IF THE CURRENT TAB IS CLICKED AGAIN) {
                activateTab($clicked, $targetPanel, ajaxUrl, callback);
        }
}

I think this will be the fix :smiley:!

JangoSteve commented 11 years ago

We actually have a check elsewhere to see if the clicked tab is the currently active tab, on lines 212 and 216. The way it's done there is:

if( $clicked.hasClass(settings.tabActiveClass) || $clicked.hasClass(settings.collapsedClass) ) {
  ...
}
DaAwesomeP commented 11 years ago

It seems that the new class is set before line 212, 216, and 221, so it still runs the code for all the tabs. Is there a variable that is set or a way to check what the previous tab was? If updateHash is enabled it could be done through the history but that could get complicated.

JangoSteve commented 11 years ago

Hmm, maybe you could use the panel that's attached to the tab with something like:

if ($clicked.data('easytabs').panel.is(':visible'))
DaAwesomeP commented 11 years ago

This is getting really odd. When I just add it to line 221, it stops the double load on the others, but when the same tab is clicked, it breaks the Ajax and tries to redirect to a bad URL. When I add it to all of them, it redirects to the bad URL for all of the tabs. It doesn't make sense to me.

DaAwesomeP commented 11 years ago

@JangoSteve, Aha! When I click on the current tab with the code you suggested in your last comment, the Firefox Error Console says:

Error: TypeError: $clicked.data(...) is undefined
Line: 221

The Code (from line 219 to 224):


// Cache is disabled => reload (e.g reload an ajax tab).
} else if ( ! settings.cache ){
    if ($clicked.data('easytabs').panel.is(':visible')) {
        activateTab($clicked, $targetPanel, ajaxUrl, callback);
    }
}