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

Asynchronously load affiliate links #8824

Merged
merged 4 commits into from
Feb 28, 2024

Conversation

jimchamp
Copy link
Collaborator

@jimchamp jimchamp commented Feb 16, 2024

Closes #8576

Asynchronously loads affiliate links on the book page. Now, on book page load, the prices have been replaced with the same type of loading indicator that is used for the related works carousel. This loading indicator is now a macro, but this can be reverted if another loading indicator should be used (like a skeleton loader).

As of writing, error cases are not handled gracefully (if at all). What should the behavior be if the call for partials fails? Should we retry? Should the loading indicator be replaced by some error message?

Technical

Replaced parameter page with title in the AffiliateLinks.html, as only the title was being accessed on the page object.

In order to simplify updating the view, the content of the affiliate links macro is now nested within a span. This entire span is replaced with the partially rendered HTML on fetch success.

Affiliate links in the DonateModal component are not asynchronously fetched. I'm not sure if this template is actively used today, so maybe it should be removed altogether?

Ready for review when:

  • We've decided what the loading indicator should look like
  • Errors are handled gracefully
  • Linting errors addressed

Testing

General:

  1. Visit a book page.
  2. Quickly try to spot the loading indicator in the databarWork section.
  3. Ensure that the loading indicator is replaced by prices.
  4. Ensure that the same is true in mobile views.

Retry Flow:

  1. Force the /partials handler to return an error.
  2. Visit a book page.
  3. Ensure that the loading indicator is replaced by a retry link.
  4. Click the retry link. Ensure that only one retry link is rendered after the second failed attempt to fetch partials.
  5. Revert changes to the /partials handler.
  6. Click the retry link again. Ensure that the retry link is replaced by affiliate links.

Screenshot

Screenshot from 2024-02-15 16-16-58
Loading indicator in desktop view.

Screenshot from 2024-02-15 16-17-28
Loading indicator in mobile view.

Screenshot from 2024-02-26 17-03-34
Retry link in desktop view.

Screenshot from 2024-02-26 17-03-55
Retry link in mobile view.

Stakeholders

@jimchamp jimchamp added the On testing.openlibrary.org This PR has been deployed to testing.openlibrary.org for testing label Feb 21, 2024
@mekarpeles mekarpeles self-assigned this Feb 26, 2024
@jimchamp jimchamp marked this pull request as ready for review February 27, 2024 01:06
@jimchamp jimchamp added On testing.openlibrary.org This PR has been deployed to testing.openlibrary.org for testing and removed On testing.openlibrary.org This PR has been deployed to testing.openlibrary.org for testing labels Feb 27, 2024
@jimchamp jimchamp changed the title Lazy-load affiliate links Asynchronously load affiliate links Feb 27, 2024
@mekarpeles mekarpeles merged commit 2aeda27 into internetarchive:master Feb 28, 2024
4 checks passed
Achorn pushed a commit to Achorn/openlibrary that referenced this pull request Mar 14, 2024
* Fetch affiliate links client-side
* Hide loading indicator from the start
* Allow for retries

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
@jimchamp jimchamp removed the On testing.openlibrary.org This PR has been deployed to testing.openlibrary.org for testing label Apr 2, 2024
@jimchamp jimchamp deleted the 8576/feature/async-prices branch April 24, 2024 00:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make BWB + Amz prices lazy load on editions page
2 participants