PP-3296 : embed external web viewers within app #89
Conversation
add new internal reader page as a full width dialog
* Demarque web reader links are returned from circulation as type: text/html
pass urls from external web viewers into iframe within app unable to provide any proper error handling because CORS prevents us from checking for successful fetches on redirects
…ing item they have already checked out
tdilauro
left a comment
tdilauro
left a comment
There was a problem hiding this comment.
Overall, this looks pretty good. I've put a first round of comments up. I do have a few security concerns, but I think we can address those after a round or two of deployed testing.
| if (credentials.token?.expirationDate) { | ||
| return isBefore(credentials?.token?.expirationDate, new Date()); | ||
| } | ||
| return true; |
There was a problem hiding this comment.
Why has the check been removed here? Testing, maybe?
There was a problem hiding this comment.
After a period of time away, I would try to read the book and would get a invalid token error. The useSWR hook wasn't revalidating my loans and thus was not refreshing my token. I removed the check so that the token would refresh more often and ensure a valid one was available.
I couldn't figure out why there was inconsistency. The logic on the check seemed correct. It would refresh that token an hour after it was created and continue to keep me signed in.
I should make a ticket to dig deeper.
There was a problem hiding this comment.
We could use some tests here for navigation behavior (back button, onclose goes back, correct readUrl, etc.)
There was a problem hiding this comment.
Fair, I'll add some more.
* ensure navigation works as expected * ensure url provided to external reader has been parsed and is valid, otherwise show 404
Pull Request Test Coverage Report for Build 21071628637Details
💛 - Coveralls |
Description
Previously, users were directed away from the web catalog to view web readers provided by distributors. We will now keep users within the app by rendering any provided web reader URL within an embedded iframe. While we cannot control how distributors decide to render their viewers nor can we control how errors will be rendered, any errors will be non-blocking and users will have the ability to exit the screen and return the web catalog.
Motivation and Context
This change provides a more integrated experience for users by keeping them within the app.
Jira PP-3296
How Has This Been Tested?
Added tests to ensure the loading and loaded states render properly. Also manually tested loans that successfully and unsuccessfully loaded within the iframe to see how the app handled those scenarios.
Checklist: