Open additional story images in an inline lightbox#1550
Conversation
Published stories link each additional (gallery) image to its full-size file, so clicking one navigated the viewer away from the story and gave no way to browse the images as a set. Wrap gallery images in a lightbox that opens the image in place and lets the viewer scroll through the whole set with next/previous controls, arrow keys, Escape, and a backdrop click. The underlying links are preserved as a no-JS fallback. Closes #1520 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
| backdropClose(event) { | ||
| if (event.target === this.modalTarget) this.close() | ||
| } | ||
|
|
There was a problem hiding this comment.
The full-size image URL is read from each item's href rather than a separate data attribute. That keeps a single source of truth and means the anchors double as a no-JS fallback (they still link to the file when Stimulus isn't loaded).
| const count = this.itemTargets.length | ||
| this.indexValue = (this.indexValue - 1 + count) % count | ||
| } | ||
|
|
There was a problem hiding this comment.
Prev/next wrap around the set (modulo length) so the viewer can keep scrolling in either direction without hitting a dead end.
| <% gallery_assets.each_with_index do |gallery_assets, idx| %> | ||
| <%= render "assets/display_image", item: gallery_assets, idx: idx, variant: :gallery, link: (defined?(link) ? link : nil) %> | ||
| <% gallery_assets.each_with_index do |gallery_asset, idx| %> | ||
| <% if link && gallery_asset.file.content_type.to_s.start_with?("image/") %> |
There was a problem hiding this comment.
Lightbox is gated on link (the existing clickable-gallery flag) AND the asset being an image. Non-image gallery assets (e.g. PDFs) fall through to the original display_image rendering, and galleries rendered without link (e.g. reports) keep their previous non-clickable behavior.
| <% if link && gallery_asset.file.content_type.to_s.start_with?("image/") %> | ||
| <a href="<%= url_for(gallery_asset.file) %>" | ||
| class="flex grow" | ||
| data-lightbox-target="item" |
There was a problem hiding this comment.
The anchor keeps href pointing at the full-size file (no-JS fallback) and data-action="lightbox#open:prevent" intercepts the click when JS is active. Inner image is rendered with link: false so we don't nest anchors.
There was a problem hiding this comment.
Pull request overview
Adds a Stimulus-powered inline lightbox for “additional images” (gallery assets) so clicking an image opens a modal in-place with next/previous navigation, instead of navigating away to the raw file URL. This implements the requested “browse as a set” behavior for published stories (and other views that render the gallery with links enabled) while keeping the existing full-size href as a no-JS fallback.
Changes:
- Add a
lightboxStimulus controller to open/close a modal, track current image index, and navigate via controls/keyboard. - Update the gallery media partial to wrap image items in lightbox-enabled links and render the lightbox modal markup when linking is enabled.
- Add a Selenium system spec covering opening/navigating/closing the lightbox and document the new controller in
AGENTS.md.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
spec/system/story_gallery_lightbox_spec.rb |
Adds a system spec validating the lightbox UX for story additional images. |
app/views/assets/_lightbox.html.erb |
Introduces the modal markup (image, counter, prev/next/close controls). |
app/views/assets/_display_gallery_media.html.erb |
Wraps image gallery items as lightbox “item” targets and conditionally renders the lightbox UI. |
app/frontend/javascript/controllers/lightbox_controller.js |
Implements the Stimulus controller logic for the lightbox modal and navigation. |
app/frontend/javascript/controllers/index.js |
Registers the new lightbox controller with the Stimulus application. |
AGENTS.md |
Updates the documented Stimulus controller list/count to include lightbox. |
Comments suppressed due to low confidence (1)
app/frontend/javascript/controllers/index.js:117
- Remove the trailing blank line at EOF to match the repo’s whitespace rules (no trailing blank lines).
import LightboxController from "./lightbox_controller"
application.register("lightbox", LightboxController)
| <div class="<%= resource.class.table_name %>-gallery text-<%= align %> mb-4" | ||
| data-controller="lightbox" | ||
| data-action="keydown.esc@window->lightbox#closeOnEscape keydown.left@window->lightbox#prev keydown.right@window->lightbox#next"> |
|
|
||
| # Modal starts hidden | ||
| modal = find("[data-lightbox-target='modal']", visible: :all) | ||
| expect(modal[:class]).to include("hidden") |
| all("[data-lightbox-target='item']").first.click | ||
|
|
||
| expect(page).to have_current_path(story_path(story)) | ||
| expect(modal[:class]).not_to include("hidden") |
|
|
||
| # Closing dismisses the modal | ||
| find("[data-action='lightbox#close']").click | ||
| expect(modal[:class]).to include("hidden") |
Closes #1520
What is the goal of this PR and why is this important?
How did you approach the change?
lightboxStimulus controller (app/frontend/javascript/controllers/lightbox_controller.js) that opens a modal, tracks the current index, and supports next/previous, arrow keys, Escape, and backdrop-click to close.app/views/assets/_display_gallery_media.html.erbso each image gallery item is a link marked as a lightboxitemtarget; the existing full-size link is kept as thehrefso it still works as a no-JS fallback.app/views/assets/_lightbox.html.erbfor the modal markup (image, counter, prev/next/close controls).link: true(stories, story shares, workshop logs, events, etc.); PDFs and other non-image assets keep their existing rendering.AGENTS.md.Anything else to add?
Test plan
spec/system/story_gallery_lightbox_spec.rb(Selenium): clicking an additional image opens it inline without navigating away, next/previous scroll through the set, and closing dismisses the modal.