From cebd4b215bb27b52766273a9c78883dc78cd4073 Mon Sep 17 00:00:00 2001 From: Dermot Duffy Date: Sat, 14 Dec 2024 14:58:22 -0800 Subject: [PATCH] fix: Render media correctly in `rtl` languages (#1757) --- src/components/carousel.ts | 6 +- src/components/live/carousel.ts | 117 +++++++++++------- src/components/next-prev-control.ts | 8 +- src/components/viewer/carousel.ts | 83 ++++++------- src/scss/next-previous-control.scss | 4 +- src/utils/embla/carousel-controller.ts | 6 + src/utils/text-direction.ts | 5 + tests/utils/embla/carousel-controller.test.ts | 2 + tests/utils/text-direction.test.ts | 26 ++++ vite.config.ts | 1 + 10 files changed, 162 insertions(+), 96 deletions(-) create mode 100644 src/utils/text-direction.ts create mode 100644 tests/utils/text-direction.test.ts diff --git a/src/components/carousel.ts b/src/components/carousel.ts index 6b223fad..e1dabd85 100644 --- a/src/components/carousel.ts +++ b/src/components/carousel.ts @@ -15,6 +15,7 @@ import { CarouselController, CarouselDirection, } from '../utils/embla/carousel-controller'; +import { getTextDirection } from '../utils/text-direction'; export type EmblaCarouselPlugins = CreatePluginType< LoosePluginType, @@ -85,13 +86,13 @@ export class FrigateCardCarousel extends LitElement { protected render(): TemplateResult | void { return html`
- +
- +
`; } @@ -108,6 +109,7 @@ export class FrigateCardCarousel extends LitElement { transitionEffect: this.transitionEffect, loop: this.loop, plugins: this.plugins, + textDirection: getTextDirection(this), }, ); } else if (changedProps.has('selected')) { diff --git a/src/components/live/carousel.ts b/src/components/live/carousel.ts index bd749399..2e1262d2 100644 --- a/src/components/live/carousel.ts +++ b/src/components/live/carousel.ts @@ -10,6 +10,7 @@ import { customElement, property, state } from 'lit/decorators.js'; import { guard } from 'lit/directives/guard.js'; import { createRef, Ref, ref } from 'lit/directives/ref.js'; import { CameraManager } from '../../camera-manager/manager.js'; +import { CameraManagerCameraMetadata } from '../../camera-manager/types.js'; import { ConditionsManagerEpoch, getOverriddenConfig, @@ -37,6 +38,7 @@ import { AutoLazyLoad } from '../../utils/embla/plugins/auto-lazy-load/auto-lazy import AutoMediaLoadedInfo from '../../utils/embla/plugins/auto-media-loaded-info/auto-media-loaded-info.js'; import AutoSize from '../../utils/embla/plugins/auto-size/auto-size.js'; import { getStreamCameraID } from '../../utils/substream.js'; +import { getTextDirection } from '../../utils/text-direction.js'; import { View } from '../../view/view.js'; import '../carousel'; import { EmblaCarouselPlugins } from '../carousel.js'; @@ -49,6 +51,16 @@ import { FrigateCardLiveProvider } from './provider.js'; const FRIGATE_CARD_LIVE_PROVIDER = 'frigate-card-live-provider'; +interface CameraNeighbor { + id: string; + metadata?: CameraManagerCameraMetadata | null; +} + +interface CameraNeighbors { + previous?: CameraNeighbor; + next?: CameraNeighbor; +} + @customElement('frigate-card-live-carousel') export class FrigateCardLiveCarousel extends LitElement { @property({ attribute: false }) @@ -315,31 +327,74 @@ export class FrigateCardLiveCarousel extends LitElement { `; } - protected _getCameraIDsOfNeighbors(): [string | null, string | null] { + protected _getSubstreamCameraID(cameraID: string, view?: View | null): string { + return view?.context?.live?.overrides?.get(cameraID) ?? cameraID; + } + + protected _getCameraNeighbors(): CameraNeighbors | null { const cameraIDs = this.cameraManager ? [...this.cameraManager?.getStore().getCameraIDsWithCapability('live')] : []; const view = this.viewManagerEpoch?.manager.getView(); if (this.viewFilterCameraID || cameraIDs.length <= 1 || !view || !this.hass) { - return [null, null]; + return {}; } const cameraID = this.viewFilterCameraID ?? view.camera; const currentIndex = cameraIDs.indexOf(cameraID); if (currentIndex < 0) { - return [null, null]; + return {}; } - - return [ - cameraIDs[currentIndex > 0 ? currentIndex - 1 : cameraIDs.length - 1], - cameraIDs[currentIndex + 1 < cameraIDs.length ? currentIndex + 1 : 0], - ]; + const prevID = cameraIDs[currentIndex > 0 ? currentIndex - 1 : cameraIDs.length - 1]; + const nextID = cameraIDs[currentIndex + 1 < cameraIDs.length ? currentIndex + 1 : 0]; + + return { + previous: { + id: prevID, + metadata: prevID + ? this.cameraManager?.getCameraMetadata( + this._getSubstreamCameraID(prevID, view), + ) + : null, + }, + next: { + id: nextID, + metadata: nextID + ? this.cameraManager?.getCameraMetadata( + this._getSubstreamCameraID(nextID, view), + ) + : null, + }, + }; } - protected _getSubstreamCameraID(cameraID: string, view?: View | null): string { - return view?.context?.live?.overrides?.get(cameraID) ?? cameraID; + protected _renderNextPrevious( + side: 'left' | 'right', + neighbors: CameraNeighbors | null, + ): TemplateResult { + const textDirection = getTextDirection(this); + const neighbor = + (textDirection === 'ltr' && side === 'left') || + (textDirection === 'rtl' && side === 'right') + ? neighbors?.previous + : neighbors?.next; + + return html` { + this._setViewCameraID(neighbor?.id); + stopEventFromActivatingCardWideActions(ev); + }} + > + `; } protected render(): TemplateResult | void { @@ -355,14 +410,8 @@ export class FrigateCardLiveCarousel extends LitElement { } const hasMultipleCameras = slides.length > 1; - const [prevID, nextID] = this._getCameraIDsOfNeighbors(); - - const cameraMetadataPrevious = prevID - ? this.cameraManager.getCameraMetadata(this._getSubstreamCameraID(prevID, view)) - : null; - const cameraMetadataNext = nextID - ? this.cameraManager.getCameraMetadata(this._getSubstreamCameraID(nextID, view)) - : null; + const neighbors = this._getCameraNeighbors(); + const forcePTZVisibility = !this._mediaHasLoaded || (!!this.viewFilterCameraID && this.viewFilterCameraID !== view.camera) || @@ -393,35 +442,11 @@ export class FrigateCardLiveCarousel extends LitElement { this._mediaHasLoaded = false; }} > - { - this._setViewCameraID(prevID); - stopEventFromActivatingCardWideActions(ev); - }} - > - + ${this._renderNextPrevious('left', neighbors)} + ${slides} - { - this._setViewCameraID(nextID); - stopEventFromActivatingCardWideActions(ev); - }} - > - + + ${this._renderNextPrevious('right', neighbors)} { + if (!neighbors || !this._media) { + return; + } + const newIndex = + (direction === 'previous' ? neighbors.previous?.index : neighbors.next?.index) ?? + null; + if (newIndex !== null) { + this._setViewSelectedIndex(newIndex); + } + }; + + const textDirection = getTextDirection(this); + const scrollDirection = + (textDirection === 'ltr' && side === 'left') || + (textDirection === 'rtl' && side === 'right') + ? 'previous' + : 'next'; + + return html` { + scroll(scrollDirection); + stopEventFromActivatingCardWideActions(ev); + }} + >`; + } + protected render(): TemplateResult | void { const mediaCount = this._media?.length ?? 0; if (!this._media || !mediaCount) { @@ -324,18 +363,6 @@ export class FrigateCardViewerCarousel extends LitElement { } const neighbors = this._getMediaNeighbors(); - const scroll = (direction: 'previous' | 'next'): void => { - if (!neighbors || !this._media) { - return; - } - const newIndex = - (direction === 'previous' ? neighbors.previous?.index : neighbors.next?.index) ?? - null; - if (newIndex !== null) { - this._setViewSelectedIndex(newIndex); - } - }; - const view = this.viewManagerEpoch?.manager.getView(); return html` @@ -356,37 +383,9 @@ export class FrigateCardViewerCarousel extends LitElement { this._player = null; }} > - ${this.showControls - ? html` { - scroll('previous'); - stopEventFromActivatingCardWideActions(ev); - }} - >` - : ''} + ${this.showControls ? this._renderNextPrevious('left', neighbors) : ''} ${guard([this._media, view], () => this._getSlides())} - ${this.showControls - ? html` { - scroll('next'); - stopEventFromActivatingCardWideActions(ev); - }} - >` - : ''} + ${this.showControls ? this._renderNextPrevious('right', neighbors) : ''} ${view ? html` { + return getComputedStyle(element).direction === 'rtl' ? 'rtl' : 'ltr'; +}; diff --git a/tests/utils/embla/carousel-controller.test.ts b/tests/utils/embla/carousel-controller.test.ts index a2eb6a79..5ee90ec5 100644 --- a/tests/utils/embla/carousel-controller.test.ts +++ b/tests/utils/embla/carousel-controller.test.ts @@ -194,6 +194,7 @@ describe('CarouselController', () => { loop: true, dragEnabled: false, plugins: plugins, + textDirection: 'rtl', }); expect(EmblaCarousel).toBeCalledWith( @@ -209,6 +210,7 @@ describe('CarouselController', () => { watchSlides: false, watchResize: true, watchDrag: false, + direction: 'rtl', }, plugins, ); diff --git a/tests/utils/text-direction.test.ts b/tests/utils/text-direction.test.ts new file mode 100644 index 00000000..4c2014d0 --- /dev/null +++ b/tests/utils/text-direction.test.ts @@ -0,0 +1,26 @@ +import { describe, expect, it } from 'vitest'; +import { getTextDirection } from '../../src/utils/text-direction.js'; + +// @vitest-environment jsdom +describe('getTextDirection', () => { + it('should return rtl', () => { + const element = document.createElement('div'); + element.style.direction = 'rtl'; + + expect(getTextDirection(element)).toBe('rtl'); + }); + + it('should return ltr', () => { + const element = document.createElement('div'); + element.style.direction = 'ltr'; + + expect(getTextDirection(element)).toBe('ltr'); + }); + + it('should return ltr by default', () => { + const element = document.createElement('div'); + element.style.direction = '_ANYTHING_ELSE_'; + + expect(getTextDirection(element)).toBe('ltr'); + }); +}); diff --git a/vite.config.ts b/vite.config.ts index bf807445..f804fd04 100644 --- a/vite.config.ts +++ b/vite.config.ts @@ -38,6 +38,7 @@ const FULL_COVERAGE_FILES_RELATIVE = [ 'utils/ptz.ts', 'utils/screenshot.ts', 'utils/substream.ts', + 'utils/text-direction.ts', 'utils/timer.ts', 'utils/zod.ts', 'view/*.ts',