Refactor Add Resource into a single-dialog wizard; scope audio-player hotkeys by nabalone · Pull Request #741 · sillsdev/apm

nabalone · 2026-10-07T03:18:10Z

Jira: https://jira.sil.org/browse/TT-7767

Reworks the "Add Audio Resource" / general-resource flow and fixes the multi-player hotkey handling it exposed.

What changed

Single-dialog wizard. Extracted AddResourceWizard from PassageDetailArtifacts; the upload/record step, Select Sections, and Configure now live in one dialog. The upload/record UI is a chrome-less PassageRecordPanel (split out of PassageRecordDlg) that Uploader renders in an embedded mode; Select Sections stays mounted (CSS-hidden) across Back, Configure mounts only on its step.
Back buttons for everyone. Removed the developer-mode gate on the wizard's Back buttons (SelectSections, ProjectResourceConfigure).
Warn on app close with unsaved work (TT-7767). The wizard registers a save-less close-guard tool with the Unsaved context whenever closing would lose work (Configure step, a staged recording on Select Sections, or an unsaved take on Upload), so a browser/app close raises the usual unsaved-changes warning in addition to the dialog's own X confirm. The configure save tool id is renamed to AddResourceWizard-Save to distinguish it from the close guard.
Scoped hotkeys (opt-out). WSAudioPlayer gains a hotkeys prop (default true) that enables/disables all of a player's shortcuts together; the wizard passes hotkeys={active} so an off-screen step doesn't grab Alt+Space/F9. HotKeyContext is unchanged (most-recent-wins); default-true keeps every existing screen identical.
Fix: stuck upload progress bar. A deferred stage left MediaUploadContent's progress true; since the embedded panel stays mounted across Back, the indeterminate bar spun forever and disabled Next. Progress is now cleared after a successful deferred stage.
Removes the now-unused projectResourceConfigure ("Chunk into Resources") string.

Test plan

npm run typecheck (web) clean.
Jest: wizard, PassageRecordDlg, Uploader, MediaRecord, WSAudioPlayer.hotkeys, ProjectResourceConfigure suites pass.
Manual: Add Audio Resource → record a take → Next → Back lands on the Record tab with the take intact; select a file → Select Sections → Back leaves Next enabled (no stuck bar); keyboard transport in guided-phrase record still controlled by the source player; closing the browser mid-configure (or with a staged take) raises the unsaved-changes warning.

🤖 Generated with Claude Code

Copilot

Copilot review overview

🔵 Needs a closer look

This is a large, high-risk refactor of stateful dialog/close/hotkey plumbing across several large components, and it already exposes a subtle stale-state defect (uploadHasTake not reset in closeAll), so a human should review the full flow before approval.

Review effort: Balanced
Findings: 1

Open (1)

Reset stale audio upload state on wizard close · New

What changed in this PR

This PR refactors the "Add Audio Resource" / general-resource flow in the passage-detail internalization UI. The previously inline, multi-dialog upload/record → select-sections → configure flow (duplicated inside PassageDetailArtifacts and its mobile twin) is extracted into a single reusable AddResourceWizard component driven by a launch request from the parent. As part of this, the record/upload body is split out of PassageRecordDlg into a chrome-less PassageRecordPanel that Uploader can render in a new embedded mode, and the audio player's keyboard shortcuts become opt-out (hotkeys prop) so an off-screen record step no longer steals Alt+Space/F9. It also fixes a stuck indeterminate progress bar on the embedded upload step and removes the now-unused projectResourceConfigure ("Chunk into Resources") string.

Changes:

Extract AddResourceWizard (single-dialog, step-state in one place) plus shared hooks (useResourceScopeLabels, useResourceArtifactTypes) and pure helpers (resourceScopeLabels), replacing the inline flow in both desktop and mobile artifact components.
Split PassageRecordPanel out of PassageRecordDlg; add Uploader embedded mode; scope WSAudioPlayer/MediaRecord hotkeys via a hotkeys prop (default true); clear progress after a deferred stage in MediaUploadContent.
Remove the developer-mode gate on wizard Back buttons and delete the unused projectResourceConfigure localization string across model/reducers/xlf/xliff/tests.

