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
This commit is contained in:
2026-09-07 21:59:29 +02:00
parent 1d658f87dc
commit 50d1b373d3
5 changed files with 33 additions and 18 deletions

View File

@@ -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) => {

View File

@@ -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',

View File

@@ -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 };

View File

@@ -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 };
}

View File

@@ -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]);