From a9685f64c461e9492a39b423326a12bb18ef6222 Mon Sep 17 00:00:00 2001
From: dtoro <d.toro.v@pm.me>
Date: Sun, 5 Jul 2026 10:12:56 +0200
Subject: [PATCH] fix(people): keep "name new faces" reachable after naming the
 first one
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Landing on /tags/people with no value in the URL auto-selects the
first named person (so there's always something to look at) — but
that same effect made the naming workflow unreachable the moment a
second person existed to redirect into: NewFacesPanel only rendered
in the "!selectedValue" branch, and there was no way back to a null
selection once one existed.

Added a pinned "Name new faces" row in the People sidebar (with a live
unnamed-cluster count) that sets a `?view=new-faces` query param
instead of clearing the `[[value]]` route param — deliberately
independent of the value-drives-selection model so it can't be
overwritten. The auto-select-first-tag effect also needed an explicit
guard for it: navigating to a bare /tags/people URL still clears
selectedValue, which re-triggers that same effect in the same tick and
would otherwise bounce straight back to the first person before the
panel ever rendered.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
---
 .../sidebar/TagsBrowserSidebar.svelte         | 69 ++++++++++++++++++-
 .../tags/[category]/[[value]]/+page.svelte    | 53 ++++++++------
 2 files changed, 99 insertions(+), 23 deletions(-)

diff --git a/web/src/lib/components/sidebar/TagsBrowserSidebar.svelte b/web/src/lib/components/sidebar/TagsBrowserSidebar.svelte
index f6b647e..48ef3e8 100644
--- a/web/src/lib/components/sidebar/TagsBrowserSidebar.svelte
+++ b/web/src/lib/components/sidebar/TagsBrowserSidebar.svelte
@@ -1,4 +1,6 @@
 <script lang="ts">
+	import { page } from '$app/state';
+	import { goto } from '$app/navigation';
 	import { createQuery } from '@tanstack/svelte-query';
 	import {
 		aggregateKeywords,
@@ -7,11 +9,13 @@
 		listLabels,
 		listPhotosByUids,
 		listSubjects,
+		listUnnamedFaces,
 		type AggregatedKeyword,
 		type PhotoMarksMap,
 		type PpCountry,
 		type PpLabel,
-		type PpSubject
+		type PpSubject,
+		type UnnamedFaceCluster
 	} from '$lib/services/photoprism';
 	import { isAuthenticated, thumbUrl } from '$lib/stores/session.svelte';
 	import { nearBottom } from '$lib/actions/nearBottom';
@@ -25,7 +29,7 @@
 	import { countryFlag, countryName } from '$lib/utils/countries';
 	import type { PpPhoto } from '$lib/types/photoprism';
 	import { EmptyState, InlineLoader } from '$lib/components/feedback';
-	import { Globe, Hash, Tag, User } from 'lucide-svelte';
+	import { Globe, Hash, Tag, User, UserPlus } from 'lucide-svelte';
 
 	interface Props {
 		category: TagCategory;
@@ -34,6 +38,29 @@
 	}
 	const { category, selectedValue, onSelect }: Props = $props();
 
+	// "Name new faces" is a pinned row, not a subject — it needs to stay
+	// reachable even after every detected face has been named once (there's
+	// always another to catch as the library grows), so it lives outside
+	// the value-drives-URL selection model the rest of this sidebar uses.
+	// Query key matches NewFacesPanel's so the two share one cache entry.
+	const unnamedFacesQuery = createQuery<UnnamedFaceCluster[]>(() => ({
+		queryKey: ['faces', 'unnamed'],
+		queryFn: listUnnamedFaces,
+		enabled: isAuthenticated() && category === 'people',
+		staleTime: 60_000
+	}));
+	const unnamedFacesCount = $derived(unnamedFacesQuery.data?.length ?? 0);
+	const newFacesActive = $derived(page.url.searchParams.get('view') === 'new-faces');
+	function showNewFaces() {
+		// Deliberately NOT onSelect/navigateToTag — that drives the
+		// `[[value]]` route param, and the auto-select-first-tag effect
+		// below immediately overwrites a null value with the first real
+		// person, which is exactly the trap this row exists to escape.
+		// The `view` query param is independent state the page reads to
+		// show the naming panel instead of (or alongside) the photo grid.
+		void goto('/tags/people?view=new-faces', { keepFocus: true, noScroll: true });
+	}
+
 	let filterText = $state('');
 
 	// Reset the inline filter input whenever the user switches categories so
@@ -243,8 +270,16 @@
 	// fires when there's genuinely no selection — once a value is picked
 	// (by the user or by this effect), the URL drives selectedValue and
 	// the effect no-ops.
+	//
+	// `newFacesActive` additionally suppresses it for People: navigating
+	// to the pinned "Name new faces" row necessarily clears selectedValue
+	// (it targets a bare `/tags/people` URL), and without this guard this
+	// effect would immediately redirect straight back to the first named
+	// person in the same tick — permanently hiding the naming workflow
+	// again the moment a second person exists to auto-select into.
 	$effect(() => {
 		if (selectedValue != null) return;
+		if (newFacesActive) return;
 		if (firstValue == null) return;
 		onSelect(firstValue, { replace: true });
 	});
@@ -416,6 +451,36 @@
 			</div>
 		{/if}
 	{:else if category === 'people'}
+		<!-- Pinned above the named-people list (and shown regardless of its
+		     loading/empty/error state) so naming stays reachable even after
+		     every currently-detected face has a name — new faces keep
+		     appearing as the library grows. -->
+		<button
+			type="button"
+			class="flex h-8 w-full shrink-0 items-center gap-2 border-b border-border px-3 text-left text-[12px] leading-tight hover:bg-accent"
+			class:bg-primary={newFacesActive}
+			class:text-primary-foreground={newFacesActive}
+			class:hover:bg-primary={newFacesActive}
+			onclick={showNewFaces}
+		>
+			<span
+				class="flex h-5 w-5 shrink-0 items-center justify-center rounded-full {newFacesActive
+					? 'bg-primary-foreground/15'
+					: 'bg-secondary'}"
+			>
+				<UserPlus class="h-3 w-3 {newFacesActive ? '' : 'text-muted-foreground'}" />
+			</span>
+			<span class="min-w-0 flex-1 truncate font-medium">Name new faces</span>
+			{#if unnamedFacesCount > 0}
+				<span
+					class="flex h-4 min-w-[20px] shrink-0 items-center justify-center rounded px-1 text-[10px] tabular-nums {newFacesActive
+						? 'bg-primary-foreground/15 text-primary-foreground'
+						: 'bg-secondary text-muted-foreground'}"
+				>
+					{unnamedFacesCount}
+				</span>
+			{/if}
+		</button>
 		{#if subjectsQuery.isPending}
 			<InlineLoader size="sm" label="Loading people…" />
 		{:else if subjectsQuery.isError}
diff --git a/web/src/routes/tags/[category]/[[value]]/+page.svelte b/web/src/routes/tags/[category]/[[value]]/+page.svelte
index c8dc12f..ed70786 100644
--- a/web/src/routes/tags/[category]/[[value]]/+page.svelte
+++ b/web/src/routes/tags/[category]/[[value]]/+page.svelte
@@ -53,6 +53,16 @@
 			: null
 	);
 
