SPIKE: Refactor the memory viewer using a state machine library (xstate) to see if it will end up cleaner by knechtandreas · Pull Request #1 · knechtandreas/immich

knechtandreas · 2025-03-17T22:32:58Z

Description

So I spent a lot of time in a previous PR fixing numerous state related bugs in the memory viewer (see immich-app#16759). The memory viewer is actually fairly complex in terms of all the different states it can be in, as well as dealing with playback etc.

Here's a spike using XState to see if this code could be cleaned up/be made more robust through the use of this library. I'm more than happy for this PR to be rejected if the majority of people think it's not a good idea.

Despite the fact that I'm new to xstate, I'm a pretty big fan for this particular use case. It was pretty quick to get going and the docs are great. Plus, you can copy and paste the code from memory-viewer-state-machine.ts directly into their UI state machine designer and you get this:

You can even play around with this on the web: https://stately.ai/registry/editor/9ecaa2c1-7d54-408f-8ae4-38048eb71fe5?mode=design&machineId=90c83855-be76-468c-ba32-a8334de917df

…uplicate object refs of the same asset.

…rrectly from the server.

…s-broken * main: feat(mobile): person age on photo properties (immich-app#16728) refactor: migrate library spec to factories (immich-app#16711) chore: remove unused file (immich-app#16707) fix(mobile): fix notification icon not displaying properly (immich-app#16710)

…s-broken * main: fix(server): set the dev server restart policy of the dev server container to match the other containers (immich-app#16753) feat(web): remember search context (immich-app#16614) feat(server): read Android and Sony video camera make/model (immich-app#16678) fix(server): adjust type of person.birthDate (immich-app#16628) fix(web): add labels to memory lane buttons (immich-app#16664) feat(mobile): locate in timeline (immich-app#16722) chore(ml): uv (immich-app#16725) fix: 🍪 packages confusion (immich-app#16735) fix(web): Update people-card favorite position (immich-app#16746) chore(mobile): upgrade riverpod (immich-app#16742) chore(mobile): upgrade flutter_web_auth_2 (immich-app#16741) fix(docs): edge case when restoring dump that is unreadable as current user (immich-app#16758)

* Updated to `Tween` (`tweened` was deprecated) * Removed `resetPromise`. Setting progressController to 0 had the same effect, so not really sure why it was there? * Removed the many duplicate places the `handleAction` method was called. Now we just called it on `afterNavigate` as well as when `galleryInView` or `$isViewing` state changes.

…y call 'reset' and 'play' once, after navigate/page load. This should hopefully fix all the various bugs around playback.

…s-broken * main: (41 commits) chore(deps): update typescript-projects (immich-app#16795) fix: immich ui toggles and switches (immich-app#16834) feat: better library rename UX (immich-app#16837) refactor: use immich/ui button component in user settings (immich-app#16836) chore(mobile): bump dependency versions (immich-app#16823) fix(web): fixed formatting of video length (immich-app#16829) refactor: user entity (immich-app#16655) fix(web): update search results when searching from info panel (immich-app#16729) fix(docs): logo not loading dark theme variant in production (immich-app#16820) refactor: use factory and kysely types for partner repository (immich-app#16812) fix(ml): dev environment dependencies (immich-app#16815) fix: run preview label remove job on PR close (immich-app#16811) feat(web): Add keyboard shortcut selection on grid (immich-app#16713) feat(web): better person naming interface (immich-app#16631) chore(mobile): use path provider foundation (immich-app#16804) chore(mobile): add orientation tests for exif (immich-app#16806) chore: ignore correct build folder (immich-app#16808) fix(server): set unit test timezone to UTC (immich-app#16805) chore: shared suffix for docker tags (immich-app#16727) chore(deps): update base-image to v20250311 (major) (immich-app#16803) ...

github-actions · 2025-03-17T22:33:09Z

Label error. Requires exactly 1 of: changelog:.*. Found: 🖥️web. A maintainer will add the required label.

knechtandreas · 2025-03-17T22:34:38Z

web/src/lib/components/memory-page/memory-viewer.svelte

+  import { onMount } from 'svelte';
+  import { memoryViewerMachine } from '$lib/components/memory-page/memory-viewer-state-machine';
+
+  const { snapshot, actorRef: memoryViewerActor } = useMachine(memoryViewerMachine);


The @xstate/svelte dep provides some nice integrations for Svelte specifically, by exposing the snapshot as a svelte store etc.

knechtandreas · 2025-03-17T22:35:38Z

web/src/lib/components/memory-page/memory-viewer.svelte


  const toProgressPercentage = (index: number) => {
-    if (!progressBarController || current?.assetIndex === undefined) {
+    if (current?.assetIndex === undefined || $snapshot.context.durationMs === 0) {


Using the state machine now to track duration and elapsed.

knechtandreas · 2025-03-17T22:36:34Z

web/src/lib/components/memory-page/memory-viewer.svelte

    }
+    const previousAssetId = memoryStore.getMemoryAsset(ids[0])?.previous?.asset.id;
    memoryStore.hideAssetsFromMemory(ids);
-    init(page);


This was a bit broken previously. Not really sure what would happen after say an asset was deleted. Usually we'd skip back to the very first asset.

Now an attempt is made to just jump to the previous asset (if there was one).

knechtandreas · 2025-03-17T22:37:14Z

web/src/lib/components/memory-page/memory-viewer.svelte

-      return;
+  $effect(() => {
+    if (memoryViewerActor.getSnapshot().can({ type: 'TIMING' })) {
+      memoryViewerActor.send({ type: 'TIMING', elapsedMs: photoProgressController.current * PHOTO_PLAY_DURATION });


When the tween updates, we send a TIMING event to the state machine to update the elapsed time.

knechtandreas · 2025-03-17T22:39:08Z

web/src/lib/components/memory-page/memory-viewer.svelte

                    muted={$videoViewerMuted}
                    transition:fade
+                    oncanplay={() => memoryViewerActor.send({ type: 'ASSET_READY', isVideo, videoElement })}
+                    ontimeupdate={() =>


This is a bit of a change unrelated to xstate. I'm now using the video player to track progress, end of playback etc. Previously it was done in the Tween. Given this element already has an idea of tracking where playback is at, we might as well use it.

The Tween is now only used for non-video playback.

knechtandreas · 2025-03-17T22:39:57Z

web/src/lib/components/memory-page/memory-viewer.svelte

        bind:this={memoryGallery}
      >
-        <GalleryViewer
-          onNext={handleNextAsset}


Now I just navigate to the asset viewed in the GalleryViewer when it's closed. Tracking onNext/onPrevious wasn't really necessary.

knechtandreas · 2025-03-17T23:04:55Z

web/src/lib/components/memory-page/memory-viewer-state-machine.ts

+        enqueue.assign({ galleryAndViewerClosed: event.galleryAndViewerClosed });
+        if (event.galleryAndViewerClosed) {
+          enqueue.raise({ type: 'PLAY' });
+        }


Note when the galleryAndViewerClosed is false, I could call PAUSE here, but it's already handled as well by the TIMING event guard below, which will go to the paused state if the gallery or viewer is open.

knechtandreas · 2025-03-17T23:06:54Z

web/src/lib/components/memory-page/memory-viewer-state-machine.ts

+  isVideo: boolean;
+  galleryAndViewerClosed: boolean;
+  currentMemoryAsset: MemoryAsset | undefined;
+  videoElement: HTMLVideoElement | undefined;


I was in 2 minds about whether I should let some of the UI elements bleed into the statemachine (i.e. the videoElement and Tween). I think it's fine as it is, but makes it a little bit harder to write unit tests perhaps.

This could also be done with events. Where the state machine simply emits events and the UI code listens for those and updates the Tween/videoElement outside of here.

knechtandreas · 2025-03-17T23:11:21Z

web/src/lib/components/memory-page/memory-viewer.svelte

  };

+  const navigateToAsset = (assetId: string | undefined) => {
+    if (current?.asset.id === assetId) {


Whoops - this should probably just be a guard in the statemachine.

* main: (37 commits) fix(web): reset selection state when adding assets to a album (immich-app#16880) fix(deps): update machine-learning (immich-app#16966) chore(deps): update actions/download-artifact digest to b14cf4c (immich-app#16934) fix(web): date alignment on timeline (immich-app#16961) fix(deps): update machine-learning (immich-app#16960) chore(deps): update prom/prometheus docker digest to 502ad90 (immich-app#16956) fix: duplicated steps in docker workflow (immich-app#16952) fix(deps): update typescript-projects (immich-app#16945) chore(deps): update dependency types-setuptools to v76 (immich-app#16949) fix(deps): update machine-learning (immich-app#16935) chore: run docs and cli builds on all PRs (immich-app#16954) fix(server): /api/stacks does not handles primaryAssetId query param (immich-app#16868) chore(docs): clarify missing ':ro' tag in volume mount as a warning (immich-app#16877) fix(server): set pixel format when scaling and not tonemapping (immich-app#16932) fix(web): asset selection on memories page is broken (immich-app#16759) chore(deps): update base-image to v20250318 (major) (immich-app#16950) refactor(mobile): remove int user id (immich-app#16814) feat: timeline performance (immich-app#16446) refactor(mobile): use user service methods (immich-app#16783) chore(deps): update dependency @types/node to ^22.13.10 (immich-app#16944) ...

…emory.

…disabled when gallery isn't shown in memory.

knechtandreas · 2025-03-19T07:05:02Z

web/src/lib/components/memory-page/memory-viewer.svelte

        <GalleryViewer
-          onNext={handleNextAsset}
-          onPrevious={handlePreviousAsset}
+          disableAssetSelect={!galleryInView}


Was getting some flickering behaviour because using arrow keys in the memory would also cause the focus to shift in the gallery below. This now disables the arrow keys when the gallery isn't in view at the bottom.

knechtandreas · 2025-03-19T07:06:02Z

web/src/lib/components/shared-components/gallery-viewer/gallery-viewer.svelte


+      if (!disableAssetSelect) {
+        shortcuts.push(
+          { shortcut: { key: 'ArrowRight' }, preventDefault: false, onShortcut: focusNextAsset },


Really no point focussing assets, when asset selection is disabled.

knechtandreas · 2025-04-03T21:20:02Z

Will re-submit this after some cleanups in immich proper.

knechtandreas and others added 30 commits March 9, 2025 16:58

16712: Proper intialisation of the memory store to avoid loading up d…

5fe5964

…uplicate object refs of the same asset.

16712: Add auth to memory mapping so isFavorite is actually return co…

bf99799

…rrectly from the server.

16712: Move logic that belongs in the store into the store.

25d6600

16712: Cleanup.

c4a7cec

16712: Fix init behaviour.

06cf89a

16712: Add comment.

7da26e1

16712: Make method private.

8e953b2

16712: Fix import.

e212773

16712: Fix format.

6cd8dd4

16712: Cleaner if/else and fix typo.

4581db8

fix: icon size mismatch

7f06454

16712: Add aria tag.

324bdd8

16712: Fix memory player duplicate invocation bugs. Now we should onl…

5d40824

…y call 'reset' and 'play' once, after navigate/page load. This should hopefully fix all the various bugs around playback.

16712: Cleanup

8917f2c

16712: Cleanup

4ee1a55

16712: Cleanup

5b5587a

16712: Cleanup

ca5f440

16712: Xstate initial.

4e5413e

16712: photoprogresscontroller is now separate

fca4490

16712: Extracted state machine into it's own file.

38f380a

16712: More progress.

8a7ae2e

16712: Fix up gallery & viewer open/closed.

9c5cf90

16712: Navigate to correct asset on viewer close.

b899187

16712: Better variable naming.

c1c9e1c

16712: Some small improvements.

620d038

16712: Actions and description

914e8f1

knechtandreas added 4 commits March 17, 2025 20:20

16712: Fix format.

90bb332

16712: Various cleanups.

6867441

16712: small fixes.

efe9af4

16712: Comment dev code.

18f5d0a

github-actions bot added the 🖥️web label Mar 17, 2025

knechtandreas commented Mar 17, 2025

View reviewed changes

knechtandreas mentioned this pull request Mar 17, 2025

fix(web): asset selection on memories page is broken immich-app/immich#16759

Merged

8 tasks

knechtandreas commented Mar 17, 2025

View reviewed changes

knechtandreas added 6 commits March 19, 2025 11:47

Fix merge issues.

9c2eec6

Move guard into state machine.

3e9a02c

Cleanup/PR feedback.

b3226a1

Only allow memory related shortcuts when we're actually viewing the m…

813810f

…emory.

Disable arrows in gallery if selection is disabled. Selection is now …

080642b

…disabled when gallery isn't shown in memory.

knechtandreas changed the base branch from fix/16712-asset-selection-on-memories-page-is-broken to main March 19, 2025 06:50

knechtandreas added 3 commits March 19, 2025 17:57

Formatting fix.

c5b068d

Remove logging code.

aa524c5

Revert.

a0afcd1

knechtandreas commented Mar 19, 2025

View reviewed changes

knechtandreas closed this Apr 3, 2025

Comments

Conversation

knechtandreas commented Mar 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

github-actions bot commented Mar 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

knechtandreas Mar 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

knechtandreas Mar 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

knechtandreas commented Apr 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

knechtandreas commented Mar 17, 2025 •

edited

Loading

github-actions bot commented Mar 17, 2025 •

edited

Loading

knechtandreas Mar 17, 2025 •

edited

Loading

knechtandreas Mar 17, 2025 •

edited

Loading