Skip to content

Commit

Permalink
fix: Ensure media info is dispatched after view change (#1587)
Browse files Browse the repository at this point in the history
  • Loading branch information
dermotduffy committed Sep 26, 2024
1 parent 4e3b16f commit f560113
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 24 deletions.
23 changes: 12 additions & 11 deletions src/utils/embla/carousel-controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,18 @@ export class CarouselController {

public selectSlide(index: number): void {
this._carousel.scrollTo(index, this._transitionEffect === 'none');

// This event exists to allow the caller to know the difference between
// programatically force slide selections and user-driven slide selections
// (e.g. carousel drags). See the note in auto-media-loaded-info.ts on how
// this is used.
const newSlide = this.getSlide(index);
if (newSlide) {
dispatchFrigateCardEvent<CarouselSelected>(this._parent, 'carousel:force-select', {
index: index,
element: newSlide,
});
}
}

protected _refreshCarouselContents = (): void => {
Expand Down Expand Up @@ -162,17 +174,6 @@ export class CarouselController {
};

carousel.on('select', () => selectSlide());
carousel.on('settle', () => {
const carouselSelected = getCarouselSelectedObject();
if (carouselSelected) {
dispatchFrigateCardEvent<CarouselSelected>(
this._parent,
'carousel:settle',
carouselSelected,
);
}
});

return carousel;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,30 @@ declare module 'embla-carousel/components/Plugins' {
}
}

/**
* On the relationship between carousel:select and carousel:force-select:
*
* There is a complex interplay here. `carousel:force-select` is an event
* dispatched by the carousel when it is forced to select a particular slide
* (i.e. the view has changed). `carousel:select` is dispatched for any
* selection -- forced or human (e.g. the user dragging the carousel).
*
* The media info should only be dispatched _after_ the view object has been
* updated (since the view will clear the loaded media info). The setting of the
* view (trigged by `carousel:select`) may require async fetches and may take a
* while -- and so if the card dispatched media on `carousel:selecte` then the
* media info may be dispatched before the view is set (which could result in
* the dispatched media immediately being cleared by the view).
*
* It is fine to have media info dispatched from the `carousel:init` event,
* since the carousel will be initialized based on a particular view object. In
* practice, the carousel will be initialized before the media is loaded, so
* there may not be anything to dispatch at that point.
*
* When media is loaded, that media loaded info will always be allowed to
* propogate upwards as long as it is selected.
*/

type AutoMediaLoadedInfoType = CreatePluginType<LoosePluginType, LooseOptionsType>;

function AutoMediaLoadedInfo(): AutoMediaLoadedInfoType {
Expand All @@ -30,7 +54,9 @@ function AutoMediaLoadedInfo(): AutoMediaLoadedInfoType {
}

emblaApi.on('init', slideSelectHandler);
emblaApi.on('select', slideSelectHandler);
emblaApi
.containerNode()
.addEventListener('frigate-card:carousel:force-select', slideSelectHandler);
}

function destroy(): void {
Expand All @@ -40,7 +66,9 @@ function AutoMediaLoadedInfo(): AutoMediaLoadedInfoType {
}

emblaApi.off('init', slideSelectHandler);
emblaApi.off('select', slideSelectHandler);
emblaApi
.containerNode()
.removeEventListener('frigate-card:carousel:force-select', slideSelectHandler);
}

function mediaLoadedInfoHandler(ev: CustomEvent<MediaLoadedInfo>): void {
Expand Down
25 changes: 18 additions & 7 deletions tests/utils/embla/carousel-controller.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,24 +110,35 @@ describe('CarouselController', () => {
it('should select given slide', () => {
const children = createTestSlideNodes();
const parent = createParent({ children: children });

const forceSelectListener = vi.fn();
parent.addEventListener('frigate-card:carousel:force-select', forceSelectListener);

const carousel = new CarouselController(createRoot(), parent);

carousel.selectSlide(4);

expect(getEmblaApi()?.scrollTo).toBeCalledWith(4, false);
expect(forceSelectListener).toBeCalledWith(
expect.objectContaining({
detail: { index: 4, element: children[4] },
}),
);
});

it('should dispatch settle event', () => {
const children = createTestSlideNodes();
it('should not select non-existent slide', () => {
const children = createTestSlideNodes({ n: 10 });
const parent = createParent({ children: children });
new CarouselController(createRoot(), parent);

const settleHandler = vi.fn();
parent.addEventListener('frigate-card:carousel:settle', settleHandler);
const forceSelectListener = vi.fn();
parent.addEventListener('frigate-card:carousel:force-select', forceSelectListener);

callEmblaHandler(getEmblaApi(), 'settle');
const carousel = new CarouselController(createRoot(), parent);

carousel.selectSlide(11);

expect(settleHandler).toBeCalled();
expect(getEmblaApi()?.scrollTo).toBeCalledWith(11, false);
expect(forceSelectListener).not.toBeCalled();
});

it('should dispatch select event', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {
} from '../../../../../src/utils/media-info';
import { createMediaLoadedInfo, createParent } from '../../../../test-utils';
import {
callEmblaHandler,
createEmblaApiInstance,
createTestEmblaOptionHandler,
createTestSlideNodes,
Expand All @@ -30,7 +29,6 @@ describe('AutoMediaLoadedInfo', () => {
plugin.destroy();

expect(emblaApi.off).toBeCalledWith('init', expect.anything());
expect(emblaApi.off).toBeCalledWith('select', expect.anything());
});

describe('should correctly propogate media load/unload depending on whether media is currently selected', () => {
Expand Down Expand Up @@ -82,11 +80,15 @@ describe('AutoMediaLoadedInfo', () => {
dispatchExistingMediaLoadedInfoAsEvent(children[5], createMediaLoadedInfo());

vi.mocked(emblaApi.selectedScrollSnap).mockReturnValue(4);
callEmblaHandler(emblaApi, 'select');
emblaApi
.containerNode()
.dispatchEvent(new Event('frigate-card:carousel:force-select'));
expect(mediaLoadedHandler).not.toBeCalled();

vi.mocked(emblaApi.selectedScrollSnap).mockReturnValue(5);
callEmblaHandler(emblaApi, 'select');
emblaApi
.containerNode()
.dispatchEvent(new Event('frigate-card:carousel:force-select'));
expect(mediaLoadedHandler).toBeCalled();
});
});

0 comments on commit f560113

Please sign in to comment.