Fix code quality issues in timeline components

• Fix variable naming consistency (leadout → leadOut)
• Remove unused props from timeline-asset-viewer interface
• Add styleMarginRightOverride prop for dynamic margin control
• Simplify skeleton component styling
This commit is contained in:
midzelis 2025-08-15 01:22:02 +00:00
parent 364468afac
commit 58a95c5a4b
8 changed files with 36 additions and 413 deletions

View File

@ -1,370 +0,0 @@
<script lang="ts">
import { afterNavigate, beforeNavigate } from '$app/navigation';
import { page } from '$app/stores';
import { resizeObserver, type OnResizeCallback } from '$lib/actions/resize-observer';
import AssetGridActions from '$lib/components/timeline/actions/timeline-keyboard-actions.svelte';
import Skeleton from '$lib/components/timeline/base-components/skeleton.svelte';
import SelectableTimelineDay from '$lib/components/timeline/internal-components/selectable-timeline-day.svelte';
import TimelineAssetViewer from '$lib/components/timeline/internal-components/timeline-asset-viewer.svelte';
import { AssetAction } from '$lib/constants';
import type { DayGroup } from '$lib/managers/timeline-manager/day-group.svelte';
import type { MonthGroup } from '$lib/managers/timeline-manager/month-group.svelte';
import { TimelineManager } from '$lib/managers/timeline-manager/timeline-manager.svelte';
import type { TimelineAsset } from '$lib/managers/timeline-manager/types';
import type { AssetInteraction } from '$lib/stores/asset-interaction.svelte';
import { assetViewingStore } from '$lib/stores/asset-viewing.store';
import { mobileDevice } from '$lib/stores/mobile-device.svelte';
import { navigate } from '$lib/utils/navigation';
import { type AlbumResponseDto, type PersonResponseDto } from '@immich/sdk';
import { onMount, type Snippet } from 'svelte';
import type { UpdatePayload } from 'vite';
import Portal from '../shared-components/portal/portal.svelte';
interface Props {
customThumbnailLayout?: Snippet<[TimelineAsset]>;
isSelectionMode?: boolean;
singleSelect?: boolean;
/** `true` if this asset grid is responds to navigation events; if `true`, then look at the
`AssetViewingStore.gridScrollTarget` and load and scroll to the asset specified, and
additionally, update the page location/url with the asset as the asset-grid is scrolled */
enableRouting: boolean;
timelineManager: TimelineManager;
assetInteraction: AssetInteraction;
removeAction?:
| AssetAction.UNARCHIVE
| AssetAction.ARCHIVE
| AssetAction.FAVORITE
| AssetAction.UNFAVORITE
| AssetAction.SET_VISIBILITY_TIMELINE;
withStacked?: boolean;
showArchiveIcon?: boolean;
isShared?: boolean;
album?: AlbumResponseDto | null;
person?: PersonResponseDto | null;
isShowDeleteConfirmation?: boolean;
onAssetOpen?: (dayGroup: DayGroup, asset: TimelineAsset, defaultAssetOpen: () => void) => void;
onSelect?: (asset: TimelineAsset) => void;
onEscape?: () => void;
header?: Snippet<[handleScrollTop: (top: number) => void]>;
children?: Snippet;
empty?: Snippet;
handleTimelineScroll?: () => void;
}
let {
customThumbnailLayout,
isSelectionMode = false,
singleSelect = false,
enableRouting,
timelineManager = $bindable(),
assetInteraction,
removeAction,
withStacked = false,
showArchiveIcon = false,
isShared = false,
album = null,
person = null,
isShowDeleteConfirmation = $bindable(false),
onAssetOpen,
onSelect = (asset: TimelineAsset) => void 0,
onEscape = () => {},
children,
empty,
header,
handleTimelineScroll = () => {},
}: Props = $props();
let { isViewing: showAssetViewer, gridScrollTarget } = assetViewingStore;
let element: HTMLElement | undefined = $state();
let timelineElement: HTMLElement | undefined = $state();
let showSkeleton = $state(true);
let scrubberWidth = $state(0);
const maxMd = $derived(mobileDevice.maxMd);
const usingMobileDevice = $derived(mobileDevice.pointerCoarse);
const isEmpty = $derived(timelineManager.isInitialized && timelineManager.months.length === 0);
$effect(() => {
const layoutOptions = maxMd
? {
rowHeight: 100,
headerHeight: 32,
}
: {
rowHeight: 235,
headerHeight: 48,
};
timelineManager.setLayoutOptions(layoutOptions);
});
const scrollTo = (top: number) => {
if (element) {
element.scrollTo({ top });
}
};
const scrollTop = (top: number) => {
if (element) {
element.scrollTop = top;
}
};
const scrollBy = (y: number) => {
if (element) {
element.scrollBy(0, y);
}
};
const scrollToTop = () => {
scrollTo(0);
};
const onScrollToTop = scrollToTop;
const getAssetHeight = (assetId: string, monthGroup: MonthGroup) => {
// the following method may trigger any layouts, so need to
// handle any scroll compensation that may have been set
const height = monthGroup!.findAssetAbsolutePosition(assetId);
while (timelineManager.scrollCompensation.monthGroup) {
scrollCompensation(timelineManager.scrollCompensation);
timelineManager.clearScrollCompensation();
}
return height;
};
const assetIsVisible = (assetTop: number): boolean => {
if (!element) {
return false;
}
const { clientHeight, scrollTop } = element;
return assetTop >= scrollTop && assetTop < scrollTop + clientHeight;
};
const scrollToAssetId = async (assetId: string) => {
const monthGroup = await timelineManager.findMonthGroupForAsset(assetId);
if (!monthGroup) {
return false;
}
const height = getAssetHeight(assetId, monthGroup);
// If the asset is already visible, then don't scroll.
if (assetIsVisible(height)) {
return true;
}
scrollTo(height);
updateSlidingWindow();
return true;
};
const scrollToAsset = (asset: TimelineAsset) => {
const monthGroup = timelineManager.getMonthGroupByAssetId(asset.id);
if (!monthGroup) {
return false;
}
const height = getAssetHeight(asset.id, monthGroup);
scrollTo(height);
updateSlidingWindow();
return true;
};
const completeNav = async () => {
const scrollTarget = $gridScrollTarget?.at;
let scrolled = false;
if (scrollTarget) {
scrolled = await scrollToAssetId(scrollTarget);
}
if (!scrolled) {
// if the asset is not found, scroll to the top
scrollToTop();
}
showSkeleton = false;
};
beforeNavigate(() => (timelineManager.suspendTransitions = true));
afterNavigate((nav) => {
const { complete } = nav;
complete.then(completeNav, completeNav);
});
const hmrSupport = () => {
// when hmr happens, skeleton is initialized to true by default
// normally, loading asset-grid is part of a navigation event, and the completion of
// that event triggers a scroll-to-asset, if necessary, when then clears the skeleton.
// this handler will run the navigation/scroll-to-asset handler when hmr is performed,
// preventing skeleton from showing after hmr
if (import.meta && import.meta.hot) {
const afterApdate = (payload: UpdatePayload) => {
const assetGridUpdate = payload.updates.some(
(update) => update.path.endsWith('asset-grid.svelte') || update.path.endsWith('assets-store.ts'),
);
if (assetGridUpdate) {
setTimeout(() => {
const asset = $page.url.searchParams.get('at');
if (asset) {
$gridScrollTarget = { at: asset };
void navigate(
{ targetRoute: 'current', assetId: null, assetGridRouteSearchParams: $gridScrollTarget },
{ replaceState: true, forceNavigate: true },
);
} else {
scrollToTop();
}
showSkeleton = false;
}, 500);
}
};
import.meta.hot?.on('vite:afterUpdate', afterApdate);
import.meta.hot?.on('vite:beforeUpdate', (payload) => {
const assetGridUpdate = payload.updates.some((update) => update.path.endsWith('asset-grid.svelte'));
if (assetGridUpdate) {
timelineManager.destroy();
}
});
return () => import.meta.hot?.off('vite:afterUpdate', afterApdate);
}
return () => void 0;
};
const updateIsScrolling = () => (timelineManager.scrolling = true);
// note: don't throttle, debounch, or otherwise do this function async - it causes flicker
const updateSlidingWindow = () => timelineManager.updateSlidingWindow(element?.scrollTop || 0);
const scrollCompensation = ({ heightDelta, scrollTop }: { heightDelta?: number; scrollTop?: number }) => {
if (heightDelta !== undefined) {
scrollBy(heightDelta);
} else if (scrollTop !== undefined) {
scrollTo(scrollTop);
}
// Yes, updateSlideWindow() is called by the onScroll event triggered as a result of
// the above calls. However, this delay is enough time to set the intersecting property
// of the monthGroup to false, then true, which causes the DOM nodes to be recreated,
// causing bad perf, and also, disrupting focus of those elements.
updateSlidingWindow();
};
const onScrollCompensation = scrollCompensation;
const topSectionResizeObserver: OnResizeCallback = ({ height }) => (timelineManager.topSectionHeight = height);
onMount(() => {
if (!enableRouting) {
showSkeleton = false;
}
const disposeHmr = hmrSupport();
return () => {
disposeHmr();
};
});
</script>
<AssetGridActions {scrollToAsset} {timelineManager} {assetInteraction} bind:isShowDeleteConfirmation {onEscape}
></AssetGridActions>
{@render header?.(scrollTop)}
<!-- Right margin MUST be equal to the width of scrubber -->
<section
id="asset-grid"
class={['scrollbar-hidden h-full overflow-y-auto outline-none', { 'm-0': isEmpty }, { 'ms-0': !isEmpty }]}
style:margin-right={(usingMobileDevice ? 0 : scrubberWidth) + 'px'}
tabindex="-1"
bind:clientHeight={timelineManager.viewportHeight}
bind:clientWidth={null, (v: number) => ((timelineManager.viewportWidth = v), updateSlidingWindow())}
bind:this={element}
onscroll={() => (handleTimelineScroll(), updateSlidingWindow(), updateIsScrolling())}
>
<section
bind:this={timelineElement}
id="virtual-timeline"
class:invisible={showSkeleton}
style:height={timelineManager.timelineHeight + 'px'}
>
<section
use:resizeObserver={topSectionResizeObserver}
class:invisible={showSkeleton}
style:position="absolute"
style:left="0"
style:right="0"
>
{@render children?.()}
{#if isEmpty}
<!-- (optional) empty placeholder -->
{@render empty?.()}
{/if}
</section>
{#each timelineManager.months as monthGroup (monthGroup.viewId)}
{@const display = monthGroup.intersecting}
{@const absoluteHeight = monthGroup.top}
{#if !monthGroup.isLoaded}
<div
style:height={monthGroup.height + 'px'}
style:position="absolute"
style:transform={`translate3d(0,${absoluteHeight}px,0)`}
style:width="100%"
>
<Skeleton
height={monthGroup.height - monthGroup.timelineManager.headerHeight}
title={monthGroup.monthGroupTitle}
/>
</div>
{:else if display}
<div
class="month-group"
style:height={monthGroup.height + 'px'}
style:position="absolute"
style:transform={`translate3d(0,${absoluteHeight}px,0)`}
style:width="100%"
>
<SelectableTimelineDay
{customThumbnailLayout}
{withStacked}
{showArchiveIcon}
{assetInteraction}
{timelineManager}
{isSelectionMode}
{singleSelect}
{monthGroup}
{onAssetOpen}
{onSelect}
{onScrollToTop}
{onScrollCompensation}
/>
</div>
{/if}
{/each}
<!-- spacer for leadout -->
<div
class="h-[60px]"
style:position="absolute"
style:left="0"
style:right="0"
style:transform={`translate3d(0,${timelineManager.timelineHeight}px,0)`}
></div>
</section>
</section>
<Portal target="body">
{#if $showAssetViewer}
<TimelineAssetViewer bind:showSkeleton {timelineManager} {removeAction} {withStacked} {isShared} {album} {person} />
{/if}
</Portal>
<style>
#asset-grid {
contain: strict;
scrollbar-width: none;
}
.month-group {
contain: layout size paint;
transform-style: flat;
backface-visibility: hidden;
transform-origin: center center;
}
</style>

View File

@ -30,6 +30,7 @@
showArchiveIcon?: boolean;
showSkeleton?: boolean;
isShowDeleteConfirmation?: boolean;
styleMarginRightOverride?: string;
onAssetOpen?: (dayGroup: DayGroup, asset: TimelineAsset, defaultAssetOpen: () => void) => void;
onSelect?: (asset: TimelineAsset) => void;
header?: Snippet<[handleScrollTop: (top: number) => void]>;
@ -49,6 +50,7 @@
withStacked = false,
showSkeleton = $bindable(true),
showArchiveIcon = false,
styleMarginRightOverride,
isShowDeleteConfirmation = $bindable(false),
onAssetOpen,
@ -63,10 +65,8 @@
let element: HTMLElement | undefined = $state();
let timelineElement: HTMLElement | undefined = $state();
let scrubberWidth = $state(0);
const maxMd = $derived(mobileDevice.maxMd);
const usingMobileDevice = $derived(mobileDevice.pointerCoarse);
const isEmpty = $derived(timelineManager.isInitialized && timelineManager.months.length === 0);
$effect(() => {
@ -213,7 +213,7 @@
};
const updateIsScrolling = () => (timelineManager.scrolling = true);
// note: don't throttle, debounch, or otherwise do this function async - it causes flicker
// note: don't throttle, debounce, or otherwise do this function async - it causes flicker
const updateSlidingWindow = () => timelineManager.updateSlidingWindow(element?.scrollTop || 0);
const scrollCompensation = ({ heightDelta, scrollTop }: { heightDelta?: number; scrollTop?: number }) => {
@ -249,7 +249,7 @@
<section
id="asset-grid"
class={['scrollbar-hidden h-full overflow-y-auto outline-none', { 'm-0': isEmpty }, { 'ms-0': !isEmpty }]}
style:margin-right={(usingMobileDevice ? 0 : scrubberWidth) + 'px'}
style:margin-right={styleMarginRightOverride}
tabindex="-1"
bind:clientHeight={timelineManager.viewportHeight}
bind:clientWidth={null, (v: number) => ((timelineManager.viewportWidth = v), updateSlidingWindow())}

View File

@ -26,8 +26,8 @@
showSkeleton?: boolean;
isShowDeleteConfirmation?: boolean;
onAssetOpen?: (dayGroup: DayGroup, asset: TimelineAsset, defaultAssetOpen: () => void) => void;
onSelect?: (asset: TimelineAsset) => void;
onEscape?: () => void;
children?: Snippet;
empty?: Snippet;
@ -51,16 +51,16 @@
empty,
}: Props = $props();
let leadout = $state(false);
let leadOut = $state(false);
let scrubberMonthPercent = $state(0);
let scrubberMonth: { year: number; month: number } | undefined = $state(undefined);
let scrubOverallPercent: number = $state(0);
let scrubberWidth: number = $state(0);
// note: don't throttle, debounch, or otherwise make this function async - it causes flicker
// note: don't throttle, debounce, or otherwise make this function async - it causes flicker
// this function updates the scrubber position based on the current scroll position in the timeline
const handleTimelineScroll = () => {
leadout = false;
leadOut = false;
if (timelineManager.timelineHeight < timelineManager.viewportHeight * 2) {
// edge case - scroll limited due to size of content, must adjust - use the overall percent instead
@ -119,7 +119,7 @@
top = next;
}
if (!found) {
leadout = true;
leadOut = true;
scrubberMonth = undefined;
scrubberMonthPercent = 0;
scrubOverallPercent = 1;
@ -127,7 +127,7 @@
}
};
// note: don't throttle, debounch, or otherwise make this function async - it causes flicker
// note: don't throttle, debounce, or otherwise make this function async - it causes flicker
// this function scrolls the timeline to the specified month group and offset, based on scrubber interaction
const onScrub: ScrubberListener = ({
scrubberMonth,
@ -167,7 +167,8 @@
export const scrollToAsset = (asset: TimelineAsset) => baseTimelineViewer?.scrollToAsset(asset) ?? false;
</script>
<TimelineViewer
<BaseTimelineViewer
{customThumbnailLayout}
{isSelectionMode}
{singleSelect}
{enableRouting}
@ -177,6 +178,7 @@
{showArchiveIcon}
{showSkeleton}
{isShowDeleteConfirmation}
styleMarginRightOverride={scrubberWidth + 'px'}
{onAssetOpen}
{onSelect}
{children}
@ -190,7 +192,7 @@
height={timelineManager.viewportHeight}
timelineTopOffset={timelineManager.topSectionHeight}
timelineBottomOffset={timelineManager.bottomSectionHeight}
{leadout}
{leadOut}
{scrubOverallPercent}
{scrubberMonthPercent}
{scrubberMonth}

View File

@ -18,7 +18,7 @@
scrubOverallPercent?: number;
scrubberMonthPercent?: number;
scrubberMonth?: { year: number; month: number };
leadout?: boolean;
leadOut?: boolean;
scrubberWidth?: number;
onScrub?: ScrubberListener;
onScrubKeyDown?: (event: KeyboardEvent, element: HTMLElement) => void;
@ -34,7 +34,7 @@
scrubOverallPercent = 0,
scrubberMonthPercent = 0,
scrubberMonth = undefined,
leadout = false,
leadOut = false,
onScrub = undefined,
onScrubKeyDown = undefined,
startScrub = undefined,
@ -100,7 +100,7 @@
offset += scrubberMonthPercent * relativeBottomOffset;
}
return offset;
} else if (leadout) {
} else if (leadOut) {
let offset = relativeTopOffset;
for (const segment of segments) {
offset += segment.height;

View File

@ -13,11 +13,7 @@
>
{title}
</div>
<div
class="animate-pulse absolute h-full ms-[10px] me-[10px]"
style:width="calc(100% - 20px)"
data-skeleton="true"
></div>
<div class="animate-pulse absolute h-full w-full" data-skeleton="true"></div>
</div>
<style>

View File

@ -7,7 +7,7 @@
import { updateStackedAssetInTimeline, updateUnstackedAssetInTimeline } from '$lib/utils/actions';
import { navigate } from '$lib/utils/navigation';
import { toTimelineAsset } from '$lib/utils/timeline-util';
import { getAssetInfo, type AlbumResponseDto, type AssetResponseDto, type PersonResponseDto } from '@immich/sdk';
import { getAssetInfo, type AlbumResponseDto, type PersonResponseDto } from '@immich/sdk';
let { asset: viewingAsset, gridScrollTarget, mutex, preloadAssets } = assetViewingStore;
@ -25,31 +25,19 @@
| AssetAction.FAVORITE
| AssetAction.UNFAVORITE
| AssetAction.SET_VISIBILITY_TIMELINE;
handlePreAction?: (action: Action) => Promise<void>;
handleAction?: (action: Action) => void;
handleNext?: () => Promise<boolean>;
handlePrevious?: () => Promise<boolean>;
handleRandom?: () => Promise<AssetResponseDto | undefined>;
handleClose?: (asset: { id: string }) => Promise<void>;
}
let {
timelineManager = $bindable(),
timelineManager,
showSkeleton = $bindable(false),
removeAction,
withStacked = false,
isShared = false,
album = null,
person = null,
handlePreAction = $bindable(),
handleAction = $bindable(),
handleNext = $bindable(),
handlePrevious = $bindable(),
handleRandom = $bindable(),
handleClose = $bindable(),
}: Props = $props();
handlePrevious = async () => {
const handlePrevious = async () => {
const release = await mutex.acquire();
const laterAsset = await timelineManager.getLaterAsset($viewingAsset);
@ -64,7 +52,7 @@
return !!laterAsset;
};
handleNext = async () => {
const handleNext = async () => {
const release = await mutex.acquire();
const earlierAsset = await timelineManager.getEarlierAsset($viewingAsset);
@ -79,7 +67,7 @@
return !!earlierAsset;
};
handleRandom = async () => {
const handleRandom = async () => {
const randomAsset = await timelineManager.getRandomAsset();
if (randomAsset) {
@ -90,14 +78,14 @@
}
};
handleClose = async (asset: { id: string }) => {
const handleClose = async (asset: { id: string }) => {
assetViewingStore.showAssetViewer(false);
showSkeleton = true;
$gridScrollTarget = { at: asset.id };
await navigate({ targetRoute: 'current', assetId: null, assetGridRouteSearchParams: $gridScrollTarget });
};
handlePreAction = async (action: Action) => {
const handlePreAction = async (action: Action) => {
switch (action.type) {
case removeAction:
case AssetAction.TRASH:
@ -116,7 +104,7 @@
}
}
};
handleAction = (action: Action) => {
const handleAction = (action: Action) => {
switch (action.type) {
case AssetAction.ARCHIVE:
case AssetAction.UNARCHIVE:

View File

@ -4,6 +4,7 @@
import BaseTimeline from '$lib/components/timeline/base-components/base-timeline.svelte';
import TimelineAssetViewer from '$lib/components/timeline/internal-components/timeline-asset-viewer.svelte';
import type { AssetAction } from '$lib/constants';
import type { DayGroup } from '$lib/managers/timeline-manager/day-group.svelte';
import type { TimelineManager } from '$lib/managers/timeline-manager/timeline-manager.svelte';
import type { TimelineAsset } from '$lib/managers/timeline-manager/types';
import type { AssetInteraction } from '$lib/stores/asset-interaction.svelte';
@ -14,6 +15,7 @@
let { isViewing: showAssetViewer } = assetViewingStore;
interface Props {
customThumbnailLayout?: Snippet<[TimelineAsset]>;
isSelectionMode?: boolean;
singleSelect?: boolean;
/** `true` if this asset grid is responds to navigation events; if `true`, then look at the
@ -34,6 +36,7 @@
album?: AlbumResponseDto | null;
person?: PersonResponseDto | null;
isShowDeleteConfirmation?: boolean;
onAssetOpen?: (dayGroup: DayGroup, asset: TimelineAsset, defaultAssetOpen: () => void) => void;
onSelect?: (asset: TimelineAsset) => void;
onEscape?: () => void;
children?: Snippet;
@ -41,6 +44,7 @@
}
let {
customThumbnailLayout,
isSelectionMode = false,
singleSelect = false,
enableRouting,
@ -53,6 +57,7 @@
album = null,
person = null,
isShowDeleteConfirmation = $bindable(false),
onAssetOpen,
onSelect = () => {},
onEscape = () => {},
children,
@ -65,6 +70,7 @@
<BaseTimeline
bind:this={viewer}
{customThumbnailLayout}
{isSelectionMode}
{singleSelect}
{enableRouting}
@ -74,6 +80,7 @@
{showArchiveIcon}
{isShowDeleteConfirmation}
{showSkeleton}
{onAssetOpen}
{onSelect}
{children}
{empty}

View File

@ -2,7 +2,7 @@
import UserPageLayout from '$lib/components/layouts/user-page-layout.svelte';
import ChangeLocation from '$lib/components/shared-components/change-location.svelte';
import EmptyPlaceholder from '$lib/components/shared-components/empty-placeholder.svelte';
import AssetGrid from '$lib/components/timeline/base-components/base-timeline.svelte';
import Timeline from '$lib/components/timeline/timeline.svelte';
import { AssetAction } from '$lib/constants';
import { authManager } from '$lib/managers/auth-manager.svelte';
import type { DayGroup } from '$lib/managers/timeline-manager/day-group.svelte';
@ -175,7 +175,7 @@
</div>
{/if}
<AssetGrid
<Timeline
isSelectionMode={true}
enableRouting={true}
{timelineManager}
@ -199,5 +199,5 @@
{#snippet empty()}
<EmptyPlaceholder text={$t('no_assets_message')} onClick={() => {}} />
{/snippet}
</AssetGrid>
</Timeline>
</UserPageLayout>