Fix carousel retry link refreshing all carousels#11418
Fix carousel retry link refreshing all carousels#11418cdrini merged 1 commit intointernetarchive:masterfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the lazy carousel JavaScript code to improve error handling and code quality. The changes eliminate jQuery usage in favor of vanilla JavaScript and add a null check for an optional DOM element.
Key Changes
- Replaced jQuery event listener with vanilla JavaScript
addEventListenerfor the retry button - Added null safety check for the
.lazy-carousel-fallbackelement before manipulating its class list - Updated comment from plural "listeners" to singular "listener" to match the actual behavior
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| retryElem.addEventListener('click', (e) => { | ||
| e.preventDefault() | ||
| handleRetry(elem); | ||
| }) |
There was a problem hiding this comment.
Potential null dereference if .retry-btn element is not found. Add a null check before calling addEventListener, similar to the null check added for carouselFallbackElem in the handleRetry function.
| retryElem.addEventListener('click', (e) => { | |
| e.preventDefault() | |
| handleRetry(elem); | |
| }) | |
| if (retryElem) { | |
| retryElem.addEventListener('click', (e) => { | |
| e.preventDefault() | |
| handleRetry(elem); | |
| }) | |
| } |
cdrini
left a comment
There was a problem hiding this comment.
Great catch! Lgtm. Putting on testing now for a test, but code makes sense to me 👍
|
Tried going offline midway through https://testing.openlibrary.org/collections/the_haunted_library , and confirmed the retry link only reloaded one carousel 👍 |
Follows #11028
Fixes regressions annotated here and here.
Technical
Testing
Screenshot
Stakeholders