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

Copilot

🔵 Needs a closer look

It is a large, intricate multi-file refactor of the upload/record/hotkey flows whose deferred-upload and callback-timing behavior warrants final human verification, and it ships several leftover review-conversation comments that should be cleaned up first.

2 open issues

_{Review effort: Balanced}

+  | { kind: 'add'; action: AddUploadAction }
+  // Research (FindTabs) produced markdown / a Faithbridge link — seeds the
+  // upload step with that content.
+  // TODO test and rename this branch


…ype constants (#795) * Rename MediaUpload to MediaUploadDialog and share content-type constants Pure renames split out of the add-resource wizard refactor (#741), no behavior change: - MediaUpload.tsx -> MediaUploadDialog.tsx (component + importers) - UriLinkType/MarkDownType/Mp3Type/FaithbridgeType move to utils/contentType (drops the duplicate copies in resourceArtifactName and its test) - MediaUploadContent prop saveText -> saveButtonText - ResourceData prop allowProject -> allowGeneralResourceOption - pullPlanMedia getPlanId -> getRemotePlanId Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Point XLIFF sourcefile context at MediaUploadDialog.tsx Follow-up to the MediaUpload -> MediaUploadDialog rename: the 30 mediaUpload.* trans-units still named MediaUpload.tsx as their source file for translators (flagged by Devin review). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* refactor: add hotkeys prop to WSAudioPlayer and MediaRecord Split out of the add-resource wizard refactor (#741). WSAudioPlayer gains a `hotkeys` prop (default true) that enables/disables all of a player's keyboard shortcuts together, so an off-screen player can yield them to the visible one. MediaRecord forwards it. Default true keeps every existing screen unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * refactor: keep passive players from touching shared hotkeys Address Devin review on #796: - Forward `hotkeys` to WSAudioPlayerRate/Zoom/Segment so a passive player doesn't claim the speed, zoom, or segment shortcuts. - Remove the mount-only unsubscribe list from the device-listener cleanup. It predates TT-7283, which gave each hotkey effect its own cleanup, so it was redundant for active players — and for a passive player it removed keys owned by the active one (HotKeyContext tracks keys, not owners). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * feat: let the source player own hotkeys in Careful Speech and PBT PassageDetailGuidedPhraseRecord (Careful Speech and Phrase Back Translate) mounts the source player above the CarefulSpeechControls recorder. Both subscribed the shared transport keys, and the recorder (mounted second) won. Pass hotkeys={!allowSourcePlayer} through CarefulSpeechControls so the top source player keeps them. hotkeys is all-or-nothing, so the recorder also drops F9 and Ctrl+C/X/V while the source player is shown; TT-7748 tracks making the prop specify which hotkeys a player claims. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Revert Careful Speech hotkeys change; add TT-7748 TODO Undo 28fc500 (the recorder in Careful Speech / PBT giving up all its hotkeys to the source player) — not wanted as an all-off change. Instead note on WSAudioPlayer's hotkeys prop that TT-7748 will let a player specify which hotkeys it claims. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

nabalone · 2026-11-12T19:21:23Z

Superseded by #853 and friends

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 Resolved

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 force-pushed the add-resource-wizard-refactor branch from 5466b3e to 2118101 Compare October 9, 2026 13:51

nabalone added 4 commits October 12, 2026 16:45

reviewed

a133115

before uploader refactor

92923c0

wip

cb6e116

useAddResourceWizard hook

aeca4ec

nabalone force-pushed the add-resource-wizard-refactor branch from 2118101 to aeca4ec Compare October 19, 2026 13:50

nabalone added 2 commits October 19, 2026 21:04

more

f8aa19e

more more

b1027fe

nabalone requested a balanced review from Copilot October 20, 2026 13:30

Copilot started reviewing on behalf of nabalone October 20, 2026 13:31

View session

Copilot AI reviewed Oct 20, 2026

View reviewed changes

Comment thread web/src/components/PassageDetail/Internalization/useAddResourceWizard.ts Outdated

| { kind: 'add'; action: AddUploadAction }

// Research (FindTabs) produced markdown / a Faithbridge link — seeds the

// upload step with that content.

// TODO test and rename this branch

more more more

159c5c9

This was referenced Oct 29, 2026

refactor: rename MediaUpload to MediaUploadDialog and share content-type constants #795

Merged

refactor: add hotkeys prop to WSAudioPlayer and MediaRecord #796

Merged

nabalone mentioned this pull request Oct 30, 2026

refactor: merge Uploader's upload pipeline into useMediaUpload #801

Merged

2 of 3 tasks

nabalone closed this Nov 12, 2026

nabalone deleted the add-resource-wizard-refactor branch November 12, 2026 19:22

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 7 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!

Copilot AI left a comment

Uh oh!

nabalone commented Nov 12, 2026

Uh oh!

Reviewers

Uh oh!

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!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

nabalone commented Nov 12, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

nabalone commented Oct 7, 2026 •

edited

Loading