File	Description
`AddResourceWizard.tsx`	New single-dialog wizard owning the upload→select→configure flow.
`AddResourceWizard.test.tsx`	Tests for wizard steps, Back/Next, close/confirm behavior.
`PassageDetailArtifacts.tsx` / `PassageDetailsArtifactsMobile.tsx`	Replace inline flow with `AddResourceWizard`; trim now-unused state/imports.
`PassageDetailArtifacts.test.tsx`	Rework tests to assert launch request handed to the wizard.
`PassageRecordPanel.tsx`	Extracted record/upload panel (embeddable, host-owned chrome).
`PassageRecordDlg.tsx` / `.test.tsx`	Dialog frame now wraps the panel; `keepMounted`; persistence tests.
`Uploader.tsx` / `.test.tsx`	New `embedded` mode rendering panel/`MediaUploadContent` directly.
`WSAudioPlayer.tsx` / `MediaRecord.tsx`	Add opt-out `hotkeys` prop gating shortcut subscriptions.
`MediaUploadContent.tsx`	Clear `progress` after a deferred (staged) upload.
`useResourceScopeLabels.ts` / `resourceScopeLabels.ts(.test)` / `useResourceArtifactTypes.ts(.test)`	Shared label/type-id resolution used by edit dialog and wizard.
`SelectSections.tsx` / `ProjectResourceConfigure.tsx`	Remove developer-mode gate on Back button; drop dead empty-content branch.
`localization/model.tsx`, `reducers.tsx`, `*.xlf/.xliff`, `ProjectResourceConfigure.test.tsx`	Remove unused `projectResourceConfigure` string.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+    setAllowProject(true);
+    setAIGenerated(false);
+    setMarkdownValue('');
+    setIsStagedRecording(false);


Copilot

🔵 Needs a closer look

The mobile host omits the per-launch remount key that the wizard's teardown design depends on, leaking stale flow state across opens, and this large stateful refactor warrants human verification.

2 open findings

Mobile wizard reuses stale state across launches · New
Reset stale audio upload state on wizard close

🧠 Review effort: Balanced

Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

Copilot

🔵 Needs a closer look

It is a large, high-risk refactor (new ~800-line wizard state machine, parallel desktop/mobile changes, recorder/dialog lifecycle, and hotkey-subscription changes) whose runtime behavior cannot be fully verified by static review, so it warrants final human review.

1 open finding

Reset stale audio upload state on wizard close

1 resolved since last review

Mobile wizard reuses stale state across launches

🧠 Review effort: Balanced

Copilot

🔵 Needs a closer look

It is a large cross-cutting dialog/state-machine refactor (an 800+ line new wizard, embedded Uploader mode, close-guard and scoped-hotkey behavior) whose many interacting UI states can only be partially verified statically and warrant human review.

1 open finding

Reset stale audio upload state on wizard close

Previously missed (1)

In code that hasn't changed since last review

Broken doc comment grammar and missing comment punctuation

web/src/components/MediaUploadContent.tsx:186

This doc comment is grammatically broken: "the selection survives is still there if the user hits next then back" has two verbs ("survives"/"is still there"), and the closing */ is missing a space before it and a sentence-ending period. Consider rewording for clarity.

🧠 Review effort: Balanced

Checkpoint before the one-dialog panel refactor. - New AddResourceWizard owns the upload-based resource flow (upload step + conditional SelectSections + ProjectResourceConfigure); PassageDetailArtifacts keeps the list, simple edit, delete, and Scripture/Shared launchers and just hands the wizard a launch request. - SelectSections stays mounted (CSS-hidden) across Back from Configure; Configure unmounts off its step. - PassageRecordDlg: keepMounted so a recorded take survives the wizard's Next -> Back and reopens on the Record tab; drop to Upload on a genuine close to release the recorder. - Tests: AddResourceWizard (8), trimmed PassageDetailArtifacts (2), and new PassageRecordDlg persistence tests (2). Also includes in-progress localization regen and PassageDetailsArtifactsMobile / ProjectResourceConfigure tweaks already present in the tree. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

- Extract PassageRecordPanel from PassageRecordDlg (dialog now a thin frame). - Uploader gains an `embedded` mode: renders the panel / MediaUploadContent chrome-less so a host can own the dialog. - AddResourceWizard hosts step 1 (record/upload), step 2 (SelectSections, CSS-hidden sibling) and step 3 (Configure, conditional) in one BigDialog; per-upload-type title + unified close/confirm that blocks mid-recording. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Remove the developer-mode gate on the "Back" buttons in SelectSections and ProjectResourceConfigure so the add-resource wizard's Back is usable by all users. The buttons still only appear when a previous step exists (onBack wired). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

