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

Open direct bulk export to ”<option>Main” window like the single reco… #871

Merged
merged 4 commits into from Dec 21, 2016

Conversation

EreMaijala
Copy link
Contributor

…rd exports and trigger the change function also for the initially selected option.

…rd exports and trigger the change function also for the initially selected option.
@demiankatz
Copy link
Member

Any chance you can elaborate on what problem you are fixing or what inconsistency you are addressing? I'm not exactly sure what to watch for while testing. (Sorry if it's obvious -- I haven't touched this code in a while, so a glance at the diffs isn't telling me much).

@EreMaijala
Copy link
Contributor Author

EreMaijala commented Dec 19, 2016

If you export a single record from the full record view, target="Main" (e.g. RefWorksMain) is added to any redirect-style export elements (this is existing functionality separate from the export form here). This allows VuFind to stay in one tab/window while the system where the export is going is opened in another, and at least our users would like the bulk export to behave the same way. Previously the bulk export would open in the same window and the user would lose the context in VuFind.

@demiankatz
Copy link
Member

Thanks for the clarification! I'll give this a closer look (and compare notes with @crhallberg) in the near future.

@@ -61,10 +61,16 @@
$('#format').change(function exportFormatChange(e) {
if (this.selectedOptions[0].getAttribute('data-redirect') === null) {
$('.export.btn').removeAttr('data-lightbox-ignore');
$('.export.btn').unbind('click');
$('form[name=exportForm]').removeAttr('target');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure we want to remove the target from the form? I have a feeling this might cause problems in the Lightbox.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can see there's no target in the form by default (that would cause it to open in another window in all cases!). It's only added by the "else" branch below the quoted part.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've of course also tested it with the lightbox and it works fine for me.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My apologies, I mixed up target and action. You're correct.

@demiankatz
Copy link
Member

demiankatz commented Dec 20, 2016

During debugging, Chris and I discovered a problem -- when the click action is unbound, things break. So this works if you go directly to an external export action, but if you click to an internal, download-oriented export (like MARC), and then switch over to an external action (like RefWorks), everything blows up. (By "blows up" I mean that if you try to perform the export, multiple lightboxes flash by, and then you're left with a non-functional grayed-out screen and no export tab).

We discovered this because we were trying to refactor the code to prevent the problem of the click action getting bound over and over again if you switch back and forth between two external actions (e.g. flipping back and forth between RefWorks and EndNoteWeb a dozen times will cause a dozen different click actions to be bound and executed). However, this other problem seems even more serious -- we just didn't notice this until we tried to test uniform unbinding.

Any ideas? Am I making any sense? (It's the end of the day -- my brain may be a bit foggy).

@EreMaijala
Copy link
Contributor Author

Ah, thanks for catching that and trying to fix it. Binding multiple times is bad, indeed, but so is unbinding since it will remove Lightbox's handler too. How about the just-committed version? This always adds the click handler (once), but it doesn't do anything for a download-oriented export.

@crhallberg
Copy link
Contributor

I think this is much better. Excellent. I'm going to look into a solution to so easily erasing Lightbox's bindings.

@demiankatz demiankatz merged commit c95871e into vufind-org:master Dec 21, 2016
EreMaijala referenced this pull request in NatLibFi/NDL-VuFind2 Dec 22, 2016
- Open direct bulk export to ”<option>Main” target (i.e. new tab/window) like the single record exports
- Trigger the change function for the initially selected option for consistency
@EreMaijala EreMaijala deleted the bulk-export-fix branch December 29, 2016 08:56
olli-gold added a commit to tubhh/vufind that referenced this pull request May 23, 2017
olli-gold added a commit to tubhh/vufind that referenced this pull request May 23, 2017
olli-gold added a commit to tubhh/vufind that referenced this pull request Nov 14, 2018
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