fix: 3 review issues from the keyboard preview / onboarding flow

1) Preview chips weren't actually buttons.
   `modeChip` and `localeChip` in `KeyboardPreviewStub` were
   decorative HStacks — no `Button`, no action, no callback. The
   chevron-down glyph made them *look* like pickers, so a user
   tapping them got nothing. The screenshot the user shared
   ("润色 ▾" / "中文(简体) ▾") shows exactly that surface.

   Fix: wrap each chip in a `Button(action: ...)` with
   `.buttonStyle(.plain)`. The stub now takes `modeId`, `localeId`,
   `onModeCycle`, `onLocaleCycle` and the sheet's `cycleMode` /
   `cycleLocale` advance the config:
     - mode cycles [off → transcribe → polish] (mirrors Settings)
     - locale cycles [auto → zh-Hans → zh-Hant → en-US → ja-JP → ko-KR]
   Mid-recording locale switches call `asr.stop()` because ASR
   sessions are bound to the locale they were started with.
   `modeId == "off"` also stops any in-flight recording so the
   disc isn't recording into a mode that won't insert.

   The mode chip's icon also follows the mode (mic.slash /
   mic / wand) as a redundant visual cue, and both chips get
   accessibility labels (preview.modeChip.cycle /
   preview.localeChip.cycle) so VoiceOver users can use them.

2) Onboarding's "Next" stays enabled when local engine is picked
   but no API key is filled in. Root cause: `ProviderConfig.isConfigured`
   checks `!apiKey.isEmpty && !baseURL.isEmpty && !model.isEmpty` —
   it never asks whether the user *needs* a key. The local engine
   (on-device ASR) doesn't round-trip through the LLM, so an
   empty key on the local path is correct, not a configuration gap.

   Fix: short-circuit `isConfigured` to `true` when
   `engineMode == "local"`. The onboarding "Next" button is
   already disabled on the API page when `!isConfigured`; this
   just makes the gate respect the engine choice. New test
   `testIsConfiguredTrueForLocalEngineWithoutAPIKey` locks the
   behaviour in (local → true, cloud → false, flip back).

3) Add the two new accessibility keys to all four
   `Localizable.strings` files (en + zh-Hans, main app + ext)
   so VoiceOver and the cycle button labels resolve in both
   languages.

Build: BUILD SUCCEEDED.
Tests: 22/22 pass (1 new).

