Merge feature/voice-selection into main - #674
Merged
Merged
Conversation
- Updated ViewModels to manage voice selection and TTS settings. - Added new Speak intent for handling text-to-speech actions. - Improved SelectionModeScreen to display selected voice label and navigate to voice selection. - Refactored PresetsScreen to utilize the new Speak intent. - Introduced voice download handling in VoiceSelectionScreen.
…and improved pagination
Language Selection (ui/languageselection/) and the FuelIX/Crowdin machine-translation pipeline were tangled into feature/voice-selection alongside the actual voice-selection work. Per #613's resolved decision, both are out of scope here and need their own ticket/QA validation. - Delete ui/languageselection/ (Screen/ViewModel/State/Event) - Remove the FuelIX Gradle plugin, fuelixTranslations {} block, and languages.txt - Revert ~150 translation-file additions/modifications back to main's baseline (main already includes the later-merged PR #611/#612 keypad grid fix, so this restores forward, not backward) - Strip Language row/state/nav wiring from SelectionModeScreen/ViewModel, VocableNavHost, AppKoinModule, IVocableSharedPreferences/ VocableSharedPreferences, and test fakes; relink SelectionModeScreen's ConstraintLayout now that the Voice button anchors directly below Head Tracking - Fix PresetsViewModel's speak() call site, which read the now-removed language preference Also, unrelated to Language Selection but discovered while touching these same files: downgrade AGP 9.1.0 -> 8.13.1 to match the team's installed Android Studio, which silently dropped Kotlin compilation entirely since this project never explicitly applies the Kotlin Android plugin (AGP 9 wired it implicitly, AGP 8 does not) - fixed by applying alias(libs.plugins.kotlinAndroid) directly. Adds .github/PULL_REQUEST_TEMPLATE.md and a CLAUDE.md convention for breaking large tickets like #613 into per-sub-issue PRs, so this process is repeatable for the rest of #613's remaining work. Verified: clean build succeeds, 38 unit tests pass with 0 failures, and the app was installed/launched on an emulator (Presets, Settings, and Selection Mode screens all render correctly with Voice functioning standalone). Closes #630. Part of #613. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…wiring Same root cause as app/build.gradle.kts's earlier fix: basetest's convention plugin (vocable.library) never explicitly applied the Kotlin plugin either, so its Kotlin sources (including FakeLocaleProvider, imported by three androidTest files) silently never compiled under AGP 8.13.1. CI's build job caught this because it runs assembleDebugAndroidTest, which local verification hadn't covered. - Apply alias(libs.plugins.kotlinAndroid) to basetest/build.gradle.kts - Delete .github/workflows/translate.yml - its only job (translateStrings) no longer exists now that the FuelIX plugin is removed; the workflow would fail on every future push otherwise - Strip the now-dead FUELIX_API_KEY env var from build.yml, pre-release-upload.yml, and ps-release.yml Verified: ./gradlew clean assembleDebug assembleDebugAndroidTest testDebug succeeds locally, matching CI's exact build job task list. Part of #613, #630. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Remove Language Selection scaffolding from feature/voice-selection
VocableTextToSpeech.applySelectedVoice() early-returned when no voice was explicitly selected, so Vocable never read or applied the device's actual configured TTS voice in that case - it just left whatever the engine's internal state defaulted to at init, with no recovery path if a previously-picked voice became unavailable. - speak() now returns Boolean (true = the persisted selection was stale and should be cleared); all three call sites (MainActivity, PresetsViewModel, KeyboardViewModel) react and clear it via setSelectedVoiceName(null) - applySelectedVoice() applies the live device default (via getDefaultVoice(), checked fresh every call, never persisted) when nothing's explicitly selected, instead of no-op-ing - Extracted the actual branch-selection logic into a pure resolveVoiceSelection() function, free of android.speech.tts types, so it's unit-testable without Robolectric/mocking (neither used in this repo) - 5 new tests in VocableTextToSpeechTest.kt - lastSetLocale skip and voice-after-setLanguage ordering preserved Also, as part of establishing a repeatable process for this feature's remaining sub-issues: - Preserved feature/voice-selection's pre-Language-Selection-removal state as branch archive/language-selection-pre-removal (commit b450b51), satisfying #617's acceptance criterion that work be kept on a branch rather than deleted outright - PR #631 had already merged before this was caught - Expanded CLAUDE.md's ticket-breakdown convention into a full "Starting new work" standard: issue-first via a new /create-ticket skill (.claude/skills/create-ticket/SKILL.md, which enforces a tight Why/Scope/Acceptance-Criteria/Out-of-Scope template and flags scope creep before an issue is created - directly countering how #613/#617/ #622 grew unmanageable), feature/<issue>/<description> branch naming, and a Documentation/work-log/ entry per unit of work - Documentation/work-log/632-tts-live-voice-fallback.md is the first entry under that new standard Known gap: the available emulator has no TTS engine installed at all (no Play Store to add one), so the live-fallback path itself could only be verified via unit tests + a no-crash regression check, not the full on-device manual-test plan from #617. Needs a real device or a Play-Store-enabled emulator before this is considered fully verified. Verified: ./gradlew clean assembleDebug assembleDebugAndroidTest testDebug succeeds (43 unit tests, 0 failures), app launches without crashing. Part of #617. Closes #632. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Confirmed the hard way: #630 stayed open after PR #631 merged into feature/voice-selection, since GitHub only auto-closes linked issues when the merging PR targets the repo's default branch. Added an explicit manual-close step to the Starting new work workflow so this doesn't keep silently happening across future sub-issue PRs. Part of #613. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ckets Moving durable background research out of live issue bodies and into Documentation/, per the same "don't embed research in the ticket, link to it" principle behind the create-ticket skill and the #613/#617/#622 cleanup. These two docs preserve research that would otherwise be lost when #627 and #629 get trimmed to the lean issue template. Part of #613. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…e-fallback TTS: live device-voice fallback when nothing is explicitly selected
applySelectedVoice()'s explicit-match check only verified a selected voice's name, locale, and network-required flag - it never checked whether the voice's data was actually downloaded. Android's TTS engine keeps a voice listed in getVoices() even after its data is uninstalled, only flagging it via KEY_FEATURE_NOT_INSTALLED rather than removing it from the list, so a previously-picked voice that later got uninstalled (the literal scenario #619 describes) would still "match" by name and keep being treated as a valid explicit selection. - Added isVoiceDownloaded() (Voice overload + a features:Set<String>? overload for unit-testability), checking KEY_FEATURE_NOT_INSTALLED - applySelectedVoice()'s availableVoiceNames now requires both isVoiceSupportedForLocale() and isVoiceDownloaded() - an uninstalled voice no longer counts as available, so resolveVoiceSelection() correctly returns STALE_FALLBACK_TO_LIVE_DEFAULT for it - getAvailableVoices() refactored to reuse the same helper instead of inlining the same check twice - 3 new unit tests on the pure isVoiceDownloaded(features) overload Verified via real on-device reproduction on a Google Play emulator image: selected a downloaded voice, uninstalled its data through the OS's own TTS voice manager, returned to the app. Before this fix: kept logging "applied voice: <name>" with no stale warning. After: correctly logs the stale warning, falls back to the live device default, and the persisted preference visibly clears to "Default" in Selection Mode - confirmed both immediately (no relaunch needed) and after a full relaunch. Part of #613. Follow-up to #632/PR #633. Fixes a real gap in #619's stale-detection scope. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds an explicit rule to CLAUDE.md's cross-repo section: where a Figma mock and the shipped iOS implementation disagree, follow iOS and flag the divergence back to design rather than building the mock. Cites the three concrete #636 cases where the mock was wrong (border vs. checkmark, one column vs. two on tablet landscape, per-voice proper names Android TTS can't produce), and adds the reminder to confirm parity is achievable at all before committing to it. Also corrects the section's stale mechanics: the iOS repo's default branch is `develop`, not `main`, and the previous text hardcoded one machine's clone layout. Clone location now described as varying by machine, with the public `gh api` read path documented as the fallback. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ownload-check Fix stale-voice detection to check download status, not just name/locale
The original template (What / Why it's safe / Out of scope, all free-text prose) was designed for the heavier #613 sub-issue PRs but added too much friction for smaller day-to-day PRs. - Summary (bullets), Ticket (GitHub issue reference - this repo uses GitHub Issues, not Jira, so adapted the wording accordingly), Type of Change / Testing / Checklist checkboxes - No Author section - CLAUDE.md's "Starting new work" step 3 updated to reference the new template shape Closes #646. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…emplate Simplify PR template to a lighter, checkbox-driven format
…-project-autolink create-ticket: auto-add issues to Project #50 and link sub-issue parents
…hroughput-forecast Port throughput-forecast skill to GitHub Projects v2 (Project #50)
Rows now take a fixed voice_row_height (60/48/80/64dp per dimens dir), matched to the square play chip so a name tile is exactly as tall as its chip, and pack from the top with any leftover space left at the bottom. This replaces the weighted fill, which stretched tiles to as much as 244dp on tablet portrait — mostly empty boxes around one line of text. Since row height no longer depends on row count, voice_rows is purely slots-per-page and the matrix is unchanged. voice_play_chip_max_size is renamed voice_row_height (same values); the chip's heightIn cap is gone because the fixed row height already bounds it. Overflow reverts from MiddleEllipsis to Ellipsis. MiddleEllipsis defeats TextAutoSize: with an overflow that can truncate to any width the text always "fits", so auto-size never steps down and the name truncated even at default font scale. With Ellipsis auto-size shrinks as intended and full names render at every breakpoint. maxLines is font-scale dependent (>1.25 gives a second line) so the name wraps rather than losing its trailing index once one line stops being possible. A flat maxLines=2 would make auto-size hold 16sp and wrap at default scale, orphaning the index on line two. Known limitation, documented rather than worked around: at large font scales the name still truncates. Compact rows mean one line cannot fit widthwise (~340dp needed, 213dp available) and two cannot fit heightwise (2x48dp line height in an 80dp row), so the extra line cannot be used. The remedies each carry a cost and need a decision — see the work log. Part of #613 — closes #644 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Completes the ticket's "replace hardcoded padding with breakpoint resources" criterion: the empty state's horizontal padding and spacer were the last hardcoded dp in the screen outside the previews. Values in `values` are unchanged (32dp / 16dp) so the default look is identical; landscape gets a tighter 24dp / 8dp. Also corrects two comments that overstated behaviour. The font-scale comment claimed the name wraps to a second line above 1.25x; it does not, because the fixed row height cannot fit two lines, so it still truncates there. Now labelled as a known limitation with a pointer to the work log. Part of #613 — closes #644 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…responsive-grid Change Voice: responsive 2-column grid matching iOS size classes
#644 laid the grid out as fixed-height rows packed from the top with the row count read from a `voice_rows` resource, so a full page covered only part of the grid area — 5 of 8 possible rows on phone portrait, leaving a 287dp empty band above the pager. Retuning the counts per breakpoint would not fix it: `sw###dp` qualifiers constrain width and never height, so the same values dir holds both a 393x851dp phone (8 rows fit) and a 360x640dp one (6 do), and a count for either is wrong on the other — dead space one way, clipped gaze targets the other. Derive the count instead, from the measured page height against a `voice_row_min_height` dimen, and let the rows fill by weight. A rendered row is then never shorter than the chip-matched height design asked for, never more than one row-pitch taller, and a full page always consumes the page exactly. `voice_columns` stays a resource and slot positions stay fixed per config, so the gaze contract is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
With 7 voices in a page of 8 slots, every bit of the leftover sat between the last tile and the pager while the first row butted up against the header. A short page now renders just its occupied rows and insets them from the top by half the leftover — centring the group — capped at half a row pitch. The cap is what makes it safe everywhere: plain centring looks right on a nearly-full page but floats a sparse one, e.g. 9 voices filling 5 of tablet portrait's 10 rows sat 276dp down an otherwise empty page. Capped, that is 52dp. It also bounds the vertical shift when paging onto a short last page to half a row rather than half a page, which matters because fixed tile positions are otherwise a gaze accessibility contract. Tile size is unchanged — rows keep the height a full page would give them — and a full page is untouched, since its leftover is zero. Note this diverges from iOS deliberately, on request: CarouselGridLayout defaults to `alignment = .top` and VoicePickerViewController never overrides it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Promotes clearAll() to IVocableSharedPreferences so it's reachable via DI like every other preference method, names the two remaining inline defaults, and adds a reset test per preference using the fake (no Robolectric here, so the real EncryptedSharedPreferences-backed class can't be exercised from a JVM test). Also fixes an inverted setFirstTime() in the fake, caught while writing the reset test. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds bulk-delete queries to each DAO, a public populateDatabase() on RoomPresetCategoriesRepository (mirroring the phrases repo), and resetToDefaults() on ICategoriesUseCase/IPhrasesUseCase that hard-wipes stored+preset rows and re-seeds from the bundled presets — reversing any #360-driven edit, hide, deletion, or custom addition, not just preset shadows. Verified end to end on-device: 7 new androidTest cases (category edit/delete/add, phrase edit/delete/add, Recents clearing) plus the full existing unit + androidTest suites, all green. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds a "Reset App Settings" row (5th SettingsButton) and reuses the Settings screen's existing confirmation-dialog block, parameterizing its copy per ExitDialogType rather than building a new dialog component. Confirm wires to categoriesUseCase.resetToDefaults() (#639) + prefs.clearAll() (#638) via the first async confirmDialog() branch. Checked the shipped iOS reset UX in ../vocable-ios first per this repo's cross-repo convention. Verified manually on-device: row renders, dialog copy correct, Cancel makes no changes, Reset dismisses without crash or stuck state, app fully responsive after (screenshots + logcat checked). Full unit suite green (10 SettingsViewModelTest cases, 6 new). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Two bugs found during manual on-device verification of #640: - Dialog Cancel/Reset text rendered pale mint instead of dark navy: Typography.labelLarge hardcodes color, which wins over Button's LocalContentColor for any Text() without its own color=. Fixed by setting color explicitly on both dialog buttons (also fixes the same latent bug on the pre-existing Privacy Policy/Contact Developers dialog buttons). - Selection Mode's Head Tracking toggle didn't reflect a reset in real time. Root cause was two-layered: FaceTrackingPermissions cached permissionState in memory, only updated via its own enable/disableFaceTracking() - added a prefs listener to fix that - but VocableSharedPreferences.clearAll()'s bare clear() never fires OnSharedPreferenceChangeListener at all (Android only notifies for keys explicitly put/removed in an edit). Fixed by following clear() with an explicit rewrite of every listener-observed key back to its default, so GazeButton/FaceTrackingViewModel/FaceTrackingPermissions all pick up a reset immediately. Added VocableSharedPreferencesTest (androidTest) with a regression case for the notification behavior, since it can't be exercised through the fake. Also corrected the reset dialog copy to match iOS's shipped wording exactly (pulled from vocable-ios's Localizable.xcstrings) instead of invented text, and reused the row's own label as the dialog title since iOS's alert has no separate title. Verified end-to-end on a physical device: toggled head tracking off, reset, and confirmed it flips back on immediately with the ARCore session actually resuming, no navigation or restart needed. Full unit suite and full connectedDebugAndroidTest suite (53 tests) green. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…lay-api-source Source versionCode from Play Console instead of per-workflow run_number
…ll-page Change Voice: fill the page height instead of leaving a dead band
…nce-wipe Reset: full preference wipe + defaults inventory
…y-phrase-reseed Reset: category/phrase data wipe + reseed
…s-ui Reset: Settings UI entry point + confirmation + accessibility
Per review on #655/#666: rather than relocating gh_fetch.py to dq-documentation while still vendoring project-specific copies here, the whole skill is now installed at ~/.claude/skills/github-throughput-forecast/ instead - available for any GitHub Projects v2 board, nothing committed into any one repo's history. Removes .claude/scripts/{gh_fetch,forecast,forecast-html}.py, .claude/skills/throughput-forecast/, and the settings.json that existed solely for their Bash allowlist. create-ticket (#654/#656) untouched - separate concern, out of scope. No merged history reverted - this is a normal forward delete commit. PR #665 closed unmerged rather than revised (its fixes carried over into the global install already). Closes #666
# Conflicts: # .github/workflows/pre-release-upload.yml # app/src/main/java/com/willowtree/vocable/core/IVocableSharedPreferences.kt # app/src/main/java/com/willowtree/vocable/core/VocableSharedPreferences.kt # app/src/main/java/com/willowtree/vocable/data/room/PresetPhrasesDao.kt # app/src/test/java/com/willowtree/vocable/utils/FakeVocableSharedPreferences.kt
Mansimran Singh (Mansimran-Singh)
approved these changes
Aug 6, 2026
…ed-throughput-forecast Remove vendored throughput-forecast files; skill now installed globally
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
main.IVocableSharedPreferences/VocableSharedPreferences/FakeVocableSharedPreferencesnear the same lines.setFirstTime/getFirstTime/KEY_FIRST_TIME) because preset seeding is now idempotent and runs every launch. This branch's preference-reset feature still referenced that concept inclearAll()and its tests. Resolved by dropping the dead first-time flag everywhere (interface, impl, fake, and the two tests that asserted on it) and keeping the reset feature's other defaults (selectedVoiceName, dwell time, sensitivity, head tracking).Testing
./gradlew testDebugUnitTestpasses on the merged branch🤖 Generated with Claude Code