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

Reduce Solr Load by Switching Related Book Carousels to Lazy Load only after visible in viewport #10354

Closed
mekarpeles opened this issue Jan 17, 2025 · 21 comments · Fixed by #10388
Assignees
Labels
Good First Issue Easy issue. Good for newcomers. [managed] Lead: @RayBB Issues overseen by Ray (Onboarding & Documentation Lead) [manages] Module: Solr Issues related to the configuration or use of the Solr subsystem. [managed] Needs: Staff / Internal Reviewed a PR but don't have merge powers? Use this. Priority: 2 Important, as time permits. [managed] Theme: Performance Issues related to UI or Server performance. [managed] Type: Feature Request Issue describes a feature or enhancement we'd like to implement. [managed]

Comments

@mekarpeles
Copy link
Member

mekarpeles commented Jan 17, 2025

TL;DR

Can we optimize RelatedCarousels (that show up on every book page so that these carousels (which are already async loaded by a partial) only fetch once the component is in viewport (via js)?

Background

Openlibrary.org is a web site that has a page for every book published. The main page is the "book" page (which showcases information about a book). One section/component is called Related Books Carousel which currently loads async, far below the fold. It calls an endpoint call /partials.json?component=relatedBooks. We'd like to change this code so that the partial is only called when the component comes into view or the patron is scrolling towards it (i.e. it's in range of the viewport). One consideration is the patron may click an anchor link (jump to section) to get to the carousels and so relying on scrolling may not be sufficient.

We could implement with a library like lozad or vanilla-lazyload.

Solution

ChatGPT recommends the Intersection Observer API gating our existing fetch code here

export function initCarouselsPartials() {
const fetchRelatedWorks = function() {
$.ajax({
url: '/partials.json',
type: 'GET',
data: {
workid: $('.RelatedWorksCarousel').data('workid'),
_component: 'RelatedWorkCarousel'
},
datatype: 'json',
success: function (response) {
$('.loadingIndicator.carousel-loading').addClass('hidden');
if (response){
response = JSON.parse(response)
$('.RelatedWorksCarousel').append(response[0]);
$('.RelatedWorksCarousel .carousel--progressively-enhanced')
.each((_i, el) => new Carousel($(el)).init());
}
}
});
};
$('.loadingIndicator.carousel-loading').removeClass('hidden');
fetchRelatedWorks();
}
:

e.g.

    const observerOptions = {
        root: null, // Observes the viewport
        rootMargin: '200px', // Trigger loading slightly before entering the viewport
        threshold: 0, // Trigger when any part of the element is visible
    };

    const observerCallback = function (entries, observer) {
        entries.forEach(entry => {
            if (entry.isIntersecting) {
                observer.unobserve(entry.target); // Stop observing once triggered
                fetchRelatedWorks();
            }
        });
    };

    const observer = new IntersectionObserver(observerCallback, observerOptions);

    observer.observe(carouselElement);

    // Ensure loading happens if the user directly navigates via anchor link
    if (window.location.hash && window.location.hash === `#${carouselElement.id}`) {
        fetchRelatedWorks();
    }

Process Notes

We may wish to coordinate with staff turn off related book carousels on book pages for some period of time (like 1h) to see how overall performance is impacted before moving forward with implementation.

Justification

Improve Open Library performance by decreasing number of calls to solr API

Breakdown

Requirements Checklist

  • [ ]

Related files

  • export function initCarouselsPartials() {
    const fetchRelatedWorks = function() {
    $.ajax({
    url: '/partials.json',
    type: 'GET',
    data: {
    workid: $('.RelatedWorksCarousel').data('workid'),
    _component: 'RelatedWorkCarousel'
    },
    datatype: 'json',
    success: function (response) {
    $('.loadingIndicator.carousel-loading').addClass('hidden');
    if (response){
    response = JSON.parse(response)
    $('.RelatedWorksCarousel').append(response[0]);
    $('.RelatedWorksCarousel .carousel--progressively-enhanced')
    .each((_i, el) => new Carousel($(el)).init());
    }
    }
    });
    };
    $('.loadingIndicator.carousel-loading').removeClass('hidden');
    fetchRelatedWorks();
    }

Stakeholders


Instructions for Contributors

Please run these commands to ensure your repository is up to date before creating a new branch to work on this issue and each time after pushing code to Github, because the pre-commit bot may add commits to your PRs upstream.

@mekarpeles mekarpeles added Lead: @RayBB Issues overseen by Ray (Onboarding & Documentation Lead) [manages] Module: Solr Issues related to the configuration or use of the Solr subsystem. [managed] Needs: Breakdown This big issue needs a checklist or subissues to describe a breakdown of work. [managed] Priority: 2 Important, as time permits. [managed] Theme: Performance Issues related to UI or Server performance. [managed] Type: Feature Request Issue describes a feature or enhancement we'd like to implement. [managed] labels Jan 17, 2025
@mekarpeles mekarpeles added this to the Sprint 2025-02 milestone Jan 17, 2025
@mekarpeles mekarpeles added Good First Issue Easy issue. Good for newcomers. [managed] and removed Needs: Breakdown This big issue needs a checklist or subissues to describe a breakdown of work. [managed] labels Jan 19, 2025
@ShilpThapak
Copy link

Hi @mekarpeles, @RayBB,
Can you assign this to me?

@RayBB
Copy link
Collaborator

RayBB commented Jan 19, 2025

@ShilpThapak
Before you go too deep into trying to solve the issue, can you please first help us with breakdown by:

  1. Clarifying what you believe the task to be
  2. Identifying which files you believe may be related from the project
  3. Asking any question(s) you may have about the goal or requirements of this issue
  4. Proposing a solution or approach

@dev-mohit06
Copy link
Contributor

Hi, @mekarpeles, @RayBB
Can you assign that issue to me?
Here is my Proposal

Optimization of Related Books Carousel Loading

Task Breakdown

  1. Clarify the Task:

    • Modify the Related Books Carousel to fetch its data only when the component is in the viewport or about to come into view.
    • Ensure the solution accounts for scenarios where users click anchor links to jump directly to the carousel.
  2. Files Likely to Be Involved:

    • Backend:
      • Endpoint: /partials.json?component=relatedBooks.
    • Frontend:
      • Book page templates (HTML/JS/CSS).
      • JS modules/scripts handling Related Books Carousel.
      • Global JS utilities (if applicable for lazy loading or fetch management).
    • SCSS/CSS for any carousel-specific styles.
  3. Questions for Clarification:

    • Is a specific JavaScript framework/library in use (e.g., React, Vue.js, or plain JS)?
    • Are there existing utilities for lazy loading in the codebase that need to be reused?
    • Should there be a fallback mechanism for browsers without Intersection Observer support?
    • Is performance optimization critical? Should we prefetch data for carousels near the viewport?

Proposed Solution

Approach

  • Use the Intersection Observer API to load carousel data dynamically when the component enters the viewport.
  • Handle anchor link scenarios by:
    1. Running the observer logic on DOM load.
    2. Checking if the component is already visible on page load.

Implementation Steps

  1. Mark Carousel Containers:

    • Add a data-lazy-carousel attribute to the container for lazy loading.
    • Include a data-fetch-url attribute for the endpoint, e.g.:
      <div class="carousel" data-lazy-carousel data-fetch-url="/partials.json?component=relatedBooks"></div>
  2. JavaScript Implementation:

    document.addEventListener('DOMContentLoaded', () => {
      const carousels = document.querySelectorAll('[data-lazy-carousel]');
    
      const fetchCarouselData = (element) => {
        const url = element.dataset.fetchUrl;
        fetch(url)
          .then((response) => response.text())
          .then((html) => {
            element.innerHTML = html;
          })
          .catch((error) => console.error('Failed to load carousel:', error));
      };
    
      const observer = new IntersectionObserver((entries, observer) => {
        entries.forEach((entry) => {
          if (entry.isIntersecting) {
            const target = entry.target;
            fetchCarouselData(target);
            observer.unobserve(target);
          }
        });
      });
    
      carousels.forEach((carousel) => observer.observe(carousel));
    });
  3. Fallback for Non-Supporting Browsers:

    • Use a polyfill or default to fetching all carousels on page load for older browsers.

Benefits

  • Reduced initial load time since the carousel data is fetched only when needed.
  • Better performance for users with slower networks.
  • The solution is flexible and can be extended to other components.

@github-actions github-actions bot added the Needs: Response Issues which require feedback from lead label Jan 22, 2025
@ShilpThapak
Copy link

@RayBB, you can assign this issue to Mohit if you like his proposal.
I am a bit occupied at the moment.

@dev-mohit06
Copy link
Contributor

Thank you, @ShilpThapak, for referring me!

@RayBB
Copy link
Collaborator

RayBB commented Jan 22, 2025

@dev-mohit06 sounds like a good proposal.
To answer your questions

Is a specific JavaScript framework/library in use (e.g., React, Vue.js, or plain JS)?

  • In this case, I think plain js is preferable.