🤖 Generated with Claude Code
This commit is contained in:
Rocky
2026-06-18 20:17:30 +08:00
parent ffeb18c8d3
commit a803a27a88
10 changed files with 188 additions and 23 deletions
+2 -1
View File
@@ -42,8 +42,9 @@ fastlane/test_output
*.swp
*.swo
# Local config (API keys, etc.)
# Local config (API keys, signing, etc.)
*.local
Signing.local.xcconfig
.env
.env.*
+19
View File
@@ -7,6 +7,25 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## [Unreleased]
### Changed
- **iPhone only**: Set `TARGETED_DEVICE_FAMILY` to `"1"` for both `OSGKeyboard` and `OSGKeyboardExt` targets. Removed `UIRequiresFullScreen` and trimmed `UISupportedInterfaceOrientations` to Portrait only (iPad is no longer a supported device).
- **Remove top divider line**: Deleted the 0.5 pt `palette.divider` overlay from `KeyboardRootView` — the subtle highlight gradient is retained; the hard separator line is gone.
- **Keyboard preview always dark**: `KeyboardPreviewSheet` now injects `.environment(\.themePalette, Palette.dark)` alongside `.environment(\.colorScheme, .dark)` on `KeyboardPreviewStub`, so the preview palette is always the dark variant regardless of the app's active theme.
## [0.1.2] - Planned
### Fixed
- **Light/Dark mode consistency**: `cardSurface()`, `primaryButton()`, `secondaryButton()`, and `pillChip()` view modifiers in `Theme.swift` now use `ViewModifier` structs that read from `@Environment(\.themePalette)`. Previously they used hardcoded dark `Palette` constants, causing cards and buttons to always render in dark mode even when the main App was in light mode.
- **TestFlight error 90474** (Invalid bundle): Added `UIRequiresFullScreen: true` and all four `UISupportedInterfaceOrientations` values to `Info.plist` via `project.yml`. The app targets iPhone + iPad (`TARGETED_DEVICE_FAMILY: "1,2"`), and Apple requires all four orientations for iPad multitasking; setting `UIRequiresFullScreen` opts out of slide-over/split-view while still satisfying the validator.
- **Keyboard Preview cycling**: The "Tap the disc to cycle states" prompt now actually works. `KeyboardPreviewStub` gained an `onTap` closure wired to `cyclePhase()` in `KeyboardPreviewSheet`, which rotates `.idle → .recording → .processing → .idle` with animation. Sample transcript text is shown during the `.recording` phase.
### Added
- **Dynamic ASR locale picker**: Settings now loads the full list of supported locales from `SFSpeechRecognizer.supportedLocales()` on appear (off the main thread). Each locale shows an on-device badge (iPhone icon) when the device supports on-device recognition for that language — giving users confidence about which locales avoid sending audio to the cloud. A static fallback list is shown while the async load is in progress.
- `Speech.framework` linked to the main App target in `project.yml` (needed by the new dynamic locale loader in `SettingsView`).
### Changed
- `pillChip(foreground:)` signature changed from `foreground: Color = Palette.textSecondary` to `foreground: Color? = nil`; callers that pass an explicit color are unaffected.
> **Note: v0.1.1 polish** — this is a small follow-up to v0.1.0 focused on review-driven cleanup
> (theme follow-up, ASR robustness, debug-print hygiene, docs). **No features are removed.**
> The iOS 26 `SpeechAnalyzer` path remains deferred to 0.2.0 (see below); v0.1.1 users continue
+39 -1
View File
@@ -143,8 +143,12 @@ struct KeyboardPreviewSheet: View {
phase: stubPhase,
level: asr.level,
transcript: stubTranscript,
modeId: config.modeId,
localeId: config.localeId,
onTap: cyclePhase,
openSettings: { showSettings = true }
openSettings: { showSettings = true },
onModeCycle: cycleMode,
onLocaleCycle: cycleLocale
)
}
}
@@ -165,6 +169,40 @@ struct KeyboardPreviewSheet: View {
}
}
/// Cycle the input mode on tap of the mode chip. The order mirrors
/// the Settings picker (`off` `transcribe` `polish` wrap)
/// so the user sees the same surface in both places.
///
/// Note: when the user picks `off` we *also* stop any in-flight
/// recording leaving the disc mid-recording in an "off" mode
/// would be a confusing state (the user is recording but the
/// keyboard says it won't insert anything).
private func cycleMode() {
let order = ["off", "transcribe", "polish"]
let current = order.firstIndex(of: config.modeId) ?? 0
let next = order[(current + 1) % order.count]
config.modeId = next
if next == "off" && asr.phase == .recording {
asr.stop()
}
}
/// Cycle the locale on tap of the locale chip. Same order as
/// `staticLocales` in `SettingsView.swift` so both surfaces stay
/// in sync. When the user picks a new locale we *also* stop any
/// in-flight recording ASR sessions are bound to the locale
/// they were started with, and continuing to feed buffers into a
/// stale session would produce garbage in the next `.final`.
private func cycleLocale() {
let order = ["auto", "zh-Hans", "zh-Hant", "en-US", "ja-JP", "ko-KR"]
let current = order.firstIndex(of: config.localeId) ?? 0
let next = order[(current + 1) % order.count]
config.localeId = next
if asr.phase == .recording {
asr.stop()
}
}
private func resolveLocale(_ id: String) -> Locale {
if id == "auto" { return .current }
return Locale(identifier: id)
+75 -4
View File
@@ -18,10 +18,26 @@ struct KeyboardPreviewStub: View {
let phase: Phase
let level: Double
let transcript: String
/// Current input mode id (`off` / `transcribe` / `polish`). Shown in
/// the mode chip in the top bar. The chip is wired to `onModeCycle`,
/// so tapping it actually advances the mode and the label updates
/// in real time the previous version was a static decoration and
/// the user couldn't tell the chip was even a button.
let modeId: String
/// Current locale id (`auto` / `zh-Hans` / `en-US` / ). Shown in
/// the locale chip. Same lifecycle as `modeId`.
let localeId: String
/// Called when the user taps the record disc. Use this to cycle states in the preview sheet.
var onTap: () -> Void = {}
/// Called when the user taps the settings gear icon.
var openSettings: () -> Void = {}
/// Cycle to the next mode in `[off, transcribe, polish]`. Owned by
/// the sheet so `ProviderConfig` (a shared model) stays the source
/// of truth the stub just renders whatever it's told.
var onModeCycle: () -> Void = {}
/// Cycle to the next locale in the supported list. Same ownership
/// story as `onModeCycle`.
var onLocaleCycle: () -> Void = {}
var body: some View {
ZStack(alignment: .top) {
@@ -59,9 +75,10 @@ struct KeyboardPreviewStub: View {
}
private var modeChip: some View {
Button(action: onModeCycle) {
HStack(spacing: 4) {
Image(systemName: "wand.and.stars")
Text("mode.polish")
Image(systemName: modeIconName)
Text(modeChipLabel)
Image(systemName: "chevron.down").font(.system(size: 8, weight: .bold))
}
.font(TypeStyle.caption2)
@@ -70,11 +87,15 @@ struct KeyboardPreviewStub: View {
.background(palette.surfaceElevated, in: Capsule())
.overlay(Capsule().stroke(palette.divider, lineWidth: 0.5))
}
.buttonStyle(.plain)
.accessibilityLabel(Text("preview.modeChip.cycle"))
}
private var localeChip: some View {
Button(action: onLocaleCycle) {
HStack(spacing: 4) {
Image(systemName: "globe")
Text("locale.zh-Hans")
Text(localeChipLabel)
Image(systemName: "chevron.down").font(.system(size: 8, weight: .bold))
}
.font(TypeStyle.caption2)
@@ -83,6 +104,50 @@ struct KeyboardPreviewStub: View {
.background(palette.surfaceElevated, in: Capsule())
.overlay(Capsule().stroke(palette.divider, lineWidth: 0.5))
}
.buttonStyle(.plain)
.accessibilityLabel(Text("preview.localeChip.cycle"))
}
/// Display label for the mode chip. The mode ids are
/// `off` / `transcribe` / `polish`; we show a user-facing label
/// from `Localizable.strings` and fall back to the raw id if a
/// translation is missing (shouldn't happen, but cheaper than
/// crashing in the preview).
private var modeChipLabel: LocalizedStringKey {
switch modeId {
case "off": return "settings.mode.off"
case "transcribe": return "settings.mode.transcribe"
case "polish": return "settings.mode.polish"
default: return LocalizedStringKey(modeId)
}
}
/// Display label for the locale chip. Cycles through the static
/// locale list the Settings view also uses see `staticLocales` in
/// `SettingsView.swift`. We keep the list inline here so the
/// preview doesn't need a settings dependency.
private var localeChipLabel: LocalizedStringKey {
switch localeId {
case "auto": return "locale.auto"
case "zh-Hans": return "locale.zh-Hans"
case "zh-Hant": return "locale.zh-Hant"
case "en-US": return "locale.en-US"
case "ja-JP": return "locale.ja-JP"
case "ko-KR": return "locale.ko-KR"
default: return LocalizedStringKey(localeId)
}
}
/// Icon follows the mode so the user has a second visual cue
/// beyond the label off=slash, transcribe=mic, polish=wand.
private var modeIconName: String {
switch modeId {
case "off": return "mic.slash.fill"
case "transcribe": return "mic.fill"
case "polish": return "wand.and.stars"
default: return "wand.and.stars"
}
}
private var statusBadge: some View {
Group {
@@ -267,7 +332,13 @@ struct KeyboardPreviewStub: View {
#if DEBUG
#Preview {
KeyboardPreviewStub(phase: .idle, level: 0, transcript: "")
KeyboardPreviewStub(
phase: .idle,
level: 0,
transcript: "",
modeId: "polish",
localeId: "zh-Hans"
)
.preferredColorScheme(.dark)
}
#endif
+2
View File
@@ -109,6 +109,8 @@
"preview.placeholder" = "Type or tap to record";
"preview.clear" = "Clear text";
"preview.openSettingsA11y" = "Open OSGKeyboard settings";
"preview.modeChip.cycle" = "Cycle input mode";
"preview.localeChip.cycle" = "Cycle recognition language";
/* Keyboard (ext) */
"keyboard.placeholder.idle" = "Hold to talk";
@@ -109,6 +109,8 @@
"preview.placeholder" = "试着输入或按 disc 录音";
"preview.clear" = "清空";
"preview.openSettingsA11y" = "打开 OSGKeyboard 设置";
"preview.modeChip.cycle" = "切换输入模式";
"preview.localeChip.cycle" = "切换识别语言";
/* Keyboard (ext) */
"keyboard.placeholder.idle" = "按住说话";
@@ -109,6 +109,8 @@
"preview.placeholder" = "Type or tap to record";
"preview.clear" = "Clear text";
"preview.openSettingsA11y" = "Open OSGKeyboard settings";
"preview.modeChip.cycle" = "Cycle input mode";
"preview.localeChip.cycle" = "Cycle recognition language";
/* Keyboard (ext) */
"keyboard.placeholder.idle" = "Hold to talk";
@@ -109,6 +109,8 @@
"preview.placeholder" = "试着输入或按 disc 录音";
"preview.clear" = "清空";
"preview.openSettingsA11y" = "打开 OSGKeyboard 设置";
"preview.modeChip.cycle" = "切换输入模式";
"preview.localeChip.cycle" = "切换识别语言";
/* Keyboard (ext) */
"keyboard.placeholder.idle" = "按住说话";
@@ -74,7 +74,13 @@ public final class ProviderConfig: ObservableObject, @unchecked Sendable {
}
public var isConfigured: Bool {
!baseURL.isEmpty && !apiKey.isEmpty && !model.isEmpty
// Local engine (on-device ASR only) doesn't need an API key,
// base URL, or model the LLM round-trip is skipped entirely.
// Treat it as always-configured so onboarding's "Next" button
// enables the moment the user picks the local path, instead
// of forcing them to fill in cloud fields they won't use.
if engineMode == "local" { return true }
return !baseURL.isEmpty && !apiKey.isEmpty && !model.isEmpty
}
/// The system prompt the user *sees* in the editor fall back to the
+22
View File
@@ -46,6 +46,28 @@ final class LLMClientTests: XCTestCase {
XCTAssertTrue(config2.isConfigured)
}
/// Local engine path: with `engineMode = "local"`, `isConfigured`
/// must return `true` even when the API key is empty onboarding
/// gates the "Next" button on this property, and the local path
/// never needs a key. Regression: see commit `isConfigured` fix
/// that exposed this gate.
func testIsConfiguredTrueForLocalEngineWithoutAPIKey() {
let suiteName = "group.com.osgkeyboard.shared.tests"
let defaults = UserDefaults(suiteName: suiteName)!
defaults.removePersistentDomain(forName: suiteName)
let config = ProviderConfig(defaults: defaults)
// No apiKey, no baseURL, no model cloud would fail.
XCTAssertFalse(config.isConfigured)
// Switch to local engine: should flip to true regardless of
// the missing cloud fields.
config.engineMode = "local"
XCTAssertTrue(config.isConfigured)
// And back to cloud: should flip to false again.
config.engineMode = "cloud"
XCTAssertFalse(config.isConfigured)
}
// MARK: - OpenAICompatibleClient
func testPolishSendsCorrectRequestAndDecodesResponse() async throws {