Restructure Train by Voice composer into a 3-step accordion - #661
Restructure Train by Voice composer into a 3-step accordion#661shreeraman96 wants to merge 7 commits into
Conversation
|
The PR Policy check is blocking this PR because required template information is missing. Please update the PR description with:
Visual files detected:
Screenshots or video are required for UI, UX, settings, onboarding, overlay, menu bar, or visual behavior changes. If this PR has no visual changes, check the no-visual-change box in the template. If this remains incomplete for 48 hours after opening, the PR may be closed. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 903a026ba3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| private var trainingVerifyStepBody: some View { | ||
| VStack(alignment: .leading, spacing: self.theme.metrics.spacing.sm) { | ||
| self.trainingFinalOutputPanel | ||
|
|
||
| Button { | ||
| Task { await self.addTrainedReplacement() } |
There was a problem hiding this comment.
When advanced voice matching is enabled and addTrainedReplacement() fails to save the pronunciation profile, it sets trainingHasError and trainingStatusMessage while leaving the accordion on Verify. Because the footer that renders those errors is now only included in trainingRecordStepBody, this Verify body shows no failure feedback after the Add Replacement attempt fails, unlike the previous always-visible footer. Include the error/footer in this step as well so the user can see and recover from save failures.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in d7381c4, though not by duplicating the footer into the Verify body.
The error label is hoisted out of the collapsible step bodies and rendered once in the accordion container (CustomDictionaryView.swift:2303-2310), directly after the three headers:
// Always-visible error label: rendered once outside the collapsible step
// bodies so a recording/processing failure is surfaced regardless of
// which step is expanded.
if self.trainingHasError {
Label(self.trainingStatusMessage, systemImage: "exclamationmark.triangle.fill")
.font(self.theme.typography.caption)
.foregroundStyle(self.theme.palette.warning)
}So a failed addTrainedReplacement() with advanced voice matching enabled surfaces trainingStatusMessage while the accordion sits on Verify, and the same holds for record/processing failures on any expanded step — one rendering site instead of per-step copies.
Greptile SummaryThis PR replaces the flat "Train by Voice" composer with a guided 3-step accordion (Word → Record → Verify & Save) and fixes the root cause of #648, where the Start button was permanently inactive and no interactive prompts were shown. Step-derivation and readiness predicates are extracted into a new SwiftUI-free
|
FluidVoice PR build readyDownload FluidVoice-PR-661-edf8f591b084 The artifact contains the ad-hoc-signed app ZIP, Xcode archive, build manifest, and installation instructions. It expires 5 days after the build. Install the app
This build has its own app identity, so its permissions are separate from the release version of FluidVoice. |
6cac7d6 to
a99cfbd
Compare
|
This pull request has been marked stale because it has had no activity for 5 days. It will be closed in 2 days if there is no further activity. Add the |
a99cfbd to
b9ddb2e
Compare
grohith327
left a comment
There was a problem hiding this comment.
Had a quick glance, added few comments. A few more requests for changes:
When someone types a word, it shows up in many places, not needed since they know what they are typing
The steps in record tab are too detailed, I would recommend shrinking it a bit to be specific
These are probably not entirely related to your changes but would recommend doing the clean up in one go
Splits the flat replacement composer into Word -> Record -> Verify & Save steps with an extracted, unit-tested step-derivation model. - Extract pure step logic into DictionaryTrainingStepModel (SwiftUI-free) - Render composer as a 3-step accordion with per-step status glyphs - Reset hasReachedVerify latch on every teardown path (Try Again, capture removal, mode toggle) so Verify never dead-ends with Save disabled - Reset lastAnnouncedTrainingStep and announce any forward step edge (including word->verify) for correct VoiceOver cues - Make Record/Verify headers non-interactive while the word is empty - Move trainingReplacement onChange to the always-mounted accordion so programmatic word writes still reset progress - Add unit tests for step derivation, readiness predicates, and edges
- Reset lastAnnouncedTrainingStep on all teardown paths so the step-3 VoiceOver cue fires again on retry (resetTrainingVerificationAttempts, removeTrainingVariant) - Show save-failure errors in the Verify step body (footer previously lived only in the Record body) - Drop the unused trainingVariantsIsEmpty param from isOutputCovered - Introduce DictionaryTrainingSnapshot to collapse the step-model param packs (fixes function_parameter_count) and move accordion helpers into an extension (fixes type_body_length); no behavior change
- Show real progress in Record subtitle when Verify is latched after a miss - Render heard-variant chips and already-correct caption in the Verify body - Move training error label to an always-visible spot below the accordion - Distinguish 'No replacement needed' from 'Ready to save' in Verify header - Lower VoiceOver announcement latch on backward step transitions - Defer word-field focus until the step body is mounted - Dedupe coverage and recording-lock predicates; extract resetTrainingStepLatches() - Pin latch-state subtitle copy in tests; use production readyCoveredCount
Typing a word made the "Verify & save" header tappable with zero
recordings, stranding the user on an empty final-output panel with
Add Replacement disabled.
- Move the header tap predicate into DictionaryTrainingStepModel.isStepInteractive
and gate .verify on derived == .verify (the latch keeps Try Again reachable
after a post-ready miss).
- Drop per-step subtitles; step titles now carry the instruction
("Type the word to teach" / "Say it 3 times" / "Verify & save"), so
DictionaryTrainingStepCopy is gone.
- Stop echoing the typed word back in readiness and instruction copy.
- Replace the subtitle tests with isStepInteractive coverage.
9810979 to
32aa246
Compare
Collapse multi-line comment blocks in the accordion to single lines and drop the ones narrating the change rather than the code.
Drops the file banner the type below already states, three near-identical "single source of truth" delegation notes, and test comments that restated their own test name. Compresses the rest to one line each, keeping the non-obvious contracts: the empty-word guard outranking the verify latch, why a post-ready miss must not snap back, and the readiness-progress mirror. Model 19 comment lines to 8, tests 38 to 20.
Description
Fixes #648, where Train by Voice / Teach Pronunciation under Custom Dictionary showed no interactive prompts and left the Start button permanently inactive. This replaces the flat replacement composer with a guided 3-step accordion (① Word → ② Record → ③ Verify & Save) that always surfaces the next action, and hardens the underlying step state machine.
Changes:
DictionaryTrainingStepModel(DictionaryTrainingSnapshotgroups the training state)hasReachedVerifylatch on every teardown path (Try Again, capture removal, mode toggle) so Verify never dead-ends with Save disabledlastAnnouncedTrainingStepon teardown and announce any forward step edge (incl.word → verify) for correct VoiceOver cuesVerify & savelocked until the derived step actually reaches verify so it can't be opened with nothing recorded and Add Replacement disabled; surface save-failure errors in the Verify stepType the word to teach/Say it 3 times/Verify & save), per-step subtitles and the word-echoing readiness strings are gonetrainingReplacementonChangeonto the always-mounted accordion so programmatic word writes still reset progressType of Change
Related Issue or Discussion
Closes #648
Testing
swiftlint --strict --config .swiftlint.yml Sources— 0 violationsswiftformat --config .swiftformat— changed files passDictionaryTrainingStepModelTests— 25/25 passingScreenshots / Video
Step 1 : Enter the word to teach
Step 2 : Record
Step 3 : Verify & save
Manual Add
Research preview
Before all shown at once
Notes