+	// Independent of selectedValue on purpose: the People sidebar's pinned
+	// "Name new faces" row sets this query param instead of the `[[value]]`
+	// route param, specifically so it can't be clobbered by the auto-
+	// select-first-tag effect (TagsBrowserSidebar) that fires whenever
+	// selectedValue is null — that effect is exactly what made the naming
+	// panel unreachable again after the first person was named.
+	const showNewFaces = $derived(
+		category === 'people' && page.url.searchParams.get('view') === 'new-faces'
+	);
+
 	// Mirror URL into the shared filter store so any other consumer of
 	// `filters` (e.g. cross-route navigation back to `/`) sees the active
 	// tag filter, and so `filtersToQ()` produces the correct DSL clause
@@ -208,7 +218,7 @@
 	{#if category}
 		<span class="text-[11px] capitalize text-muted-foreground">{category}</span>
 	{/if}
-	{#if selectedValue}
+	{#if selectedValue && !showNewFaces}
 		<span class="text-[11px] font-medium">{drillTitle}</span>
 		<span class="text-[11px] text-muted-foreground">
 			{drillCount} photo{drillCount === 1 ? '' : 's'}
@@ -216,29 +226,30 @@
 	{/if}
 </Toolbar>
 
-{#if !selectedValue}
-	{#if category === 'people'}
-		<!-- No person selected: surface the naming workflow instead of a
-		     bare prompt — naming is what creates people in the first place. -->
-		<main class="min-h-0 flex-1 overflow-y-auto p-6">
-			<NewFacesPanel />
-			<div class="mt-8 flex items-center justify-center">
-				<EmptyState
-					icon={Tag}
-					title="Pick a person from the sidebar"
-					description="Click a row in the panel on the left to see that person's photos."
-				/>
-			</div>
-		</main>
-	{:else}
-		<main class="flex min-h-0 flex-1 items-center justify-center p-8">
+{#if category === 'people' && (showNewFaces || !selectedValue)}
+	<!-- Naming workflow: reachable both on bare landing (no person picked
+	     yet) and via the sidebar's pinned "Name new faces" row at any time
+	     — the latter is what makes it possible to get back here after the
+	     first person's been named, once the auto-select-first-tag effect
+	     would otherwise always jump straight to an existing person. -->
+	<main class="min-h-0 flex-1 overflow-y-auto p-6">
+		<NewFacesPanel />
+		<div class="mt-8 flex items-center justify-center">
 			<EmptyState
 				icon={Tag}
-				title={`Pick a ${category ?? 'tag'} from the sidebar`}
-				description="Click a row in the panel on the left to filter the photo grid by that tag."
+				title="Pick a person from the sidebar"
+				description="Click a row in the panel on the left to see that person's photos."
 			/>
-		</main>
-	{/if}
+		</div>
+	</main>
+{:else if !selectedValue}
+	<main class="flex min-h-0 flex-1 items-center justify-center p-8">
+		<EmptyState
+			icon={Tag}
+			title={`Pick a ${category ?? 'tag'} from the sidebar`}
+			description="Click a row in the panel on the left to filter the photo grid by that tag."
+		/>
+	</main>
 {:else}
 	<div class="flex min-h-0 flex-1">
 		<div class="flex min-w-0 flex-1 flex-col">