Adopt the unused useAsyncState hook in runAutoDigitize / ExportDialog / upload flow — hook exists with 8 tests but is orphaned #135

New issue

Closed

opened 2026-10-03 03:58:51 +00:00 by Mark · 1 comment

Mark commented

2026-10-03 03:58:51 +00:00

Owner

Finding

src/hooks/useAsyncState.ts is a generic async-state hook ({ data, status, error, refetch } shape) with 8 test cases. It has zero production callers. Meanwhile, useWorkspace.ts and the digitize flow do manual state-machine work for loading/success/error states.

The hook exists to encapsulate a pattern. Adoption is the fix.

Where

Shipped hook: src/hooks/useAsyncState.ts (40 lines, generic <T> async state)
Test coverage: src/hooks/useAsyncState.test.ts (8 tests, ~190 lines)
Manual state-machine consumers: useWorkspace.ts (the digitize and export flows), ExportDialog.tsx (the export button's loading state), CanvasArea.tsx (the upload form's loading state)

Suggested fix shape

Adopt useAsyncState in 2-3 places where the manual state machine is most painful:

useWorkspace.ts runAutoDigitize — currently tracks processingStep and processingMessage in the reducer. The status enum (idle | processing | complete | error) maps cleanly to useAsyncState's status. The hook would own the status; the reducer would own the resulting data.
ExportDialog.tsx export button — currently has a local useState for loading | success | error plus toast emission. Adopt useAsyncState to drive the button's disabled state and the success/error toasts.
CanvasArea.tsx upload form — has a local uploading state. Smaller change but same pattern.

Each adoption:

Replace the local loading/error state with useAsyncState
Use the returned status to drive disabled state on the relevant button
Use the returned error to drive error toasts
Use the returned data for success paths (the digitize result, the export blob)

Severity

Low. The hook is small and self-contained. Adoption is a code-quality improvement, not a feature gap. The benefit is a single, tested async state shape used consistently across the app.

Discovered by

Gap analysis (2026-10-02) of unreached components.

src/hooks/useAsyncState.test.ts — full test coverage of the hook. Adoption is safe; if the existing tests pass, the hook works.

src/components/workspace/SettingsModal.tsx:113-208 — the Settings modal has a full thread-library CRUD form (add name + hex, list with remove buttons, reset to defaults). Above it, the modal renders <ThreadLibrarySection> which opens a separate <ThreadLibraryView> modal that's read-only.

The user has two ways to interact with the thread library:

Inside the Settings modal: full CRUD form, can add/remove/reset
Separate "Manage threads…" modal: read-only list of colors

The two surfaces overlap. The "Manage threads…" view shows the same data the CRUD form is already listing. The CRUD form is the more powerful surface; the "Manage threads…" view doesn't add anything.

Suggested fix shape: Decide which surface is the primary one and remove the duplicate. The CRUD form inside the Settings modal is more functional; the "Manage threads…" view should either be removed (its button deleted from ThreadLibrarySection) or upgraded to be a quick-glance view that shows colors with a "click to edit" affordance, but the current read-only state isn't useful when the editable list is right there.

Pick one and consolidate. Do not delete the unused surface without first deciding which is the canonical UX — that's a product decision, but the user should make it, not have two slightly different ways to do the same thing.

