From b4595867b3c55560e736384f4e8228cdd1d43d17 Mon Sep 17 00:00:00 2001
From: Dermot Duffy <dermot.duffy@gmail.com>
Date: Sun, 13 Aug 2023 10:00:29 -0700
Subject: [PATCH] Avoid carousel init duplication.

---
 src/components/carousel.ts   | 26 +++++++++++++++++++-------
 src/components/media-grid.ts |  4 ----
 src/utils/basic.ts           |  8 +++++++-
 tests/utils/basic.test.ts    | 20 ++++++++++++++++----
 4 files changed, 42 insertions(+), 16 deletions(-)

diff --git a/src/components/carousel.ts b/src/components/carousel.ts
index c7de9885..59a43c87 100644
--- a/src/components/carousel.ts
+++ b/src/components/carousel.ts
@@ -11,10 +11,11 @@ import {
 } from 'lit';
 import { customElement, property } from 'lit/decorators.js';
 import { Ref, createRef, ref } from 'lit/directives/ref.js';
+import isEqual from 'lodash-es/isEqual';
 import throttle from 'lodash-es/throttle';
 import carouselStyle from '../scss/carousel.scss';
 import { TransitionEffect } from '../types';
-import { dispatchFrigateCardEvent } from '../utils/basic.js';
+import { dispatchFrigateCardEvent, isHTMLElement } from '../utils/basic.js';
 
 export interface CarouselSelect {
   index: number;
@@ -175,6 +176,13 @@ export class FrigateCardCarousel extends LitElement {
     this._carousel = undefined;
   }
 
+  protected _getSlideElements(): HTMLElement[] {
+    return (
+      this._refSlot.value?.assignedElements({ flatten: true }).filter(isHTMLElement) ??
+      []
+    );
+  }
+
   /**
    * Initialize the carousel.
    */
@@ -187,7 +195,7 @@ export class FrigateCardCarousel extends LitElement {
       root: carouselNode,
       // As the slides are slotted, need to explicitly pull them out and pass
       // them to Embla.
-      slides: this._refSlot.value?.assignedElements({ flatten: true }) as HTMLElement[],
+      slides: this._getSlideElements(),
     };
 
     if (carouselNode && nodes.slides) {
@@ -219,7 +227,7 @@ export class FrigateCardCarousel extends LitElement {
         // the selected slide as returned by the carousel. This need should be
         // verified in future versions of Embla (tested as necessary on v7.0.9).
         // Test case:
-        // 
+        //
         // - Start in `live` view in grid mode.
         // - Select any camera that is not the first one.
         // - Go to non-grid mode.
@@ -256,10 +264,14 @@ export class FrigateCardCarousel extends LitElement {
    * Called when the slotted children in the carousel change.
    */
   protected _slotChanged(): void {
-    // Cannot just re-init, because the slide elements themselves may have
-    // changed, and only a carousel init can pass in new (slotted) children. If
-    this._destroyCarousel();
-    this.requestUpdate();
+    // Check whether the slotted elements have changed (without this check the
+    // carousel initializations are duplicated).
+    if (!isEqual(this._getSlideElements(), this._carousel?.slideNodes())) {
+      // Cannot just re-init, because the slide elements themselves may have
+      // changed, and only a carousel init can pass in new (slotted) children.
+      this._destroyCarousel();
+      this.requestUpdate();
+    }
   }
 
   protected render(): TemplateResult | void {
diff --git a/src/components/media-grid.ts b/src/components/media-grid.ts
index 2c2f5d72..b894f32b 100644
--- a/src/components/media-grid.ts
+++ b/src/components/media-grid.ts
@@ -1,8 +1,4 @@
 // TODO: Performance of video scanning (pause/play?)
-// TODO: Investigate query spam during a grid load
-// TODO: Do I need column max?
-// TODO: frigate-card-live-go2rtc.hidden double media load event.
-// TODO: Why am I creating so many carousels in the race-condition issue? Expecting 5, getting 10.
 
 import {
   CSSResultGroup,
diff --git a/src/utils/basic.ts b/src/utils/basic.ts
index 2321fe5e..51efa4cc 100644
--- a/src/utils/basic.ts
+++ b/src/utils/basic.ts
@@ -225,4 +225,10 @@ export const setOrRemoveAttribute = (
 /**
  * Allow typescript to narrow types based on truthy filter.
  */
-export const filterTruthy = <T>(x: T | false | undefined | null | '' | 0): x is T => !!x;
+export const isTruthy = <T>(x: T | false | undefined | null | '' | 0): x is T => !!x;
+
+/**
+ * Allow typescript to narrow types for HTMLElements.
+ */
+export const isHTMLElement = (element: unknown): element is HTMLElement =>
+  element instanceof HTMLElement;
diff --git a/tests/utils/basic.test.ts b/tests/utils/basic.test.ts
index 6d45f08c..574a9803 100644
--- a/tests/utils/basic.test.ts
+++ b/tests/utils/basic.test.ts
@@ -8,7 +8,7 @@ import {
   dayToDate,
   dispatchFrigateCardEvent,
   errorToConsole,
-  filterTruthy,
+  isTruthy,
   formatDate,
   formatDateAndTime,
   getDurationString,
@@ -20,6 +20,7 @@ import {
   setify,
   setOrRemoveAttribute,
   sleep,
+  isHTMLElement,
 } from '../../src/utils/basic';
 
 // @vitest-environment jsdom
@@ -248,11 +249,22 @@ describe('setOrRemoveAttribute', () => {
   });
 });
 
-describe('filterTruthy', () => {
+describe('isTruthy', () => {
   it('should return true for true', () => {
-    expect(filterTruthy(true)).toBeTruthy();
+    expect(isTruthy(true)).toBeTruthy();
   });
   it('should return false for false', () => {
-    expect(filterTruthy(false)).toBeFalsy();
+    expect(isTruthy(false)).toBeFalsy();
+  });
+});
+
+describe('isHTMLElement', () => {
+  it('should return true for HTMLElement', () => {
+    const htmlElement = document.createElement('div');
+    expect(isHTMLElement(htmlElement)).toBeTruthy();
+  });
+  it('should return false for Element', () => {
+    const svgElement = document.createElementNS('http://www.w3.org/2000/svg', 'circle');
+    expect(isHTMLElement(svgElement)).toBeFalsy();
   });
 });