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

perf(carousel): reduce Solr load via lazy loading Related Books Carousels (dev-mohit06) #10388

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dev-mohit06
Copy link

Reduce Solr Load via Lazy Loading Related Books Carousels (dev-mohit06)

Closes #10354

Performance Optimization

Implement lazy loading for Related Books Carousels to reduce unnecessary Solr API calls and improve page load performance.

Technical

  • Used Intersection Observer API for viewport-based loading
  • Added fallback mechanism for browsers without Intersection Observer support
  • Modified carousels_partials.js to fetch carousel data only when visible
  • Minimal changes to existing carousel initialization logic

Testing

  1. Open book pages with Related Books Carousels
  2. Verify carousels do not load immediately on page load
  3. Scroll to trigger carousel loading
  4. Check browser network tab for reduced initial API calls
  5. Test anchor link navigation to carousels
  6. Verify functionality in browsers with and without Intersection Observer

Screenshot

No UI changes - performance optimization

Stakeholders

@RayBB @mekarpeles

Attribution

Code implementation based on recommended approach in issue discussion.

@github-actions github-actions bot added the Priority: 2 Important, as time permits. [managed] label Jan 26, 2025
@codecov-commenter
Copy link

codecov-commenter commented Jan 27, 2025

Codecov Report

Attention: Patch coverage is 0% with 27 lines in your changes missing coverage. Please review.

Project coverage is 17.50%. Comparing base (3025f5d) to head (fb469a6).
Report is 275 commits behind head on master.

Files with missing lines Patch % Lines
...brary/plugins/openlibrary/js/carousels_partials.js 0.00% 23 Missing and 4 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #10388      +/-   ##
==========================================
+ Coverage   17.44%   17.50%   +0.05%     
==========================================
  Files          89       87       -2     
  Lines        4792     4793       +1     
  Branches      848      852       +4     
==========================================
+ Hits          836      839       +3     
+ Misses       3436     3432       -4     
- Partials      520      522       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mekarpeles
Copy link
Member

Thank you, nice work submitting a PR!

@dev-mohit06
Copy link
Author

@mekarpeles What changes should I make since I'm a beginner? I tried reading the repo's docs and did what I understood - please guide me on how to improve the code and merge the branch.

@dev-mohit06
Copy link
Author

dev-mohit06 commented Jan 28, 2025

Hi @mekarpeles, I recently sent you a LinkedIn connection request (Mohit Paddhariya). As a bachelor’s student passionate about open source and tech, connecting with you would mean a lot to me and help guide my career. Hope you can approve it! 😊

Thanks a ton!

Copy link
Collaborator

@RayBB RayBB left a comment

Choose a reason for hiding this comment

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

I tested it locally and it works well.
Maybe we can bump up the root margin so that it loads a bit sooner. Like 500px? I'd say do 50vh but it has to be absolute or percentages.

@mekarpeles do you want to give it a spin and see if this achieves what you'd like it?

@RayBB RayBB added the Needs: Staff / Internal Reviewed a PR but don't have merge powers? Use this. label Jan 28, 2025
@dev-mohit06
Copy link
Author

Hello @RayBB, could you please let me know when my branch is expected to be merged? Thank you!

@RayBB
Copy link
Collaborator

RayBB commented Jan 28, 2025 via email

@dev-mohit06
Copy link
Author

Hi @RayBB and @mekarpeles,

Thank you again for your thoughtful review of my PR implementing lazy loading for the Related Books carousels! Your feedback was incredibly helpful, and I’m grateful for the chance to learn through contributing to such a meaningful project.

As a bachelor’s student passionate about open-source development, I’m eager to apply for Google Summer of Code (GSoC) this year and would love to collaborate with the Internet Archive as my chosen organization. I deeply align with the Archive’s mission to provide universal access to knowledge, and I’m committed to contributing meaningfully to its technical ecosystem.

To strengthen my GSoC application and continue growing as a contributor, I’d appreciate your advice on:

  1. Specific technical or community-focused areas where I can make the most impact.
  2. Recommended resources or practices to prepare for GSoC with the Internet Archive.
  3. Any ongoing or upcoming beginner/intermediate-friendly issues I could tackle (e.g., accessibility improvements, performance optimizations, or documentation tasks).

I’m enthusiastic about deepening my collaboration with the team and am open to any suggestions—whether contributing to more issues, participating in discussions, or exploring project ideas aligned with the Archive’s roadmap.

Thank you for your mentorship and for considering my request. I’m excited to keep learning and contributing!

Best regards,
Mohit Paddhariya (@dev-mohit06)

@RayBB RayBB force-pushed the 10354/optimize/related-books-carousel-lazy-loading branch from 2947f59 to fb469a6 Compare January 29, 2025 04:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs: Staff / Internal Reviewed a PR but don't have merge powers? Use this. Priority: 2 Important, as time permits. [managed]
Projects
Status: Waiting Review/Merge from Staff
Development

Successfully merging this pull request may close these issues.

Reduce Solr Load by Switching Related Book Carousels to Lazy Load only after visible in viewport
4 participants