A deferred stage (general-resource flow) left MediaUploadContent's `progress` true, since handleAddOrSave only cleared it on failure. In the embedded wizard the upload panel stays mounted across Back, so the indeterminate LinearProgress spun forever and kept the Next button disabled (progress feeds saveDisabled). Staging starts no upload, so clear progress after a successful deferred stage. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…ploads A general-resource addition is a wizard step, not a durable task. Adding one is atomic: finish it in the wizard, or it's abandoned — there is no "resume the configure step later" path. - Delete useResumePendingProjectResourceConfig + the pendingProjectResourceConfig store, and stop restoreAfterPendingUpload from queuing a config resume. This retires the only reason the wizard had to stay mounted while closed. - Stop enrolling ProjectResource uploads in the crash-recovery pending queue (actions.tsx pre-PUT site): an interrupted general-resource upload is dropped rather than surfaced in Pending Uploads, where a retry could only produce an unconfigurable orphan. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…rdown list The parent bumps a `key` on every wizard launch, so the wizard remounts fresh each time it opens. closeAll no longer hand-clears ~18 pieces of state (which had already missed fields twice); it just hides the wizard (step=None), which unmounts the step children — releasing the recorder/player and clearing Configure's unsaved-changes flag. Fresh state on the next open is now structural. Enabled by the previous commit removing the config-resume: the wizard no longer needs to stay mounted while closed to listen for a pending-config restore. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Not worth special-casing: a general-resource upload interrupted by a crash mid-PUT is unlikely enough that letting it participate in the generic pending/ retry queue like every other upload is fine. A retried one just becomes an orphaned unconfigured resource, which is acceptable. The config-resume removal (prior commit) stands. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

With the config-resume gone, every project-resource upload reaches afterUpload with sectionsPreselectedRef already true (the deferred step-2 flow is the only way to upload one), so the non-preselected else-branch — and the projResSetup state and its two effects — were unreachable. Dropped them. openForMedia stays; it is still the editGeneral launch entry. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…work Register a close-guard tool with the Unsaved context whenever closing the add-resource wizard would lose work (Configure step, staged recording on Select Sections, or an unsaved take on Upload), so a browser/app close raises the usual unsaved-changes warning in addition to the dialog's own X confirm. Rename the configure save tool id to 'AddResourceWizard-Save' to distinguish it from the save-less close guard. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Project (general) resources are configured in one go through the add-resource wizard and are never staged as pending uploads, so the projectresource restore kind and its handling were dead. Drop it from the PendingUploadRestore union, restoreAfterPendingUpload, and buildResourcePendingRestore, and have the wizard return no restore metadata for project resources. Remove the now-obsolete tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

nabalone requested a balanced review from Copilot October 7, 2026 03:18

Copilot started reviewing on behalf of nabalone October 7, 2026 03:18 View session

Copilot AI reviewed Oct 7, 2026

View reviewed changes

Comment thread web/src/components/PassageDetail/Internalization/AddResourceWizard.tsx Outdated

setAllowProject(true);

setAIGenerated(false);

setMarkdownValue('');

setIsStagedRecording(false);

nabalone force-pushed the add-resource-wizard-refactor branch from f4c43f8 to b10d33c Compare October 8, 2026 15:41

nabalone requested a balanced review from Copilot October 8, 2026 15:42

Copilot started reviewing on behalf of nabalone October 8, 2026 15:43 View session

Copilot AI reviewed Oct 8, 2026

View reviewed changes

Comment thread web/src/components/PassageDetail/Internalization/PassageDetailsArtifactsMobile.tsx

nabalone requested a balanced review from Copilot October 8, 2026 17:39

Copilot started reviewing on behalf of nabalone October 8, 2026 17:39 View session

Copilot AI reviewed Oct 8, 2026

View reviewed changes

nabalone requested a balanced review from Copilot October 9, 2026 13:08

Copilot started reviewing on behalf of nabalone October 9, 2026 13:09 View session

Copilot AI reviewed Oct 9, 2026

View reviewed changes

nabalone and others added 17 commits October 9, 2026 09:39

reorg shared add/edit code

071ef45

make hotkeys work

246b5cc

ai code review

ddebb76

styling

8be00cb

fix record to configure and back

11274ef

release microphone

b1cc839

mobile

e43ecd5

wip, mostly looks good but more review needed

2118101

nabalone force-pushed the add-resource-wizard-refactor branch from 5466b3e to 2118101 Compare October 9, 2026 13:51

Navigation Menu

Repository navigation

Refactor Add Resource into a single-dialog wizard; scope audio-player hotkeys - #741

Refactor Add Resource into a single-dialog wizard; scope audio-player hotkeys#741
nabalone wants to merge 18 commits into
developfrom
add-resource-wizard-refactor

nabalone commented Oct 7, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

nabalone commented Oct 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changed

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

nabalone commented Oct 7, 2026 •

edited

Loading