Added support for loading all record tabs, including the initial one,…

[EreMaijala]

… via ajax.

I've tried to make sure that:

1.) The hash is not updated on initial load of the record page
2.) The hash is updated on moving to other tabs
3.) The hash is also updated when moving back to the initial tab

Moving some of the code out of the success handler in record.js helps update the tab state faster when loading the tab takes time.

[demiankatz]

An issue and a question:

Issue: This mechanism does not appear to take into account the possibility of tabs that do not support AJAX (we have a few that simply cannot function if loaded asynchronously -- for example, Google Maps). There's a supportsAjax() method on the RecordTab classes which, when it returns false, causes a .noajax class to be applied to the associated tab. When this class is present, clicking the tab should reload the whole page instead of triggering AJAX. We should also think about how to support this inside the default tab.

Question: Does it make sense to update the hash when returning to the initial tab? Wouldn't it be better to simply clear it? That is, if we start on Record/ABC with tab 1 selected, then we click on tab 2 and go to Record/ABC#tab2, and then click back on tab1, shouldn't we go back to Record/ABC instead of to Record/ABC#tab1? I think we should be able to support this given that we're tracking which tab was initially opened.... but maybe there's some wrinkle I'm not thinking of.

Thanks!

[EreMaijala]

Maybe I'm missing something, but both changes in view.phtml include a supportsAjax() check.

About adding the hash, that's just how things work with ajaxTabs disabled too, so I didn't change it. I think there are arguments for both behaviors, but the reason I decided to leave it as is was that it makes the tab click behavior consistent as it causes the browser to scroll to the tab (given enough content). It could be changed, but I think that's a separate issue.

[demiankatz]

Strange -- I thought I was able to break things by adding supportsAjax() methods that returned false to the tab classes... but as of this morning, it seems to be working correctly. I think perhaps I was confused because the way you rearranged the success handler causes the "loading..." spinner to show up while the page reloads, even when the tab isn't being loaded via AJAX. (That's not a bad thing -- I just may have assumed I was seeing AJAX when I actually wasn't).

I see what you're saying about the hash as well. We can leave that alone.

I also see that we now have some merge conflicts here due to @crhallberg's latest JS refactoring. I'll ask him to take a look to see if we can save you any trouble on conflict resolution.

[crhallberg]

I've taken a long look and I'm confident that your code is great and any suggestions I would make are overly complicated compared to this. May do a couple style fixes after we merge, but overall I don't see any possible conflicts with refactoring.

[crhallberg]

Would you prefer a PR on your branch to fix the merge conflicts or would you prefer if I manually merge this in?

[EreMaijala]

It'd be great if you could do a manual merge. Thanks!

[demiankatz]

One more thing to do before we merge: recent discussion on #205 reminded me of the hideHoldingsTabWhenEmpty setting. I expect that this new code will still respect that, but it's a setting we should probably test to be on the safe side. @crhallberg, let's look at this together a little later today if time permits.

[EreMaijala]

According to my tests hideHoldingsTabWhenEmpty works properly, but it won't hurt to double-check. Nevertheless this should not care at all which tabs are displayed.

[demiankatz]

Unfortunately, Chris is out sick today, and I'll be on vacation soon, so this may get held up until after the Thanksgiving holiday. Hopefully we can get it sorted out shortly after that!