From 50d1b373d3cbc43899b462c09a874a50f5c11906 Mon Sep 17 00:00:00 2001
From: highperfocused <highperfocused@pm.me>
Date: Mon, 7 Sep 2026 21:59:29 +0200
Subject: [PATCH] Address remaining review feedback on app folders

- Cap reconcileFolderState() at MAX_FOLDERS and 500 membership entries so
  it always produces schema-valid state, matching loadFolderState's parse
  limits (previously a state that saved fine could reload as empty).
- Recognize folder tiles during desktop drag-over/drop hit-testing by also
  tagging them with data-home-icon-id and matching it in elementFromPoint
  lookups, so dragging an app onto a folder now highlights and files it.
- Make moveToFolder stable via a ref in the mobile pointer-listeners
  effect, and drop the now-unnecessary exhaustive-deps suppression.
- Let FolderVisual take a statusId prop instead of hardcoding
  'icon-layout-status', so mobile folder tiles point aria-describedby at
  the mobile live region instead of a nonexistent desktop one.
- Mint folder ids before calling setState instead of inside the updater,
  so createFolder's updater is pure (safe under strict-mode retries)
  while still returning the right id synchronously.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BYiUtZMQeA5RHggQw73wto
---
 src/components/os/Desktop.tsx        |  9 +++++----
 src/components/os/FolderIcon.tsx     |  5 ++++-
 src/components/os/MobileAppShell.tsx | 10 ++++++++--
 src/os/folders.ts                    | 11 ++++++++++-
 src/os/useFolders.ts                 | 16 ++++++----------
 5 files changed, 33 insertions(+), 18 deletions(-)

diff --git a/src/components/os/Desktop.tsx b/src/components/os/Desktop.tsx
index 2a8f7c0..bb48181 100644
--- a/src/components/os/Desktop.tsx
+++ b/src/components/os/Desktop.tsx
@@ -177,16 +177,16 @@ export function Desktop() {
     setCandidate(cellAt(event.clientX, event.clientY));
     // Dragging an app over a folder icon highlights it as a drop target.
     if (!active.id.startsWith(FOLDER_ID_PREFIX)) {
-      const hit = document.elementFromPoint(event.clientX, event.clientY)?.closest<HTMLElement>('[data-icon-id]');
-      const over = hit?.dataset.iconId;
+      const hit = document.elementFromPoint(event.clientX, event.clientY)?.closest<HTMLElement>('[data-icon-id], [data-home-icon-id]');
+      const over = hit?.dataset.iconId ?? hit?.dataset.homeIconId;
       setDropFolder(over?.startsWith(FOLDER_ID_PREFIX) && over !== active.id ? over : null);
     }
   }, [cellAt]);
   const finishPointer = useCallback((event: PointerEvent) => {
     const active = pointerStart.current;
     if (active?.moved) {
-      const hit = document.elementFromPoint(event.clientX, event.clientY)?.closest<HTMLElement>('[data-icon-id]');
-      const over = hit?.dataset.iconId;
+      const hit = document.elementFromPoint(event.clientX, event.clientY)?.closest<HTMLElement>('[data-icon-id], [data-home-icon-id]');
+      const over = hit?.dataset.iconId ?? hit?.dataset.homeIconId;
       if (!active.id.startsWith(FOLDER_ID_PREFIX) && over?.startsWith(FOLDER_ID_PREFIX) && over !== active.id) {
         moveToFolder(active.id, over.slice(FOLDER_ID_PREFIX.length));
       } else {
@@ -354,6 +354,7 @@ export function Desktop() {
                       dragging={dragging === iconId}
                       pickedUp={picked === iconId}
                       dropTarget={dropFolder === iconId}
+                      data-home-icon-id={iconId}
                       tabIndex={0}
                       style={{ position: 'absolute', left: SURFACE_PADDING + displaySlot.col * CELL_WIDTH, top: SURFACE_PADDING + displaySlot.row * CELL_HEIGHT, zIndex: dragging === iconId ? 2 : 1 }}
                       onPointerDown={(event) => {
diff --git a/src/components/os/FolderIcon.tsx b/src/components/os/FolderIcon.tsx
index 9595b72..3700038 100644
--- a/src/components/os/FolderIcon.tsx
+++ b/src/components/os/FolderIcon.tsx
@@ -25,6 +25,8 @@ export interface FolderVisualProps {
   className?: string;
   /** Hit-target id used by drag-and-drop on the home screen. */
   'data-home-icon-id'?: string;
+  /** id of the live region announcing pick-up/move status; differs between desktop and mobile. */
+  statusId?: string;
 }
 
 /**
@@ -48,6 +50,7 @@ export function FolderVisual({
   style,
   className,
   'data-home-icon-id': homeIconId,
+  statusId = 'icon-layout-status',
 }: FolderVisualProps) {
   return (
     <button
@@ -62,7 +65,7 @@ export function FolderVisual({
       onKeyDown={onKeyDown}
       aria-label={`${label} — folder with ${count} ${count === 1 ? 'app' : 'apps'}`}
       aria-pressed={pickedUp || undefined}
-      aria-describedby={pickedUp ? 'icon-layout-status' : undefined}
+      aria-describedby={pickedUp ? statusId : undefined}
       className={cn(
         'group flex w-20 flex-col items-center gap-1.5 rounded-lg p-2 text-center transition-[background-color,transform,box-shadow] motion-reduce:transition-none',
         'focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-ring',
diff --git a/src/components/os/MobileAppShell.tsx b/src/components/os/MobileAppShell.tsx
index 0408e50..938957b 100644
--- a/src/components/os/MobileAppShell.tsx
+++ b/src/components/os/MobileAppShell.tsx
@@ -237,6 +237,12 @@ function HomeScreen({ onOpen }: { onOpen: (id: string) => void }) {
     setAppFolder(appId, folderId);
     setAnnouncement(`${entryLabel(appId)} moved into folder ${folderTitle(folderId) ?? folderId}.`);
   };
+  // Kept current every render so the pointer-listeners effect below can call
+  // the latest labeling logic without re-arming its global listeners.
+  const moveToFolderRef = useRef(moveToFolder);
+  useEffect(() => {
+    moveToFolderRef.current = moveToFolder;
+  });
 
   const removeFromFolder = (appId: string) => {
     setAppFolder(appId, null);
@@ -291,7 +297,7 @@ function HomeScreen({ onOpen }: { onOpen: (id: string) => void }) {
         suppressClick.current = true;
         const over = targetRef.current;
         if (!active.id.startsWith(FOLDER_ID_PREFIX) && over?.startsWith(FOLDER_ID_PREFIX)) {
-          moveToFolder(active.id, over.slice(FOLDER_ID_PREFIX.length));
+          moveToFolderRef.current(active.id, over.slice(FOLDER_ID_PREFIX.length));
         } else {
           reorder(active.id, over && over !== active.id ? over : null);
         }
@@ -309,7 +315,6 @@ function HomeScreen({ onOpen }: { onOpen: (id: string) => void }) {
       window.removeEventListener('pointerup', onEnd);
       window.removeEventListener('pointercancel', onEnd);
     };
-    // eslint-disable-next-line react-hooks/exhaustive-deps -- moveToFolder is a plain function (the compiler refuses a useCallback here); listing it would re-arm the global listeners every render
   }, [reorder]);
 
   const onKeyDown = (id: string, event: React.KeyboardEvent<HTMLButtonElement>) => {
@@ -544,6 +549,7 @@ function HomeScreen({ onOpen }: { onOpen: (id: string) => void }) {
                   dragging={dragging === entry.id}
                   dropTarget={target === entry.id && dragging !== entry.id}
                   data-home-icon-id={entry.id}
+                  statusId="mobile-icon-layout-status"
                   onPointerDown={(event) => {
                     if (event.button !== 0) return;
                     dragStart.current = { id: entry.id, x: event.clientX, y: event.clientY, moved: false };
diff --git a/src/os/folders.ts b/src/os/folders.ts
index 37a7cbe..85e6b1c 100644
--- a/src/os/folders.ts
+++ b/src/os/folders.ts
@@ -51,11 +51,14 @@ export function folderNameTaken(folders: Folder[], name: string, excludeId?: str
   return folders.some((folder) => folder.id !== excludeId && folder.name.toLowerCase() === needle);
 }
 
+const MAX_MEMBERSHIP = 500;
+
 /** Drops assignments to folders/apps that no longer exist and dedupes folder ids. */
 export function reconcileFolderState(state: FolderState, appIds: string[]): FolderState {
   const seen = new Set<string>();
   const folders: Folder[] = [];
   for (const folder of state.folders) {
+    if (folders.length >= MAX_FOLDERS) break;
     if (seen.has(folder.id)) continue;
     const name = normalizeFolderName(folder.name);
     if (!name) continue;
@@ -66,6 +69,7 @@ export function reconcileFolderState(state: FolderState, appIds: string[]): Fold
   const knownApps = new Set(appIds);
   const membership: Record<string, string> = {};
   for (const [appId, folderId] of Object.entries(state.membership)) {
+    if (Object.keys(membership).length >= MAX_MEMBERSHIP) break;
     if (eligible.has(folderId) && knownApps.has(appId)) membership[appId] = folderId;
   }
   return { folders, membership };
@@ -79,7 +83,12 @@ export function createFolderId(): string {
 }
 
 export function addFolder(state: FolderState, name: string): { state: FolderState; folder: Folder } {
-  const folder = { id: createFolderId(), name };
+  return addFolderWithId(state, createFolderId(), name);
+}
+
+/** Pure variant of {@link addFolder} for callers that must mint the id up front. */
+export function addFolderWithId(state: FolderState, id: string, name: string): { state: FolderState; folder: Folder } {
+  const folder = { id, name };
   return { state: { ...state, folders: [...state.folders, folder] }, folder };
 }
 
diff --git a/src/os/useFolders.ts b/src/os/useFolders.ts
index c4bf365..ed7196a 100644
--- a/src/os/useFolders.ts
+++ b/src/os/useFolders.ts
@@ -2,8 +2,9 @@ import { useCallback, useEffect, useMemo, useRef, useState } from 'react';
 import { useCurrentUser } from '@/hooks/useCurrentUser';
 import {
   EMPTY_FOLDERS,
-  addFolder,
+  addFolderWithId,
   clearFolderState,
+  createFolderId,
   loadFolderState,
   reconcileFolderState,
   removeFolder,
@@ -62,15 +63,10 @@ export function useFolders(appIds: string[]) {
   }, [stableIds]);
 
   const createFolder = useCallback((name: string): string => {
-    // The updater runs during the setState re-render, so the id has to be
-    // minted inside it — capturing it beforehand could give callers an id
-    // that a concurrent update (or a React strict-mode retry) dropped.
-    let id = '';
-    update((current) => {
-      const added = addFolder(current, name);
-      id = added.folder.id;
-      return added.state;
-    });
+    // Mint the id up front so the updater stays pure (no side effects, safe
+    // to call more than once, e.g. under strict-mode double-invocation).
+    const id = createFolderId();
+    update((current) => addFolderWithId(current, id, name).state);
     return id;
   }, [update]);