FIX: capture click target in lightbox click handler (#22732)

In Safari, clicking any image in a lightbox gallery results in the first image loading (instead of the clicked image).

Previously we relied on document.activeElement to determine which lightbox image was clicked. However in Chrome the active element is the lightbox selector (a.lightbox), whereas in Safari the active element defaults to the body tag.

Currently the startingIndex that is calculated within processHTML() is used by lightbox to determine which image to load first. The starting index is currently achieved by checking each lightbox element within the gallery against the active element.

To fix this issue we can use the event.target to get the clicked image, then use the closest selector and pass that into the function to do the matching and return the correct startingIndex.
This commit is contained in:
David Battersby 2023-07-21 12:28:37 +08:00 committed by GitHub
parent a0ad7d0a04
commit 351e8e2359
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 13 additions and 8 deletions

View File

@ -2,14 +2,12 @@ import { SELECTORS } from "./constants";
import { escapeExpression } from "discourse/lib/utilities";
import { htmlSafe } from "@ember/template";
export async function processHTML({ container, selector }) {
export async function processHTML({ container, selector, clickTarget }) {
selector ??= SELECTORS.DEFAULT_ITEM_SELECTOR;
const items = [...container.querySelectorAll(selector)];
let _startingIndex = items.findIndex(
(item) => item === document.activeElement
);
let _startingIndex = items.findIndex((item) => item === clickTarget);
if (_startingIndex === -1) {
_startingIndex = 0;

View File

@ -107,8 +107,12 @@ export default class LightboxService extends Service {
}
@bind
async openLightbox({ container, selector }) {
const { items, startingIndex } = await processHTML({ container, selector });
async openLightbox({ container, selector, clickTarget }) {
const { items, startingIndex } = await processHTML({
container,
selector,
clickTarget,
});
if (!items.length) {
return;
@ -236,6 +240,7 @@ export default class LightboxService extends Service {
}
handleClickEvent(event, trigger) {
const closestTrigger = event.target.closest(trigger);
const isLightboxClick = event
.composedPath()
.find(
@ -254,6 +259,7 @@ export default class LightboxService extends Service {
this.openLightbox({
container: event.currentTarget,
selector: trigger,
clickTarget: closestTrigger,
});
event.target.toggleAttribute(SELECTORS.DOCUMENT_LAST_FOCUSED_ELEMENT);

View File

@ -60,8 +60,9 @@ module("Unit | Service | Experimental Lightbox", function (hooks) {
const openLightboxSpy = sinon.spy(this.lightbox, "openLightbox");
const removeEventListenerSpy = sinon.spy(container, "removeEventListener");
const clickTarget = container.querySelector(selector);
await this.lightbox.setupLightboxes({ container, selector });
await this.lightbox.setupLightboxes({ container, selector, clickTarget });
await click(container.querySelector(selector));
@ -70,7 +71,7 @@ module("Unit | Service | Experimental Lightbox", function (hooks) {
await click(container.querySelector("p"));
assert.strictEqual(
openLightboxSpy.calledWith({ container, selector }),
openLightboxSpy.calledWith({ container, selector, clickTarget }),
true,
"calls openLightbox on lightboxed element click"
);