Skip to content

Migrate to Interstitial for Trusted Book Provider read actions #10586

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

Conversation

SivanC
Copy link
Contributor

@SivanC SivanC commented Mar 15, 2025

Closes #10561

Feature/refactor

Technical

Add new behavior to borrow API to redirect to an interstitial page in the case that a read/borrow action is requested on a non-IA provided edition. Also removes the existing toasts that are generated when using a direct provider read button link.

Testing

  1. Click a read/borrow button for an edition provided by a trusted third party provider
  2. Observe that you are now redirected to an interstitial page, and that you can observe the countdown timer, cancel the redirection, or immediately follow to the link.
  3. Observe that there is no longer a toast on the original page after clicking the button.

Screenshot

image

Stakeholders

@mekarpeles

@SivanC
Copy link
Contributor Author

SivanC commented Mar 15, 2025

There were a couple URLs that I wasn't sure behaved the same way as the standard read/borrow action URls that I haven't touched, those being the librivox template (because it's an audio link) and the direct provider preview link (uses the same URL as the regular read link did but not sure if we want the same behavior?)

@mekarpeles mekarpeles changed the title Clean up interstitial template and add borrow API behavior for redire… Migrate to Interstitial for Trusted Book Provider read actions Mar 15, 2025
Copy link
Member

@mekarpeles mekarpeles left a comment

Choose a reason for hiding this comment

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

The Interstitial design / presentation needs a bit of work; some suggestions provided

@github-actions github-actions bot added the Needs: Response Issues which require feedback from lead label Mar 16, 2025
@SivanC
Copy link
Contributor Author

SivanC commented Mar 19, 2025

Marking as ready for review according to discussion from the community meeting. For some reason I am unable to get the template CSS to work on my end, but the lines that could be added optimally are $putctx('cssfile', 'interstitial') in templates/interstitial.html and .interstitial { padding: 15%; } in a new file static/css/components/interstitial.less

Copy link
Collaborator

@jimchamp jimchamp left a comment

Choose a reason for hiding this comment

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

Thanks @SivanC!

I left some review notes, and am happy to merge this once those are addressed.

The main issues are:

  1. Interstitial page needs some margins
  2. Some aria attributes are no longer appropriate without toast messages
  3. JS in template should be moved to Webpack (cancel link code is optional)


<p>
<a href="#" onclick="window.close(); return false;">$_("Cancel")</a> |
Copy link
Collaborator

Choose a reason for hiding this comment

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

This isn't necessary for this PR, but noting that we may want to move this into Webpack in the future. This could be a good first issue, and the solution could be modeled on our recent cancel link changes (#9795).

Definitely not a blocker.

@jimchamp jimchamp added Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed] and removed Needs: Response Issues which require feedback from lead labels Mar 20, 2025
@SivanC
Copy link
Contributor Author

SivanC commented Mar 20, 2025

@jimchamp Thanks for the detailed review! I'll take a crack tomorrow, haven't dived too deep into the JS side of things on the project so might shoot you some questions if I get stuck somewhere

@github-actions github-actions bot added the Needs: Response Issues which require feedback from lead label Mar 20, 2025
@mekarpeles mekarpeles removed the Needs: Response Issues which require feedback from lead label Mar 20, 2025
Copy link
Collaborator

@jimchamp jimchamp left a comment

Choose a reason for hiding this comment

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

Thanks for the quick turn-around, @SivanC! I'm going to make a small commit to remove my unhelpful comments, then merge. This is great!

@jimchamp jimchamp merged commit 6978961 into internetarchive:trustedbookprovider-interstitial Mar 20, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants