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.

Sign up for GitHub

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

Jump to bottom

Move ajax-loader.gif for better caching #9104

Closed
RayBB opened this issue Apr 17, 2024 · 5 comments · Fixed by #9167
Closed

Move ajax-loader.gif for better caching #9104

RayBB opened this issue Apr 17, 2024 · 5 comments · Fixed by #9167
Assignees
@Spedi
Labels
Good First Issue Easy issue. Good for newcomers. [managed] Lead: @RayBB Issues overseen by Ray (Onboarding & Documentation Lead) [manages] Needs: Response Issues which require feedback from lead Needs: Staff / Internal Reviewed a PR but don't have merge powers? Use this. Priority: 3 Issues that we can consider at our leisure. [managed] Type: Bug Something isn't working. [managed] Type: Refactor/Clean-up Issues related to reorganization/clean-up of data or code (e.g. for maintainability). [managed]

Comments

@RayBB
Copy link
Collaborator

RayBB commented Apr 17, 2024 •
edited

Problem

This PR can't be worked on until the following is merged:
#8930

Currently we have different loader files:

  • /static/images/ajax-loader.gif used nowhere
  • static/css/ajax-loader.gif used in two places. But the it is only gif in this folder.
    • static/css/lib/slick-theme.css
    • static/css/components/tagging-menu.less

What we should do is:

  1. Move static/css/ajax-loader.gif -> /static/images/ajax-loader.gif (overwriting/deleting the image previously there)
  2. Update all the references from to reflect this new move

Why?
When I was working on the service worker caching PR (#8930) I had to make a special case to cache this gif because it is in a weird place.

Reproducing the bug

No response

Context

No response

Notes from this Issue's Lead

Proposal & constraints

Related files

Stakeholders
@RayBB RayBB added Type: Bug Something isn't working. [managed] Needs: Triage This issue needs triage. The team needs to decide who should own it, what to do, by when. [managed] State: Blocked Work has stopped, waiting for something (Info, Dependent fix, etc. See comments). [managed] Needs: Lead Type: Refactor/Clean-up Issues related to reorganization/clean-up of data or code (e.g. for maintainability). [managed] labels Apr 17, 2024
@mekarpeles mekarpeles added Priority: 3 Issues that we can consider at our leisure. [managed] Good First Issue Easy issue. Good for newcomers. [managed] Lead: @RayBB Issues overseen by Ray (Onboarding & Documentation Lead) [manages] and removed Needs: Triage This issue needs triage. The team needs to decide who should own it, what to do, by when. [managed] Needs: Lead labels Apr 22, 2024
@Spedi
Copy link
Contributor

Spedi commented Apr 22, 2024

Hi!
Once #8930 is merged, is there any chance that I can tackle this issue?

@RayBB RayBB removed the State: Blocked Work has stopped, waiting for something (Info, Dependent fix, etc. See comments). [managed] label Apr 23, 2024
@RayBB
Copy link
Collaborator Author

RayBB commented Apr 23, 2024

@Spedi you can go ahead and work on it now.
Since the PR I wanted merged first is taking longer than expected I think it is fine to just do this and I'll have to update the other PR later. Good luck!

@Spedi
Copy link
Contributor

Spedi commented Apr 23, 2024

I suppose I solved the issue but, even if I created a new branch, when I try to push the changes the PR contains the changes about Issue #9065 too. Do you have any idea on how to keep the two PR separated?
I supposed creating a new branch would solve the problem, but maybe I'm doing something wrong.
Thank you for helping!

@RayBB
Copy link
Collaborator Author

RayBB commented Apr 24, 2024

@Spedi please branch off master. I'll resolve things with my other branch later.

@github-actions github-actions bot added the Needs: Response Issues which require feedback from lead label Apr 25, 2024
@Spedi Spedi mentioned this issue Apr 28, 2024
Merged
@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 Apr 30, 2024
@RayBB
Copy link
Collaborator Author

RayBB commented Apr 30, 2024

@mekarpeles need staff to merge this one.

@github-actions github-actions bot added the Needs: Response Issues which require feedback from lead label May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Spedi Spedi

Labels
Good First Issue Easy issue. Good for newcomers. [managed] Lead: @RayBB Issues overseen by Ray (Onboarding & Documentation Lead) [manages] Needs: Response Issues which require feedback from lead Needs: Staff / Internal Reviewed a PR but don't have merge powers? Use this. Priority: 3 Issues that we can consider at our leisure. [managed] Type: Bug Something isn't working. [managed] Type: Refactor/Clean-up Issues related to reorganization/clean-up of data or code (e.g. for maintainability). [managed]
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Moved ajax-loader.gif to achieve better caching Spedi/openlibrary
3 participants
@RayBB @mekarpeles @Spedi