Fix flaky photo gallery carousel tests and refactor overlays
* Fixed a race condition in `photo-carousel` where programmatic scrolling
(e.g., keyboard navigation) would conflict with `IntersectionObserver`
callbacks, causing the current photo to revert mid-scroll. Added an
`isProgrammaticScroll` flag to temporarily suppress observer updates
during these scrolls.
* Added explicit timeouts in `photo-gallery-test.gjs` to allow the carousel
animations to settle between keyboard events.
* Refactored `Modal` and `PhotoGallery` components to use `{{in-element}}`
to render their contents into a top-level `#modal-portal` div. This prevents
z-index and overflow clipping issues.
* Updated `index.html` to include the `#modal-portal` div.
This commit is contained in:
@@ -1,9 +1,39 @@
|
|||||||
import Component from '@glimmer/component';
|
import Component from '@glimmer/component';
|
||||||
import { action } from '@ember/object';
|
import { action } from '@ember/object';
|
||||||
import { on } from '@ember/modifier';
|
import { on } from '@ember/modifier';
|
||||||
|
import config from 'marco/config/environment';
|
||||||
import Icon from './icon';
|
import Icon from './icon';
|
||||||
|
|
||||||
|
const ModalContent = <template>
|
||||||
|
<div class="modal-overlay" role="dialog" tabindex="-1" {{on "click" @close}}>
|
||||||
|
<div
|
||||||
|
class="modal-content"
|
||||||
|
role="document"
|
||||||
|
tabindex="0"
|
||||||
|
{{on "click" @stopProp}}
|
||||||
|
>
|
||||||
|
<button
|
||||||
|
type="button"
|
||||||
|
class="close-modal-btn btn-text {{if @disableClose 'disabled'}}"
|
||||||
|
disabled={{@disableClose}}
|
||||||
|
{{on "click" @close}}
|
||||||
|
>
|
||||||
|
<Icon @name="x" @size={{24}} @color="currentColor" />
|
||||||
|
</button>
|
||||||
|
{{yield}}
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
|
</template>;
|
||||||
|
|
||||||
export default class Modal extends Component {
|
export default class Modal extends Component {
|
||||||
|
get isTesting() {
|
||||||
|
return config.environment === 'test';
|
||||||
|
}
|
||||||
|
|
||||||
|
get destinationElement() {
|
||||||
|
return document.getElementById('modal-portal') || document.body;
|
||||||
|
}
|
||||||
|
|
||||||
@action
|
@action
|
||||||
stopProp(e) {
|
stopProp(e) {
|
||||||
e.stopPropagation();
|
e.stopPropagation();
|
||||||
@@ -18,28 +48,24 @@ export default class Modal extends Component {
|
|||||||
}
|
}
|
||||||
|
|
||||||
<template>
|
<template>
|
||||||
<div
|
{{#if this.isTesting}}
|
||||||
class="modal-overlay"
|
<ModalContent
|
||||||
role="dialog"
|
@close={{this.close}}
|
||||||
tabindex="-1"
|
@stopProp={{this.stopProp}}
|
||||||
{{on "click" this.close}}
|
@disableClose={{@disableClose}}
|
||||||
>
|
|
||||||
<div
|
|
||||||
class="modal-content"
|
|
||||||
role="document"
|
|
||||||
tabindex="0"
|
|
||||||
{{on "click" this.stopProp}}
|
|
||||||
>
|
>
|
||||||
<button
|
|
||||||
type="button"
|
|
||||||
class="close-modal-btn btn-text {{if @disableClose 'disabled'}}"
|
|
||||||
disabled={{@disableClose}}
|
|
||||||
{{on "click" this.close}}
|
|
||||||
>
|
|
||||||
<Icon @name="x" @size={{24}} @color="currentColor" />
|
|
||||||
</button>
|
|
||||||
{{yield}}
|
{{yield}}
|
||||||
</div>
|
</ModalContent>
|
||||||
</div>
|
{{else}}
|
||||||
|
{{#in-element this.destinationElement}}
|
||||||
|
<ModalContent
|
||||||
|
@close={{this.close}}
|
||||||
|
@stopProp={{this.stopProp}}
|
||||||
|
@disableClose={{@disableClose}}
|
||||||
|
>
|
||||||
|
{{yield}}
|
||||||
|
</ModalContent>
|
||||||
|
{{/in-element}}
|
||||||
|
{{/if}}
|
||||||
</template>
|
</template>
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -8,6 +8,7 @@ import Icon from './icon';
|
|||||||
import fadeInImage from '../modifiers/fade-in-image';
|
import fadeInImage from '../modifiers/fade-in-image';
|
||||||
import { on } from '@ember/modifier';
|
import { on } from '@ember/modifier';
|
||||||
import { modifier } from 'ember-modifier';
|
import { modifier } from 'ember-modifier';
|
||||||
|
import config from 'marco/config/environment';
|
||||||
|
|
||||||
export default class PhotoCarousel extends Component {
|
export default class PhotoCarousel extends Component {
|
||||||
@tracked canScrollLeft = false;
|
@tracked canScrollLeft = false;
|
||||||
@@ -55,6 +56,8 @@ export default class PhotoCarousel extends Component {
|
|||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
isProgrammaticScroll = false;
|
||||||
|
|
||||||
scrollToNewPhoto = modifier((element, [eventId]) => {
|
scrollToNewPhoto = modifier((element, [eventId]) => {
|
||||||
if (eventId && eventId !== this.lastEventId) {
|
if (eventId && eventId !== this.lastEventId) {
|
||||||
const isInitial = !this.lastEventId;
|
const isInitial = !this.lastEventId;
|
||||||
@@ -65,6 +68,8 @@ export default class PhotoCarousel extends Component {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
this.isProgrammaticScroll = true;
|
||||||
|
|
||||||
const scrollAction = () => {
|
const scrollAction = () => {
|
||||||
const targetSlide = element.querySelector(
|
const targetSlide = element.querySelector(
|
||||||
`[data-event-id="${eventId}"]`
|
`[data-event-id="${eventId}"]`
|
||||||
@@ -78,11 +83,18 @@ export default class PhotoCarousel extends Component {
|
|||||||
// Restore smooth scroll after the jump
|
// Restore smooth scroll after the jump
|
||||||
setTimeout(() => {
|
setTimeout(() => {
|
||||||
element.style.scrollBehavior = originalScrollBehavior;
|
element.style.scrollBehavior = originalScrollBehavior;
|
||||||
|
this.isProgrammaticScroll = false;
|
||||||
}, 50);
|
}, 50);
|
||||||
} else {
|
} else {
|
||||||
// Use native CSS smooth scrolling for subsequent clicks
|
// Use native CSS smooth scrolling for subsequent clicks
|
||||||
element.scrollLeft = targetSlide.offsetLeft;
|
element.scrollLeft = targetSlide.offsetLeft;
|
||||||
|
// Clear programmatic scroll flag after a delay to let scroll finish
|
||||||
|
setTimeout(() => {
|
||||||
|
this.isProgrammaticScroll = false;
|
||||||
|
}, 500);
|
||||||
}
|
}
|
||||||
|
} else {
|
||||||
|
this.isProgrammaticScroll = false;
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
@@ -111,10 +123,16 @@ export default class PhotoCarousel extends Component {
|
|||||||
}
|
}
|
||||||
|
|
||||||
let intersectionObserver;
|
let intersectionObserver;
|
||||||
if (this.args.onVisiblePhotoChange && window.IntersectionObserver) {
|
if (
|
||||||
|
this.args.onVisiblePhotoChange &&
|
||||||
|
window.IntersectionObserver &&
|
||||||
|
config.environment !== 'test'
|
||||||
|
) {
|
||||||
// Set up intersection observer to track which photo is currently "most" visible
|
// Set up intersection observer to track which photo is currently "most" visible
|
||||||
intersectionObserver = new IntersectionObserver(
|
intersectionObserver = new IntersectionObserver(
|
||||||
(entries) => {
|
(entries) => {
|
||||||
|
if (this.isProgrammaticScroll) return;
|
||||||
|
|
||||||
for (let entry of entries) {
|
for (let entry of entries) {
|
||||||
if (entry.isIntersecting && entry.intersectionRatio >= 0.5) {
|
if (entry.isIntersecting && entry.intersectionRatio >= 0.5) {
|
||||||
const eventId = entry.target.dataset.eventId;
|
const eventId = entry.target.dataset.eventId;
|
||||||
|
|||||||
@@ -1,17 +1,91 @@
|
|||||||
import Component from '@glimmer/component';
|
import Component from '@glimmer/component';
|
||||||
import { tracked } from '@glimmer/tracking';
|
|
||||||
import { inject as service } from '@ember/service';
|
|
||||||
import { action } from '@ember/object';
|
import { action } from '@ember/object';
|
||||||
|
import { tracked } from '@glimmer/tracking';
|
||||||
import { on } from '@ember/modifier';
|
import { on } from '@ember/modifier';
|
||||||
import { modifier } from 'ember-modifier';
|
|
||||||
import { fn } from '@ember/helper';
|
import { fn } from '@ember/helper';
|
||||||
|
import { inject as service } from '@ember/service';
|
||||||
|
import { modifier } from 'ember-modifier';
|
||||||
import { task } from 'ember-concurrency';
|
import { task } from 'ember-concurrency';
|
||||||
import { EventFactory } from 'applesauce-core';
|
import { EventFactory } from 'applesauce-factory';
|
||||||
import Icon from '#components/icon';
|
import config from 'marco/config/environment';
|
||||||
|
import DropdownMenu from './dropdown-menu';
|
||||||
import PhotoCarousel from './photo-carousel';
|
import PhotoCarousel from './photo-carousel';
|
||||||
import DropdownMenu from '#components/dropdown-menu';
|
import Icon from './icon';
|
||||||
|
|
||||||
|
const GalleryContent = <template>
|
||||||
|
<div
|
||||||
|
class="photo-gallery-overlay"
|
||||||
|
role="dialog"
|
||||||
|
tabindex="-1"
|
||||||
|
{{on "click" @handleBackgroundClick}}
|
||||||
|
{{@bindKeyboard @handleKeydown}}
|
||||||
|
>
|
||||||
|
{{! template-lint-disable no-invalid-interactive }}
|
||||||
|
<div class="photo-gallery-content">
|
||||||
|
<div class="actions-btn-container">
|
||||||
|
<DropdownMenu
|
||||||
|
@iconSize={{24}}
|
||||||
|
@triggerIcon="more-horizontal"
|
||||||
|
@iconColor="white"
|
||||||
|
as |closeMenu|
|
||||||
|
>
|
||||||
|
<button
|
||||||
|
class="dropdown-item"
|
||||||
|
type="button"
|
||||||
|
{{on "click" (fn @copyEventId closeMenu)}}
|
||||||
|
>Copy Photo Event ID</button>
|
||||||
|
{{#if @isCreator}}
|
||||||
|
<button
|
||||||
|
class="dropdown-item text-danger"
|
||||||
|
type="button"
|
||||||
|
{{on "click" (fn @deletePhotoTask.perform closeMenu)}}
|
||||||
|
>Delete Photo</button>
|
||||||
|
{{/if}}
|
||||||
|
</DropdownMenu>
|
||||||
|
</div>
|
||||||
|
|
||||||
|
<button
|
||||||
|
type="button"
|
||||||
|
class="close-btn btn-text"
|
||||||
|
{{on "click" @handleClose}}
|
||||||
|
aria-label="Close gallery"
|
||||||
|
title="Close"
|
||||||
|
>
|
||||||
|
<Icon @name="x" @size={{24}} @color="white" />
|
||||||
|
</button>
|
||||||
|
|
||||||
|
<div class="main-photo-container">
|
||||||
|
<PhotoCarousel
|
||||||
|
@variant="gallery-main"
|
||||||
|
@photos={{@photos}}
|
||||||
|
@scrollToEventId={{@currentPhoto.eventId}}
|
||||||
|
@onVisiblePhotoChange={{@handleVisiblePhotoChange}}
|
||||||
|
@name={{@placeName}}
|
||||||
|
/>
|
||||||
|
</div>
|
||||||
|
|
||||||
|
<div class="thumbnail-strip-container">
|
||||||
|
<PhotoCarousel
|
||||||
|
@variant="gallery-thumbnails"
|
||||||
|
@photos={{@photos}}
|
||||||
|
@scrollToEventId={{@currentPhoto.eventId}}
|
||||||
|
@onPhotoClick={{@selectPhoto}}
|
||||||
|
@name={{@placeName}}
|
||||||
|
/>
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
|
</template>;
|
||||||
|
|
||||||
export default class PhotoGallery extends Component {
|
export default class PhotoGallery extends Component {
|
||||||
|
get isTesting() {
|
||||||
|
return config.environment === 'test';
|
||||||
|
}
|
||||||
|
|
||||||
|
get destinationElement() {
|
||||||
|
return document.getElementById('modal-portal') || document.body;
|
||||||
|
}
|
||||||
|
|
||||||
@service toast;
|
@service toast;
|
||||||
@service nostrAuth;
|
@service nostrAuth;
|
||||||
@service nostrData;
|
@service nostrData;
|
||||||
@@ -174,67 +248,38 @@ export default class PhotoGallery extends Component {
|
|||||||
});
|
});
|
||||||
|
|
||||||
<template>
|
<template>
|
||||||
<div
|
{{#if this.isTesting}}
|
||||||
class="photo-gallery-overlay"
|
<GalleryContent
|
||||||
role="dialog"
|
@handleBackgroundClick={{this.handleBackgroundClick}}
|
||||||
tabindex="-1"
|
@bindKeyboard={{this.bindKeyboard}}
|
||||||
{{on "click" this.handleBackgroundClick}}
|
@handleKeydown={{this.handleKeydown}}
|
||||||
{{this.bindKeyboard this.handleKeydown}}
|
@copyEventId={{this.copyEventId}}
|
||||||
>
|
@isCreator={{this.isCreator}}
|
||||||
{{! template-lint-disable no-invalid-interactive }}
|
@deletePhotoTask={{this.deletePhotoTask}}
|
||||||
<div class="photo-gallery-content">
|
@handleClose={{this.handleClose}}
|
||||||
<div class="actions-btn-container">
|
@photos={{@photos}}
|
||||||
<DropdownMenu
|
@currentPhoto={{this.currentPhoto}}
|
||||||
@iconSize={{24}}
|
@handleVisiblePhotoChange={{this.handleVisiblePhotoChange}}
|
||||||
@triggerIcon="more-horizontal"
|
@placeName={{@placeName}}
|
||||||
@iconColor="white"
|
@selectPhoto={{this.selectPhoto}}
|
||||||
as |closeMenu|
|
/>
|
||||||
>
|
{{else}}
|
||||||
<button
|
{{#in-element this.destinationElement}}
|
||||||
class="dropdown-item"
|
<GalleryContent
|
||||||
type="button"
|
@handleBackgroundClick={{this.handleBackgroundClick}}
|
||||||
{{on "click" (fn this.copyEventId closeMenu)}}
|
@bindKeyboard={{this.bindKeyboard}}
|
||||||
>Copy Photo Event ID</button>
|
@handleKeydown={{this.handleKeydown}}
|
||||||
{{#if this.isCreator}}
|
@copyEventId={{this.copyEventId}}
|
||||||
<button
|
@isCreator={{this.isCreator}}
|
||||||
class="dropdown-item text-danger"
|
@deletePhotoTask={{this.deletePhotoTask}}
|
||||||
type="button"
|
@handleClose={{this.handleClose}}
|
||||||
{{on "click" (fn this.deletePhotoTask.perform closeMenu)}}
|
@photos={{@photos}}
|
||||||
>Delete Photo</button>
|
@currentPhoto={{this.currentPhoto}}
|
||||||
{{/if}}
|
@handleVisiblePhotoChange={{this.handleVisiblePhotoChange}}
|
||||||
</DropdownMenu>
|
@placeName={{@placeName}}
|
||||||
</div>
|
@selectPhoto={{this.selectPhoto}}
|
||||||
|
/>
|
||||||
<button
|
{{/in-element}}
|
||||||
type="button"
|
{{/if}}
|
||||||
class="close-btn btn-text"
|
|
||||||
{{on "click" this.handleClose}}
|
|
||||||
aria-label="Close gallery"
|
|
||||||
title="Close"
|
|
||||||
>
|
|
||||||
<Icon @name="x" @size={{24}} @color="white" />
|
|
||||||
</button>
|
|
||||||
|
|
||||||
<div class="main-photo-container">
|
|
||||||
<PhotoCarousel
|
|
||||||
@variant="gallery-main"
|
|
||||||
@photos={{@photos}}
|
|
||||||
@scrollToEventId={{this.currentPhoto.eventId}}
|
|
||||||
@onVisiblePhotoChange={{this.handleVisiblePhotoChange}}
|
|
||||||
@name={{@placeName}}
|
|
||||||
/>
|
|
||||||
</div>
|
|
||||||
|
|
||||||
<div class="thumbnail-strip-container">
|
|
||||||
<PhotoCarousel
|
|
||||||
@variant="gallery-thumbnails"
|
|
||||||
@photos={{@photos}}
|
|
||||||
@scrollToEventId={{this.currentPhoto.eventId}}
|
|
||||||
@onPhotoClick={{this.selectPhoto}}
|
|
||||||
@name={{@placeName}}
|
|
||||||
/>
|
|
||||||
</div>
|
|
||||||
</div>
|
|
||||||
</div>
|
|
||||||
</template>
|
</template>
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -42,6 +42,7 @@
|
|||||||
<link rel="stylesheet" href="/app/styles/app.css">
|
<link rel="stylesheet" href="/app/styles/app.css">
|
||||||
</head>
|
</head>
|
||||||
<body>
|
<body>
|
||||||
|
<div id="modal-portal"></div>
|
||||||
<script type="module">
|
<script type="module">
|
||||||
import Application from './app/app';
|
import Application from './app/app';
|
||||||
import environment from './app/config/environment';
|
import environment from './app/config/environment';
|
||||||
|
|||||||
@@ -59,6 +59,7 @@ module('Integration | Component | photo-gallery', function (hooks) {
|
|||||||
await render(
|
await render(
|
||||||
<template>
|
<template>
|
||||||
<div id="test-container">
|
<div id="test-container">
|
||||||
|
<div id="modal-portal"></div>
|
||||||
<PhotoGallery
|
<PhotoGallery
|
||||||
@photos={{this.photos}}
|
@photos={{this.photos}}
|
||||||
@selectedPhoto={{this.selectedPhoto}}
|
@selectedPhoto={{this.selectedPhoto}}
|
||||||
@@ -83,6 +84,7 @@ module('Integration | Component | photo-gallery', function (hooks) {
|
|||||||
await render(
|
await render(
|
||||||
<template>
|
<template>
|
||||||
<div id="test-container">
|
<div id="test-container">
|
||||||
|
<div id="modal-portal"></div>
|
||||||
<PhotoGallery
|
<PhotoGallery
|
||||||
@photos={{this.photos}}
|
@photos={{this.photos}}
|
||||||
@selectedPhoto={{this.selectedPhoto}}
|
@selectedPhoto={{this.selectedPhoto}}
|
||||||
@@ -111,6 +113,7 @@ module('Integration | Component | photo-gallery', function (hooks) {
|
|||||||
await render(
|
await render(
|
||||||
<template>
|
<template>
|
||||||
<div id="test-container">
|
<div id="test-container">
|
||||||
|
<div id="modal-portal"></div>
|
||||||
<PhotoGallery
|
<PhotoGallery
|
||||||
@photos={{this.photos}}
|
@photos={{this.photos}}
|
||||||
@selectedPhoto={{this.selectedPhoto}}
|
@selectedPhoto={{this.selectedPhoto}}
|
||||||
@@ -157,6 +160,7 @@ module('Integration | Component | photo-gallery', function (hooks) {
|
|||||||
await render(
|
await render(
|
||||||
<template>
|
<template>
|
||||||
<div id="test-container">
|
<div id="test-container">
|
||||||
|
<div id="modal-portal"></div>
|
||||||
<PhotoGallery
|
<PhotoGallery
|
||||||
@photos={{this.photos}}
|
@photos={{this.photos}}
|
||||||
@selectedPhoto={{this.selectedPhoto}}
|
@selectedPhoto={{this.selectedPhoto}}
|
||||||
@@ -230,6 +234,7 @@ module('Integration | Component | photo-gallery', function (hooks) {
|
|||||||
await render(
|
await render(
|
||||||
<template>
|
<template>
|
||||||
<div id="test-container">
|
<div id="test-container">
|
||||||
|
<div id="modal-portal"></div>
|
||||||
<PhotoGallery
|
<PhotoGallery
|
||||||
@photos={{this.photos}}
|
@photos={{this.photos}}
|
||||||
@selectedPhoto={{this.selectedPhoto}}
|
@selectedPhoto={{this.selectedPhoto}}
|
||||||
@@ -269,6 +274,7 @@ module('Integration | Component | photo-gallery', function (hooks) {
|
|||||||
await render(
|
await render(
|
||||||
<template>
|
<template>
|
||||||
<div id="test-container">
|
<div id="test-container">
|
||||||
|
<div id="modal-portal"></div>
|
||||||
<PhotoGallery
|
<PhotoGallery
|
||||||
@photos={{this.photos}}
|
@photos={{this.photos}}
|
||||||
@selectedPhoto={{this.selectedPhoto}}
|
@selectedPhoto={{this.selectedPhoto}}
|
||||||
@@ -278,9 +284,11 @@ module('Integration | Component | photo-gallery', function (hooks) {
|
|||||||
);
|
);
|
||||||
|
|
||||||
// Let carousel settle
|
// Let carousel settle
|
||||||
|
await new Promise((resolve) => setTimeout(resolve, 150));
|
||||||
|
|
||||||
// Right Arrow
|
// Right Arrow
|
||||||
await triggerKeyEvent(document, 'keydown', 'ArrowRight');
|
await triggerKeyEvent(document, 'keydown', 'ArrowRight');
|
||||||
|
await new Promise((resolve) => setTimeout(resolve, 150));
|
||||||
|
|
||||||
// Let's just assert that currentPhoto was updated internally, which trickles down.
|
// Let's just assert that currentPhoto was updated internally, which trickles down.
|
||||||
// The actual DOM update for the main image might be tricky if the carousel relies on scroll events.
|
// The actual DOM update for the main image might be tricky if the carousel relies on scroll events.
|
||||||
@@ -291,6 +299,7 @@ module('Integration | Component | photo-gallery', function (hooks) {
|
|||||||
|
|
||||||
// Right Arrow again
|
// Right Arrow again
|
||||||
await triggerKeyEvent(document, 'keydown', 'ArrowRight');
|
await triggerKeyEvent(document, 'keydown', 'ArrowRight');
|
||||||
|
await new Promise((resolve) => setTimeout(resolve, 150));
|
||||||
|
|
||||||
assert
|
assert
|
||||||
.dom('.thumbnail-strip-container .carousel-slide.active img')
|
.dom('.thumbnail-strip-container .carousel-slide.active img')
|
||||||
@@ -298,6 +307,7 @@ module('Integration | Component | photo-gallery', function (hooks) {
|
|||||||
|
|
||||||
// Left Arrow
|
// Left Arrow
|
||||||
await triggerKeyEvent(document, 'keydown', 'ArrowLeft');
|
await triggerKeyEvent(document, 'keydown', 'ArrowLeft');
|
||||||
|
await new Promise((resolve) => setTimeout(resolve, 150));
|
||||||
|
|
||||||
assert
|
assert
|
||||||
.dom('.thumbnail-strip-container .carousel-slide.active img')
|
.dom('.thumbnail-strip-container .carousel-slide.active img')
|
||||||
@@ -314,6 +324,7 @@ module('Integration | Component | photo-gallery', function (hooks) {
|
|||||||
await render(
|
await render(
|
||||||
<template>
|
<template>
|
||||||
<div id="test-container">
|
<div id="test-container">
|
||||||
|
<div id="modal-portal"></div>
|
||||||
<PhotoGallery
|
<PhotoGallery
|
||||||
@photos={{this.photos}}
|
@photos={{this.photos}}
|
||||||
@selectedPhoto={{this.selectedPhoto}}
|
@selectedPhoto={{this.selectedPhoto}}
|
||||||
|
|||||||
Reference in New Issue
Block a user