Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added listener to close map tab popups #817

Closed
wants to merge 4 commits into from
Closed

Added listener to close map tab popups #817

wants to merge 4 commits into from

Conversation

lmgonzales
Copy link
Contributor

Bug fix for Map View functionality in VuFind 3.1. Added listener to record.js that closes any Map View popups if the user switches to any record tab other than the Map View tab.

@demiankatz
Copy link
Member

@lmgonzales, thanks for the fix -- though I think we may be able to refine it a little bit before we merge it.

First of all, I think this can actually be simplified a little bit -- rather than using getElementById and then wrapping the element with jQuery, you can probably grab it directly with a jQuery selector like $('#popup').

The other disadvantage to this approach is that we end up with some map-specific logic embedded in the more generic record.js. @crhallberg, might there be some mechanism to attach this action from within the map code so that we can put it in a more appropriate place? Or alternatively, might it make sense to leave it where it is but remove the "if != map" check, since in theory any popovers should probably close when we switch tabs -- that would at least remove the map-specific aspect of the code if we have to leave it in a shared, generic place.

@crhallberg
Copy link
Contributor

There are event hooks for tab switching, it depends of what balance of simplicity vs modularity you want. A listener in the map file might look like this:

$('.record-tabs .map').on('hide.bs.tab', function () {
  $('#popup').popover('destroy');
});

@lmgonzales
Copy link
Contributor Author

Thanks @crhallberg! That works beautifully. I backed the changes out of record.js. @demiankatz, I was about to add the code to the map_tab_ol.js file, and am not sure what happened, but I can't find any of the geographic code in the master branch or this branch. Any ideas? Thanks!

@demiankatz
Copy link
Member

@lmgonzales, your branch was a bit behind the current master. I've brought it up to date. Please let me know if the file is still missing.

@lmgonzales
Copy link
Contributor Author

Thanks @demiankatz! That worked and I've applied the code to the map_tab_ol.js file.

@demiankatz
Copy link
Member

Looks great! I've merged this to the release-3.1 and master branches.

@demiankatz demiankatz closed this Sep 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants