-
Notifications
You must be signed in to change notification settings - Fork 124
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
Force loading of UV #2908
Force loading of UV #2908
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 but first try testing on a rebased branch that includes recent turbolinks-related PR that fixes a bunch of turbolinks issues; it may render this PR moot. If so, close it. If not, then merge.
Does this turbolinks related PR impact this work? #2875 |
I tested again locally with the latest from master and was able to reproduce the issue without the code change using these steps:
Applying this PR made this scenario work. So I think this change is still needed and not addressed by the previous turbolinks PR. |
I wouldn't expect #2875 to directly fix, but it might give you an avenue for better integration of this UV flag with turbolinks globally instead of just on this page. I'm not sure of the order of events here, but you could potentially move your fix into the turbolinks:load event https://github.com/samvera/hyrax/pull/2875/files#diff-da58ad8d67bcab5238c1389809939992R3. If the load event happens too late for this, see https://github.com/turbolinks/turbolinks#full-list-of-events |
@jgondron Great suggestion! I tested and that worked so I've updated the PR to use that approach. |
Fixes #2906.
Set flag to false before loading UV embed javascript (https://github.com/pulibrary/pul_uv_rails/blob/master/public/universalviewer/dist/uv-2.0.1/lib/embed.js#L77-L80) to force loading of UV. Without this the UV loading script will sometimes think UV is already loaded and skip the loading. This is probably due to turbolinks keeping the parent window context.
I took this approach of inlining the javascript to keep it close to what it is related to and avoid having to make changes to
pul_uv_rails
oruniversalviewer
. If there is a more elegant solution or if disabling turbolinks is preferred I'd happily close this PR.@samvera/hyrax-code-reviewers