## Finding `src/hooks/useAsyncState.ts` is a generic async-state hook (`{ data, status, error, refetch }` shape) with 8 test cases. **It has zero production callers.** Meanwhile, `useWorkspace.ts` and the digitize flow do manual state-machine work for loading/success/error states. The hook exists to encapsulate a pattern. Adoption is the fix. ## Where - Shipped hook: `src/hooks/useAsyncState.ts` (40 lines, generic `<T>` async state) - Test coverage: `src/hooks/useAsyncState.test.ts` (8 tests, ~190 lines) - Manual state-machine consumers: `useWorkspace.ts` (the digitize and export flows), `ExportDialog.tsx` (the export button's loading state), `CanvasArea.tsx` (the upload form's loading state) ## Suggested fix shape Adopt `useAsyncState` in 2-3 places where the manual state machine is most painful: 1. **`useWorkspace.ts` `runAutoDigitize`** — currently tracks `processingStep` and `processingMessage` in the reducer. The status enum (`idle | processing | complete | error`) maps cleanly to `useAsyncState`'s `status`. The hook would own the status; the reducer would own the resulting data. 2. **`ExportDialog.tsx` export button** — currently has a local `useState` for `loading | success | error` plus toast emission. Adopt `useAsyncState` to drive the button's disabled state and the success/error toasts. 3. **`CanvasArea.tsx` upload form** — has a local `uploading` state. Smaller change but same pattern. Each adoption: - Replace the local loading/error state with `useAsyncState` - Use the returned `status` to drive disabled state on the relevant button - Use the returned `error` to drive error toasts - Use the returned `data` for success paths (the digitize result, the export blob) ## Severity Low. The hook is small and self-contained. Adoption is a code-quality improvement, not a feature gap. The benefit is a single, tested async state shape used consistently across the app. ## Discovered by Gap analysis (2026-10-02) of unreached components. ## Related - `src/hooks/useAsyncState.test.ts` — full test coverage of the hook. Adoption is safe; if the existing tests pass, the hook works. --- ## Bundled: Settings modal has the thread library CRUD surface in two places `src/components/workspace/SettingsModal.tsx:113-208` — the Settings modal has a full thread-library CRUD form (add name + hex, list with remove buttons, reset to defaults). Above it, the modal renders `<ThreadLibrarySection>` which opens a separate `<ThreadLibraryView>` modal that's read-only. The user has two ways to interact with the thread library: - **Inside the Settings modal**: full CRUD form, can add/remove/reset - **Separate "Manage threads…" modal**: read-only list of colors The two surfaces overlap. The "Manage threads…" view shows the same data the CRUD form is already listing. The CRUD form is the more powerful surface; the "Manage threads…" view doesn't add anything. **Suggested fix shape:** Decide which surface is the primary one and remove the duplicate. The CRUD form inside the Settings modal is more functional; the "Manage threads…" view should either be removed (its button deleted from `ThreadLibrarySection`) or upgraded to be a quick-glance view that shows colors with a "click to edit" affordance, but the current read-only state isn't useful when the editable list is right there. Pick one and consolidate. Do not delete the unused surface without first deciding which is the canonical UX — that's a product decision, but the user should make it, not have two slightly different ways to do the same thing.

Mark added the

feature-request

triage-approved

labels

2026-10-03 03:58:53 +00:00

Mark referenced this issue

2026-10-03 04:59:42 +00:00

Consolidate thread library surfaces — Settings modal has CRUD form AND a separate read-only ThreadLibraryView #147

Mark commented

2026-10-03 04:59:52 +00:00

Author

Owner

Closed via commit 5ffe353 on main. Adoption in ExportDialog.tsx (the cleanest fit; runAutoDigitize was skipped because the 4-step processingStep/processingMessage reducer doesn't map cleanly to the hook's 4-state enum — forcing it would lose the progress UI in CanvasArea.tsx:439-504. The CanvasArea suggestion in the body is stale; that upload state lives in useWorkspace.handleFile via appState, not as a local useState).

Side effects of the same commit:

Added getThreadCatalog to the ExportDialog.test.tsx mock (was missing since #126 added the service catalog fetch — masked every test in this file).
Added a waitFor around the clickSpy assertion in the download-success test (the download now fires from an effect, not synchronously in the click handler).
Dropped a dead makeStitchObject helper in issue-128.test.tsx (unused since the file split into two describe blocks; the only lint error on baseline main).

Bundled finding (thread-library CRUD duplication): filed as #147 with the same recommendation. Label it triage-approved if you want it auto-fixed, or close as not-planned if the read-only view is intentional UX.

Closed via commit `5ffe353` on `main`. Adoption in `ExportDialog.tsx` (the cleanest fit; `runAutoDigitize` was skipped because the 4-step `processingStep`/`processingMessage` reducer doesn't map cleanly to the hook's 4-state enum — forcing it would lose the progress UI in `CanvasArea.tsx:439-504`. The CanvasArea suggestion in the body is stale; that upload state lives in `useWorkspace.handleFile` via `appState`, not as a local `useState`). Side effects of the same commit: - Added `getThreadCatalog` to the `ExportDialog.test.tsx` mock (was missing since #126 added the service catalog fetch — masked every test in this file). - Added a `waitFor` around the `clickSpy` assertion in the download-success test (the download now fires from an effect, not synchronously in the click handler). - Dropped a dead `makeStitchObject` helper in `issue-128.test.tsx` (unused since the file split into two `describe` blocks; the only lint error on baseline main). Bundled finding (thread-library CRUD duplication): filed as **#147** with the same recommendation. Label it `triage-approved` if you want it auto-fixed, or close as `not-planned` if the read-only view is intentional UX.

Mark referenced this issue from a commit

2026-10-03 04:59:56 +00:00

fix(workspace): adopt useAsyncState in ExportDialog (#135)

Mark closed this issue

2026-10-03 05:00:02 +00:00