From b038917f7a3798df55be3ffabc32d805dcb53b8f Mon Sep 17 00:00:00 2001 From: Min Idzelis Date: Thu, 8 May 2025 02:14:13 +0000 Subject: [PATCH] Review comments --- .../assets/thumbnail/thumbnail.svelte | 29 +-- .../photos-page/actions/focus-actions.ts | 45 ++-- .../components/photos-page/asset-grid.svelte | 6 +- .../gallery-viewer/gallery-viewer.svelte | 22 +- .../shared-components/select-date.svelte | 12 +- web/src/lib/stores/assets-store.svelte.ts | 231 ++++++++++-------- web/src/lib/utils/focus-util.ts | 2 +- web/src/lib/utils/retry.ts | 2 +- 8 files changed, 177 insertions(+), 172 deletions(-) diff --git a/web/src/lib/components/assets/thumbnail/thumbnail.svelte b/web/src/lib/components/assets/thumbnail/thumbnail.svelte index b6ca63ff29..588e6dff7a 100644 --- a/web/src/lib/components/assets/thumbnail/thumbnail.svelte +++ b/web/src/lib/components/assets/thumbnail/thumbnail.svelte @@ -19,7 +19,7 @@ import { thumbhash } from '$lib/actions/thumbhash'; import { authManager } from '$lib/managers/auth-manager.svelte'; import { mobileDevice } from '$lib/stores/mobile-device.svelte'; - import { focusNext } from '$lib/utils/focus-util'; + import { moveFocus } from '$lib/utils/focus-util'; import { currentUrlReplaceAssetId } from '$lib/utils/navigation'; import { TUNABLES } from '$lib/utils/tunables'; import { onMount } from 'svelte'; @@ -136,17 +136,13 @@ let startX: number = 0; let startY: number = 0; - // As of iOS17, there is a preference for long press speed, which is not available for mobile web. - // The defaults are as follows: - // fast: 200ms - // default: 500ms - // slow: ??ms function longPress(element: HTMLElement, { onLongPress }: { onLongPress: () => void }) { let didPress = false; const start = (evt: PointerEvent) => { startX = evt.clientX; startY = evt.clientY; didPress = false; + // 350ms for longpress. For reference: iOS uses 500ms for default long press, or 200ms for fast long press. timer = setTimeout(() => { onLongPress(); element.addEventListener('contextmenu', preventContextMenu, { once: true }); @@ -211,7 +207,7 @@ onSelect?.(asset); } if (document.activeElement === element && evt.key === 'Escape') { - focusNext((element) => element.dataset.thumbnailFocusContainer === undefined, true); + moveFocus((element) => element.dataset.thumbnailFocusContainer === undefined, true); } }} onclick={handleClick} @@ -243,25 +239,6 @@ class={['group absolute top-[0px] bottom-[0px]', { 'cursor-not-allowed': disabled, 'cursor-pointer': !disabled }]} style:width="inherit" style:height="inherit" - onmouseenter={onMouseEnter} - onmouseleave={onMouseLeave} - use:longPress={{ onLongPress: () => onSelect?.($state.snapshot(asset)) }} - onkeydown={(evt) => { - if (evt.key === 'Enter') { - callClickHandlers(); - } - if (evt.key === 'x') { - onSelect?.(asset); - } - if (document.activeElement === element && evt.key === 'Escape') { - focusNext((element) => element.dataset.thumbnailFocusContainer === undefined, true); - } - }} - onclick={handleClick} - bind:this={element} - data-thumbnail-focus-container - tabindex={0} - role="link" > {#if !usingMobileDevice && mouseOver && !disableLinkMouseOver} diff --git a/web/src/lib/components/photos-page/actions/focus-actions.ts b/web/src/lib/components/photos-page/actions/focus-actions.ts index f06fc2e944..b489ac3984 100644 --- a/web/src/lib/components/photos-page/actions/focus-actions.ts +++ b/web/src/lib/components/photos-page/actions/focus-actions.ts @@ -1,5 +1,5 @@ import type { AssetStore } from '$lib/stores/assets-store.svelte'; -import { focusNext } from '$lib/utils/focus-util'; +import { moveFocus } from '$lib/utils/focus-util'; import { InvocationTracker } from '$lib/utils/invocationTracker'; import { retry } from '$lib/utils/retry'; @@ -12,9 +12,9 @@ const getFocusedThumb = () => { } }; -export const focusNextAsset = () => focusNext((element) => element.dataset.thumbnailFocusContainer !== undefined, true); +export const focusNextAsset = () => moveFocus((element) => element.dataset.thumbnailFocusContainer !== undefined, true); export const focusPreviousAsset = () => - focusNext((element) => element.dataset.thumbnailFocusContainer !== undefined, false); + moveFocus((element) => element.dataset.thumbnailFocusContainer !== undefined, false); export const setFocusToAsset = async (scrollToAsset: (id: string) => Promise, asset: { id: string }) => { const scrolled = await scrollToAsset(asset.id); @@ -40,25 +40,30 @@ export const setFocusTo = async ( } const invocation = tracker.startInvocation(); + const id = thumb.dataset.asset; + if (!thumb || !id) { + invocation.endInvocation(); + return; + } try { - if (thumb) { - const id = thumb?.dataset.asset; - if (id) { - const asset = - direction === 'next' ? await store.getNextAsset({ id }, skip) : await store.getPreviousAsset({ id }, skip); - invocation.checkStillValid(); - if (asset) { - const scrolled = await scrollToAsset(asset.id); - invocation.checkStillValid(); - if (scrolled) { - invocation.checkStillValid(); - const element = await waitForElement(`[data-thumbnail-focus-container][data-asset="${asset.id}"]`); - invocation.checkStillValid(); - element?.focus(); - } - } - } + const asset = + direction === 'next' ? await store.getNextAsset({ id }, skip) : await store.getPreviousAsset({ id }, skip); + invocation.checkStillValid(); + + if (!asset) { + invocation.endInvocation(); + return; } + + const scrolled = await scrollToAsset(asset.id); + invocation.checkStillValid(); + + if (scrolled) { + const element = await waitForElement(`[data-thumbnail-focus-container][data-asset="${asset.id}"]`); + invocation.checkStillValid(); + element?.focus(); + } + invocation.endInvocation(); } catch (error: unknown) { if (invocation.isInvalidInvocationError(error)) { diff --git a/web/src/lib/components/photos-page/asset-grid.svelte b/web/src/lib/components/photos-page/asset-grid.svelte index 075c8644d7..229f2bbc32 100644 --- a/web/src/lib/components/photos-page/asset-grid.svelte +++ b/web/src/lib/components/photos-page/asset-grid.svelte @@ -21,7 +21,7 @@ import { handlePromiseError } from '$lib/utils'; import { deleteAssets, updateStackedAssetInTimeline, updateUnstackedAssetInTimeline } from '$lib/utils/actions'; import { archiveAssets, cancelMultiselect, selectAllAssets, stackAssets } from '$lib/utils/asset-utils'; - import { focusNext } from '$lib/utils/focus-util'; + import { moveFocus } from '$lib/utils/focus-util'; import { navigate } from '$lib/utils/navigation'; import { type ScrubberListener } from '$lib/utils/timeline-util'; import type { AlbumResponseDto, AssetResponseDto, PersonResponseDto } from '@immich/sdk'; @@ -629,8 +629,8 @@ } }; - const focusNextAsset = () => focusNext((element) => element.dataset.thumbnailFocusContainer !== undefined, true); - const focusPreviousAsset = () => focusNext((element) => element.dataset.thumbnailFocusContainer !== undefined, false); + const focusNextAsset = () => moveFocus((element) => element.dataset.thumbnailFocusContainer !== undefined, true); + const focusPreviousAsset = () => moveFocus((element) => element.dataset.thumbnailFocusContainer !== undefined, false); let isTrashEnabled = $derived($featureFlags.loaded && $featureFlags.trash); let isEmpty = $derived(assetStore.isInitialized && assetStore.buckets.length === 0); diff --git a/web/src/lib/components/shared-components/gallery-viewer/gallery-viewer.svelte b/web/src/lib/components/shared-components/gallery-viewer/gallery-viewer.svelte index 09c126e0df..ab373f3cf6 100644 --- a/web/src/lib/components/shared-components/gallery-viewer/gallery-viewer.svelte +++ b/web/src/lib/components/shared-components/gallery-viewer/gallery-viewer.svelte @@ -1,28 +1,28 @@ a.id == id); - if (index !== -1) { - return true; - } - } - return false; + return this.assets().some((asset) => asset.id == id); } sortDateGroups() { @@ -1333,7 +1327,7 @@ export class AssetStore { idable: { id: string }, skipTo: 'asset' | 'day' | 'month' | 'year' = 'asset', ): Promise { - let bucket = this.#findBucketForAsset(idable.id); + const bucket = this.#findBucketForAsset(idable.id); if (!bucket) { return; } @@ -1343,61 +1337,77 @@ export class AssetStore { } switch (skipTo) { case 'day': { - let nextDay = DateTime.fromISO(asset.localDateTime).toUTC().get('day') + 1; - const bIdx = this.buckets.indexOf(bucket); - - let nextDaygroup; - while (nextDay <= 31) { - nextDaygroup = bucket.findDateGroupByDay(nextDay); - if (nextDaygroup) { - break; - } - nextDay++; - } - if (nextDaygroup === undefined) { - let bucketIndex = bIdx - 1; - while (bucketIndex >= 0) { - bucket = this.buckets[bucketIndex]; - if (!bucket) { - return; - } - await this.loadBucket(bucket.bucketDate, { cancelable: false }); - const previous = bucket.lastDateGroup?.intersetingAssets.at(0)?.asset; - if (previous) { - return previous; - } - bucketIndex--; - } - } else { - return nextDaygroup.intersetingAssets.at(0)?.asset; - } - return; + return this.#getPreviousDay(asset, bucket); } case 'month': { - const bIdx = this.buckets.indexOf(bucket); - const otherBucket = this.buckets[bIdx - 1]; - if (otherBucket) { - await this.loadBucket(otherBucket.bucketDate, { cancelable: false }); - return otherBucket.dateGroups[0]?.intersetingAssets[0]?.asset; - } - return; + return this.#getPreviousMonth(asset, bucket); } case 'year': { - const nextYear = DateTime.fromISO(asset.localDateTime).toUTC().get('year') + 1; - const bIdx = this.buckets.indexOf(bucket); - - for (let idx = bIdx; idx >= 0; idx--) { - const otherBucket = this.buckets[idx]; - const otherBucketYear = DateTime.fromISO(otherBucket.bucketDate).toUTC().get('year'); - if (otherBucketYear >= nextYear) { - await this.loadBucket(otherBucket.bucketDate, { cancelable: false }); - return otherBucket.dateGroups[0]?.intersetingAssets[0]?.asset; - } - } - return; + return this.#getPreviousYear(asset, bucket); + } + case 'asset': { + return this.#getPreviousAsset(asset, bucket); } } + } + async #getPreviousDay(asset: AssetResponseDto, bucket: AssetBucket) { + let nextDay = DateTime.fromISO(asset.localDateTime).toUTC().get('day') + 1; + const bucketIndex = this.buckets.indexOf(bucket); + + let nextDaygroup; + while (nextDay <= 31) { + nextDaygroup = bucket.findDateGroupByDay(nextDay); + if (nextDaygroup) { + break; + } + nextDay++; + } + if (nextDaygroup === undefined) { + let previousBucketIndex = bucketIndex - 1; + while (previousBucketIndex >= 0) { + bucket = this.buckets[previousBucketIndex]; + if (!bucket) { + return; + } + await this.loadBucket(bucket.bucketDate, { cancelable: false }); + const previous = bucket.lastDateGroup?.intersetingAssets.at(0)?.asset; + if (previous) { + return previous; + } + previousBucketIndex--; + } + } else { + return nextDaygroup.intersetingAssets.at(0)?.asset; + } + } + + async #getPreviousMonth(asset: AssetResponseDto, bucket: AssetBucket) { + const bucketIndex = this.buckets.indexOf(bucket); + const previousBucket = this.buckets[bucketIndex - 1]; + if (previousBucket) { + await this.loadBucket(previousBucket.bucketDate, { cancelable: false }); + return previousBucket.dateGroups[0]?.intersetingAssets[0]?.asset; + } + return; + } + + async #getPreviousYear(asset: AssetResponseDto, bucket: AssetBucket) { + const nextYear = DateTime.fromISO(asset.localDateTime).toUTC().get('year') + 1; + const bIdx = this.buckets.indexOf(bucket); + + for (let idx = bIdx; idx >= 0; idx--) { + const otherBucket = this.buckets[idx]; + const otherBucketYear = DateTime.fromISO(otherBucket.bucketDate).toUTC().get('year'); + if (otherBucketYear >= nextYear) { + await this.loadBucket(otherBucket.bucketDate, { cancelable: false }); + return otherBucket.dateGroups[0]?.intersetingAssets[0]?.asset; + } + } + return; + } + + async #getPreviousAsset(asset: AssetResponseDto, bucket: AssetBucket) { // Find which date group contains this asset for (let groupIndex = 0; groupIndex < bucket.dateGroups.length; groupIndex++) { const group = bucket.dateGroups[groupIndex]; @@ -1462,7 +1472,7 @@ export class AssetStore { idable: { id: string }, skipTo: 'asset' | 'day' | 'month' | 'year' = 'asset', ): Promise { - let bucket = this.#findBucketForAsset(idable.id); + const bucket = this.#findBucketForAsset(idable.id); if (!bucket) { return; } @@ -1473,58 +1483,73 @@ export class AssetStore { switch (skipTo) { case 'day': { - let prevDay = DateTime.fromISO(asset.localDateTime).toUTC().get('day') - 1; - const bIdx = this.buckets.indexOf(bucket); - - let prevDayGroup; - while (prevDay >= 0) { - prevDayGroup = bucket.findDateGroupByDay(prevDay); - if (prevDayGroup) { - break; - } - prevDay--; - } - if (prevDayGroup === undefined) { - let bucketIndex = bIdx + 1; - while (bucketIndex < this.buckets.length) { - const otherBucket = this.buckets[bucketIndex]; - await this.loadBucket(otherBucket.bucketDate, { cancelable: false }); - const next = otherBucket.dateGroups[0]?.intersetingAssets[0]?.asset; - if (next) { - return next; - } - bucketIndex++; - } - } else { - return prevDayGroup.intersetingAssets.at(0)?.asset; - } - - return; + return this.#getNextDay(asset, bucket); } case 'month': { - const bIdx = this.buckets.indexOf(bucket); - const otherBucket = this.buckets[bIdx + 1]; - if (otherBucket) { - await this.loadBucket(otherBucket.bucketDate, { cancelable: false }); - return otherBucket.dateGroups[0]?.intersetingAssets[0]?.asset; - } - return; + return this.#getNextMonth(asset, bucket); } case 'year': { - const prevYear = DateTime.fromISO(asset.localDateTime).toUTC().get('year') - 1; - const bIdx = this.buckets.indexOf(bucket); - for (let idx = bIdx; idx < this.buckets.length - 1; idx++) { - const otherBucket = this.buckets[idx]; - const otherBucketYear = DateTime.fromISO(otherBucket.bucketDate).toUTC().get('year'); - if (otherBucketYear <= prevYear) { - await this.loadBucket(otherBucket.bucketDate, { cancelable: false }); - const a = otherBucket.dateGroups[0]?.intersetingAssets[0]?.asset; - return a; - } - } - return; + return this.#getNextYear(asset, bucket); + } + case 'asset': { + return this.#getNextAsset(asset, bucket); } } + } + + async #getNextDay(asset: AssetResponseDto, bucket: AssetBucket) { + let prevDay = DateTime.fromISO(asset.localDateTime).toUTC().get('day') - 1; + const bucketIndex = this.buckets.indexOf(bucket); + + let prevDayGroup; + while (prevDay >= 0) { + prevDayGroup = bucket.findDateGroupByDay(prevDay); + if (prevDayGroup) { + break; + } + prevDay--; + } + if (prevDayGroup === undefined) { + let nextBucketIndex = bucketIndex + 1; + while (nextBucketIndex < this.buckets.length) { + const otherBucket = this.buckets[nextBucketIndex]; + await this.loadBucket(otherBucket.bucketDate, { cancelable: false }); + const next = otherBucket.dateGroups[0]?.intersetingAssets[0]?.asset; + if (next) { + return next; + } + nextBucketIndex++; + } + } else { + return prevDayGroup.intersetingAssets.at(0)?.asset; + } + } + + async #getNextMonth(asset: AssetResponseDto, bucket: AssetBucket) { + const bucketIndex = this.buckets.indexOf(bucket); + const nextMonthBucketIndex = this.buckets[bucketIndex + 1]; + if (nextMonthBucketIndex) { + await this.loadBucket(nextMonthBucketIndex.bucketDate, { cancelable: false }); + return nextMonthBucketIndex.dateGroups[0]?.intersetingAssets[0]?.asset; + } + return; + } + + async #getNextYear(asset: AssetResponseDto, bucket: AssetBucket) { + const prevYear = DateTime.fromISO(asset.localDateTime).toUTC().get('year') - 1; + const bucketIndex = this.buckets.indexOf(bucket); + for (let idx = bucketIndex; idx < this.buckets.length - 1; idx++) { + const otherBucket = this.buckets[idx]; + const otherBucketYear = DateTime.fromISO(otherBucket.bucketDate).toUTC().get('year'); + if (otherBucketYear <= prevYear) { + await this.loadBucket(otherBucket.bucketDate, { cancelable: false }); + return otherBucket.dateGroups[0]?.intersetingAssets[0]?.asset; + } + } + return; + } + + async #getNextAsset(asset: AssetResponseDto, bucket: AssetBucket) { // Find which date group contains this asset for (let groupIndex = 0; groupIndex < bucket.dateGroups.length; groupIndex++) { const group = bucket.dateGroups[groupIndex]; diff --git a/web/src/lib/utils/focus-util.ts b/web/src/lib/utils/focus-util.ts index c95ed3f31d..ae3f853040 100644 --- a/web/src/lib/utils/focus-util.ts +++ b/web/src/lib/utils/focus-util.ts @@ -12,7 +12,7 @@ export const setDefaultTabbleOptions = (options: TabbableOpts) => { export const getTabbable = (container: Element, includeContainer: boolean = false) => tabbable(container, { ...defaultOpts, includeContainer }); -export const focusNext = (selector: (element: HTMLElement | SVGElement) => boolean, forwardDirection: boolean) => { +export const moveFocus = (selector: (element: HTMLElement | SVGElement) => boolean, forwardDirection: boolean) => { const focusElements = focusable(document.body, { includeContainer: true }); const current = document.activeElement as HTMLElement; const index = focusElements.indexOf(current); diff --git a/web/src/lib/utils/retry.ts b/web/src/lib/utils/retry.ts index 3f57c3c239..df4791b939 100644 --- a/web/src/lib/utils/retry.ts +++ b/web/src/lib/utils/retry.ts @@ -1,4 +1,4 @@ -export const retry = ( +export const retry = ( fn: (arg: A) => R, interval: number = 10, timeout: number = 1000,