Are there existing utilities for lazy loading in the codebase that need to be reused?

  • Not that I'm aware of. We lazy load images but that's much simpler. You can search the codebase a bit and double check though.

Should there be a fallback mechanism for browsers without Intersection Observer support?

  • We should fallback to loading right away

Is performance optimization critical? Should we prefetch data for carousels near the viewport?

  • Yes the performance is critical. We should only load the carousels when they're near the viewport.

Let me know if you have further questions.

@dev-mohit06
Copy link
Contributor

As of now, I don’t have it, but if I do, I will definitely ask.

@PredictiveManish
Copy link
Contributor

As of now, I don’t have it, but if I do, I will definitely ask.

Are you working on this? If not then I can work on this.

@dev-mohit06
Copy link
Contributor

dev-mohit06 commented Jan 24, 2025

@PredictiveManish Yes, I am currently working on it and figuring out the best approach to proceed.

@RayBB RayBB removed the Needs: Response Issues which require feedback from lead label Jan 24, 2025
@github-actions github-actions bot added the Needs: Response Issues which require feedback from lead label Jan 25, 2025
@dev-mohit06
Copy link
Contributor

@RayBB, I am experiencing some issues while setting up the Open Library project. Could you please assist me in resolving them?

@RayBB
Copy link
Collaborator

RayBB commented Jan 26, 2025

Please explain the issue in detail with the exact commands you run and errors you see

@PredictiveManish
Copy link
Contributor

PredictiveManish commented Jan 26, 2025

Please explain the issue in detail with the exact commands you run and errors you see

After all the steps when I am giving the command

docker compose build

it is not building images completely but it shows created everything, but when I use

docker compose up

It doesn't starts the setup
I tried multiple times but facing the same issues I am seeing the images

docker images

Showing all images but unable to see project locally

I mentioned the issue in discussion channel on gitter.

@PredictiveManish
Copy link
Contributor

Image

These are not starting as it is showing like this

exec docker/ol-covers-start.sh: no such file or directory

@dev-mohit06
Copy link
Contributor

Hi @RayBB, I’m facing an issue while trying to run OpenLibrary using Docker on my MacBook Air with an M2 chip. After running the docker-compose up command, I encountered the following error

Image

Could you help me figure out what’s going wrong? Thanks!

@dev-mohit06
Copy link
Contributor

@PredictiveManish, I’ve been assigned this issue by @RayBB, so please refrain from working on it. Feel free to focus on other tasks.

@dev-mohit06
Copy link
Contributor

@RayBB, it might be helpful to remove the “Needs: Response” tag from this issue, as I’ve been assigned to resolve it. Thank you!

@ShilpThapak
Copy link

@dev-mohit06, can you try to update the file docker/Dockerfile.oldev
from:
FROM openlibrary/olbase:latest
to:
FROM --platform=linux/amd64 openlibrary/olbase:latest

@dev-mohit06
Copy link
Contributor

@ShilpThapak Thanks a lot

@PredictiveManish
Copy link
Contributor

@PredictiveManish, I’ve been assigned this issue by @RayBB, so please refrain from working on it. Feel free to focus on other tasks.

Brother I am not working on this issue, I am working on other issue. I was just have to reset my laptop due to some issues and now when I am setting up the project I am facing some trouble so that's why I had sent these images here as someone else is also facing trouble.

@dev-mohit06
Copy link
Contributor

Thanks for clarifying, @PredictiveManish. Appreciate you letting me know. If you’re facing issues while setting up the project, feel free to share the details, and I’ll try to help if I can.

@PredictiveManish
Copy link
Contributor

Thanks for clarifying, @PredictiveManish. Appreciate you letting me know. If you’re facing issues while setting up the project, feel free to share the details, and I’ll try to help if I can.

If you can see in gitter community I had shared my issues, or please see it above messaging again will make trouble to others too.

@RayBB RayBB added Needs: Staff / Internal Reviewed a PR but don't have merge powers? Use this. and removed Needs: Response Issues which require feedback from lead labels Jan 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good First Issue Easy issue. Good for newcomers. [managed] Lead: @RayBB Issues overseen by Ray (Onboarding & Documentation Lead) [manages] Module: Solr Issues related to the configuration or use of the Solr subsystem. [managed] Needs: Staff / Internal Reviewed a PR but don't have merge powers? Use this. Priority: 2 Important, as time permits. [managed] Theme: Performance Issues related to UI or Server performance. [managed] Type: Feature Request Issue describes a feature or enhancement we'd like to implement. [managed]
Projects
None yet
5 participants