Fix/homepage preview button#12036
Merged
mekarpeles merged 6 commits intoMar 11, 2026
Merged
Conversation
Updated BookPreview macro to return a div element.
…ically-added buttons. Update related selectors and remove unnecessary initialization in lazy-carousel.js.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Builds on #11903 — Closes #11899
This PR takes the fix from #11903 and improves the JavaScript approach to be less brittle and more maintainable.
Technical
Changes carried over from #11903:
index.html: Added$:macros.BookPreview('')to render the#bookPreviewmodal HTML on the homepage so it's available when lazy-loaded carousel buttons are clickedBookPreview.html: Added$if ocaid:guard to prevent rendering a broken preview button when noocaidis providedtest_home.py: Updated test to account for the newBookPreviewmacro call on the homepageImprovements over #11903:
Event delegation instead of re-initialization — The original fix imported
initPreviewDialogsintolazy-carousel.jsand called it after each AJAX load. This creates tight coupling: every future dynamic content loader would need to know about and manually callinitPreviewDialogs. Instead, this PR uses a single delegated click handler ondocument($(document).on('click.bookPreview', '[data-book-preview]', ...)), so any preview button works automatically regardless of when it's added to the DOM. This eliminates the import and call fromlazy-carousel.jsentirely.data-book-previewattribute as JS hook — The original fix targeted the CSS class.cta-btn--previewfor JS behavior. This PR adds adata-book-previewattribute and targets that instead, separating styling concerns from behavioral hooks. The CSS class remains for styling; the data attribute is the JS contract. This prevents accidental breakage if someone renames or removes the CSS class.Idempotency guard — Uses jQuery event namespacing (
.off('click.bookPreview').on('click.bookPreview', ...)) so the handler is safe ifinitPreviewDialogs()is ever called more than once, preventing duplicate handler stacking.Fixed module loading guard — Changed the dialog module guard selector in
index.jsfrom.cta-btn--previewto#bookPreview. The guard runs at page load to decide whether to lazy-import the dialog module. With event delegation, preview buttons don't exist at page load (they come from lazy carousels), but#bookPreview(the modal target) does — so the module loads correctly.Testing
Stakeholders
@bhardwajparth51 cc: @mekarpeles