-
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
Fix Universal Viewer embed code #3341
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.
Looks good, but it seems like two distinct changes which would have been easier to review if each was its own pr.
5ef1356
to
40f8576
Compare
@jcoyne good spot, there is a separate PR for the NL translation, which I've backed out of here. |
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.
👍 Looks good, but there is a failing test that is testing exact equality that needs to get updated here: https://github.com/samvera/hyrax/blob/master/spec/features/work_show_spec.rb#L46
It would probably be good to update the comment above that line as well.
8ca1ccf
to
24308ae
Compare
The travis builds got all in a tangle when there were problems the other day, and now I can't seem to get them to run. Circle CI passed, is that enough?! |
8f1d236
to
9d4fe69
Compare
and respond with json to html get request to the manifest url
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.
👍
I restarted the failed PR builds and now they don't have any test failures but are still exiting with an error. I'm going to go ahead and approve and merge this PR. |
UV data-uri needs full URI otherwise the embed code won't work on other sites
Also, external sites will make a standard get request to the manifest, so we need to return the json manifest for both a json and html request