feat(multi-language): Add in-keyboard language switching#135
Open
kiensh wants to merge 6 commits into
Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughIntroduces persisted multi-language support: ordered enabled languages, optional pinned default, UI to enable/disable and set default per language, globe-key cycling across enabled languages, and tests for migration, persistence, toggling, pinning, and cycling behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant KeyboardRootView
participant KeyboardViewModel
participant LanguageSettings
participant UserDefaults
User->>KeyboardRootView: Swipe right on globe key
activate KeyboardRootView
KeyboardRootView->>KeyboardViewModel: switchToNextLanguage()
deactivate KeyboardRootView
activate KeyboardViewModel
KeyboardViewModel->>LanguageSettings: nextLanguageId(after: currentId)
activate LanguageSettings
LanguageSettings-->>KeyboardViewModel: next enabled language ID
deactivate LanguageSettings
KeyboardViewModel->>UserDefaults: Persist selectedLanguageId
KeyboardViewModel->>KeyboardViewModel: applyLanguage(_:) — rebuild layout, update locale, reset shift
KeyboardViewModel-->>User: Render updated keyboard
deactivate KeyboardViewModel
sequenceDiagram
participant User
participant LanguageSelectionView
participant LanguageSettings
participant UserDefaults
User->>LanguageSelectionView: Tap language row / Toggle enable switch
activate LanguageSelectionView
LanguageSelectionView->>LanguageSettings: selectLanguage(...) or toggleLanguage(...)
deactivate LanguageSelectionView
activate LanguageSettings
alt Attempt to disable last enabled language
LanguageSettings-->>LanguageSelectionView: return false / reject
LanguageSelectionView-->>User: Present "Cannot Disable" alert
else Valid change
LanguageSettings->>UserDefaults: Persist enabledLanguageIds (JSON) and selected/pinned IDs
LanguageSettings-->>LanguageSelectionView: Published updates
end
deactivate LanguageSettings
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@wurstfingerKeyboard/KeyboardViewModel.swift`:
- Around line 177-183: The init uses LanguageSettings.shared.selectedLanguage
for layout/locale but reads enabledLanguageIds from the injected UserDefaults
via LanguageSettings.loadEnabledLanguageIds(from: defaults), causing a mismatch;
replace LanguageSettings.shared.selectedLanguage with a selectedLanguage loaded
from the injected store (e.g. call LanguageSettings.loadSelectedLanguage(from:
defaults) or equivalent), then compute self.layout = KeyboardLayout.layout(for:
selectedLanguage, numpadStyle: numpadStyle) and locale = selectedLanguage.locale
so both layout/locale and enabledLanguageIds consistently use the passed-in
UserDefaults rather than LanguageSettings.shared.
- Line 96: enabledLanguageIds is being updated in reloadLanguage() without
emitting objectWillChange, so views depending on hasMultipleLanguages don't
update; modify reloadLanguage() to compare the new enabledLanguageIds array with
the existing enabledLanguageIds and, if they differ and applyLanguage(_:) was
not called, call objectWillChange.send() (or mark enabledLanguageIds `@Published`)
so that changes to enabledLanguageIds trigger view updates used by
hasMultipleLanguages and KeyboardRootView.
In `@wurstfingerKeyboard/LanguageSettings.swift`:
- Around line 21-25: The public setter on enabledLanguageIds allows callers to
bypass invariants; change the declaration of `@Published` var enabledLanguageIds
to be read-only from outside (use private(set)) and ensure all updates go
through the existing mutation helpers that enforce "non-empty + selected
language is enabled" (e.g., the add/remove/toggle methods and
saveEnabledLanguages()). Also remove any external direct assignments to
enabledLanguageIds and replace them with calls to the mutation helper methods so
duplicates, empty lists, or stale IDs cannot be written.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 65c2063f-655a-4490-a9e0-8f7108d675f4
📒 Files selected for processing (8)
wurstfinger/LanguageSelectionView.swiftwurstfinger/SettingsView.swiftwurstfingerKeyboard/KeyHintOverlay.swiftwurstfingerKeyboard/KeyboardRootView.swiftwurstfingerKeyboard/KeyboardSettings.swiftwurstfingerKeyboard/KeyboardViewModel.swiftwurstfingerKeyboard/LanguageSettings.swiftwurstfingerTests/LanguageSettingsTests.swift
cl445
reviewed
Apr 8, 2026
Owner
cl445
left a comment
cl445
left a comment
There was a problem hiding this comment.
Great feature — clean implementation with thorough tests (19 new tests covering all edge cases). A few issues to address:
1. Nested Button in LanguageRow (Major)
LanguageRow has a Button wrapping the whole row and another Button for the toggle icon. Nested buttons in SwiftUI are unreliable — tapping the toggle can fire both onToggle and onTap, immediately reversing the disable action. Use contentShape(Rectangle()).onTapGesture() for the row tap and keep only the inner Button for the toggle.
2. enabledLanguageIds not triggering view updates in ViewModel (Major)
enabledLanguageIds is a plain private var in KeyboardViewModel. When languages are enabled/disabled in Settings without changing the active language, objectWillChange is never emitted — the globe key label stays stale. Fix: make it @Published private(set) or manually call objectWillChange.send() in reloadLanguage() when the array changes.
3. Inconsistent UserDefaults in init (Major)
KeyboardViewModel.init reads layout/locale from LanguageSettings.shared (backed by SharedDefaults.store) but reads enabledLanguageIds from the injected defaults parameter. This can cause a mismatch. Both should read from the same source.
4. Public setter on enabledLanguageIds (Minor)
In LanguageSettings, @Published var enabledLanguageIds should be @Published private(set) var to prevent callers from bypassing the invariants enforced by toggleLanguage() / selectLanguage() (non-empty list, selected language always in enabled set).
5. Singleton pattern broken
LanguageSettings.init() was intentionally made private in v1.2.0 (singleton pattern). This PR makes it public for testability. Consider making the init(userDefaults:) overload internal (visible to tests) while keeping the default init() private, so production code still goes through .shared.
6. Duplicate cycling logic
nextLanguageId(after:) in LanguageSettings and switchToNextLanguage() in KeyboardViewModel implement the same cycling logic independently. The ViewModel should call LanguageSettings.nextLanguageId(after:) instead of reimplementing the index arithmetic.
7. JSON serialization for String array (Minor)
saveEnabledLanguageIds uses JSONEncoder/JSONDecoder for a [String]. UserDefaults natively supports string arrays via set(_:forKey:) and stringArray(forKey:), including cross-process — the JSON round-trip adds unnecessary complexity.
kiensh
added a commit
to kiensh/wurstfinger
that referenced
this pull request
Apr 9, 2026
- Fix nested Button in LanguageRow: use contentShape + onTapGesture for row tap, keep only inner Button for toggle (cl445 cl445#1) - Make enabledLanguageIds @published in KeyboardViewModel so globe key label updates when enabled list changes (cl445 cl445#2) - Use injected UserDefaults consistently in KeyboardViewModel init instead of mixing .shared and raw defaults (cl445 cl445#3) - Make enabledLanguageIds private(set) in LanguageSettings to enforce mutation through toggleLanguage/selectLanguage (cl445 cl445#4) - Deduplicate cycling logic: extract static nextLanguageId(after:in:) on LanguageSettings, call from both instance method and ViewModel (cl445 cl445#6) - Replace JSON encode/decode with native UserDefaults stringArray for enabled language persistence (cl445 cl445#7) Made-with: Cursor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
wurstfingerKeyboard/LanguageSettings.swift (1)
10-25:⚠️ Potential issue | 🔴 CriticalImport
CombineforObservableObjectand@Published.This file only imports
Foundation, butObservableObjectand@Publishedare defined in the Combine framework. The code will not compile without importing Combine.🛠️ Suggested fix
import Foundation +import Combine🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wurstfingerKeyboard/LanguageSettings.swift` around lines 10 - 25, Add the missing Combine import at the top of the file so ObservableObject and `@Published` are available: update the imports for the LanguageSettings class (which declares ObservableObject conformance and uses `@Published` properties like selectedLanguageId and enabledLanguageIds) to include "import Combine" alongside existing Foundation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@wurstfingerKeyboard/KeyboardViewModel.swift`:
- Around line 229-239: reloadLanguage currently reads the raw array from
sharedDefaults and assigns it directly to enabledLanguageIds, bypassing
LanguageSettings' normalization; change reloadLanguage to use
LanguageSettings.loadEnabledLanguageIds(from: sharedDefaults) (or its
initializer/normalize path) to repopulate enabledLanguageIds so stale/empty IDs
are filtered and a non-empty set is guaranteed before calling applyLanguage or
allowing switchToNextLanguage to run; ensure you still read the selected
languageId from sharedDefaults and only call applyLanguage(_: ) if the
normalized enabledLanguageIds contain that id.
In `@wurstfingerKeyboard/LanguageSettings.swift`:
- Around line 113-134: selectLanguage(_:) and the removal branch of
toggleLanguage(_:) can transiently persist a selectedLanguageId that is not
present in enabledLanguageIds; fix by always mutating the
enabledLanguageIds/enabled set in the correct order so the selectedLanguageId is
never outside the enabled set: in selectLanguage(_:) ensure you append
language.id to enabledLanguageIds before assigning selectedLanguageId, and in
toggleLanguage(_:) when removing the currently selected language assign
selectedLanguageId to the replacement (e.g. enabledLanguageIds.first(where: !=
language.id) or the first remaining id) before removing language.id from
enabledLanguageIds so that at persistence time selectedLanguageId is always one
of enabledLanguageIds.
- Around line 107-111: Change the user-visible language label to use
locale-aware casing and centralize the formatting logic in LanguageSettings:
replace the plain uppercased() in the currentLanguageLabel computed property
(which currently derives lang via
selectedLanguage.locale.language.languageCode?.identifier ?? selectedLanguageId)
with uppercased(using the appropriate Locale) — e.g. uppercased(with:
selectedLanguage.locale) — and extract that logic into a single public formatter
(a computed property or a static method on LanguageSettings) so
KeyboardViewModel can call the same formatter instead of duplicating the
identifier + uppercasing logic.
---
Outside diff comments:
In `@wurstfingerKeyboard/LanguageSettings.swift`:
- Around line 10-25: Add the missing Combine import at the top of the file so
ObservableObject and `@Published` are available: update the imports for the
LanguageSettings class (which declares ObservableObject conformance and uses
`@Published` properties like selectedLanguageId and enabledLanguageIds) to include
"import Combine" alongside existing Foundation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 97210d0c-419b-4f97-aad2-854762a17db4
📒 Files selected for processing (3)
wurstfinger/LanguageSelectionView.swiftwurstfingerKeyboard/KeyboardViewModel.swiftwurstfingerKeyboard/LanguageSettings.swift
Owner
|
Hi @kiensh, thanks a lot for this contribution — in-keyboard language switching is a great addition and I really appreciate the effort you put into it! 🙏 Before I can take another pass at merging, could you please address the remaining unresolved CodeRabbit findings on this PR? There are currently three open ones that still apply to the latest commit (
Once those are addressed I'll re-review and we can get this merged. Thanks again! |
kiensh
added a commit
to kiensh/wurstfinger
that referenced
this pull request
Apr 11, 2026
- Fix nested Button in LanguageRow: use contentShape + onTapGesture for row tap, keep only inner Button for toggle (cl445 cl445#1) - Make enabledLanguageIds @published in KeyboardViewModel so globe key label updates when enabled list changes (cl445 cl445#2) - Use injected UserDefaults consistently in KeyboardViewModel init instead of mixing .shared and raw defaults (cl445 cl445#3) - Make enabledLanguageIds private(set) in LanguageSettings to enforce mutation through toggleLanguage/selectLanguage (cl445 cl445#4) - Deduplicate cycling logic: extract static nextLanguageId(after:in:) on LanguageSettings, call from both instance method and ViewModel (cl445 cl445#6) - Replace JSON encode/decode with native UserDefaults stringArray for enabled language persistence (cl445 cl445#7) Made-with: Cursor
kiensh
force-pushed
the
feature/support-multi-language
branch
from
13c6aac to
6902e2a
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
wurstfingerKeyboard/KeyboardViewModel.swift (1)
267-289:⚠️ Potential issue | 🟠 MajorNormalize the reloaded enabled-language list before using it.
If shared defaults contain
[]or only stale IDs, this branch leavesenabledLanguageIdsunchanged.switchToNextLanguage()then keeps cycling the old in-memory list, which is the same dead-globe failure mode from the earlier review.Proposed fix
private func reloadLanguage() { - if let raw = LanguageSettings.loadEnabledLanguageIds(from: sharedDefaults) { - let valid = raw.filter { LanguageConfig.language(withId: $0) != nil } - if !valid.isEmpty { - enabledLanguageIds = valid - } - } + let storedSelected = sharedDefaults.string(forKey: SettingsKey.selectedLanguageId.rawValue) + ?? LanguageSettings.detectSystemLanguage() + let resolvedSelected = LanguageConfig.language(withId: storedSelected)?.id + ?? LanguageConfig.english.id + + if let raw = LanguageSettings.loadEnabledLanguageIds(from: sharedDefaults) { + let valid = raw.filter { LanguageConfig.language(withId: $0) != nil } + enabledLanguageIds = valid.isEmpty ? [resolvedSelected] : valid + } else { + enabledLanguageIds = [resolvedSelected] + } + + if !enabledLanguageIds.contains(resolvedSelected) { + enabledLanguageIds.insert(resolvedSelected, at: 0) + } // Pinned language overrides last-used on keyboard appear let pinned = sharedDefaults.string(forKey: SettingsKey.pinnedLanguageId.rawValue)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wurstfingerKeyboard/KeyboardViewModel.swift` around lines 267 - 289, reloadLanguage() currently ignores an empty or all-invalid enabled-language list and leaves enabledLanguageIds unchanged, causing switchToNextLanguage() to keep cycling stale IDs; update reloadLanguage() to normalize the loaded list by replacing enabledLanguageIds with the filtered valid list even when that list is empty (or, if empty, fall back to a sensible default such as LanguageSettings.detectSystemLanguage() or a single available LanguageConfig.language(withId:) id), i.e. use LanguageSettings.loadEnabledLanguageIds(from:) -> valid = raw.filter { LanguageConfig.language(withId: $0) != nil } and then assign enabledLanguageIds = valid (or enabledLanguageIds = [fallbackId] when valid.isEmpty) before applying the pinned/selected logic so switchToNextLanguage() operates on a fresh normalized list.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@wurstfingerKeyboard/KeyboardViewModel.swift`:
- Around line 180-193: The initializer currently always uses
languageSettings.selectedLanguage; change it to prefer
languageSettings.pinnedLanguageId when present so startup honors the pinned
language. Concretely: when computing selectedLanguage (used by
KeyboardLayout.layout(for:...) and locale), check if
languageSettings.pinnedLanguageId exists and resolve that to the Language
instance (via the existing lookup method on LanguageSettings or Language),
otherwise fall back to selectedLanguage; then use that resolved language to
construct self.layout, set locale, and enabledLanguageIds. Update references to
selectedLanguage in this block (KeyboardLayout.layout(for:), locale assignment)
to use the resolved pinned-or-selected language.
In `@wurstfingerTests/LanguageSettingsTests.swift`:
- Around line 564-579: Current tests assert LanguageSettings.startupLanguageId
but not that KeyboardViewModel boots from the pin; add a ViewModel-level test
that uses createTestDefaults/createSettings to persist a pinnedId (e.g.
"ru_RU"), instantiate KeyboardViewModel(userDefaults: defaults) and assert the
view model's startup/current language property (e.g. KeyboardViewModel's
selectedLanguageId or startupLanguageId) equals the pinnedId; this will catch
regressions where KeyboardViewModel reads selectedId instead of
LanguageSettings.startupLanguageId.
---
Duplicate comments:
In `@wurstfingerKeyboard/KeyboardViewModel.swift`:
- Around line 267-289: reloadLanguage() currently ignores an empty or
all-invalid enabled-language list and leaves enabledLanguageIds unchanged,
causing switchToNextLanguage() to keep cycling stale IDs; update
reloadLanguage() to normalize the loaded list by replacing enabledLanguageIds
with the filtered valid list even when that list is empty (or, if empty, fall
back to a sensible default such as LanguageSettings.detectSystemLanguage() or a
single available LanguageConfig.language(withId:) id), i.e. use
LanguageSettings.loadEnabledLanguageIds(from:) -> valid = raw.filter {
LanguageConfig.language(withId: $0) != nil } and then assign enabledLanguageIds
= valid (or enabledLanguageIds = [fallbackId] when valid.isEmpty) before
applying the pinned/selected logic so switchToNextLanguage() operates on a fresh
normalized list.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 370a9301-3e20-4934-bff5-f3d8e5bd469c
📒 Files selected for processing (6)
wurstfinger/LanguageSelectionView.swiftwurstfinger/SettingsView.swiftwurstfingerKeyboard/KeyboardSettings.swiftwurstfingerKeyboard/KeyboardViewModel.swiftwurstfingerKeyboard/LanguageSettings.swiftwurstfingerTests/LanguageSettingsTests.swift
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
wurstfingerTests/AutoCapitalizationTests.swift (1)
223-247:⚠️ Potential issue | 🟠 MajorAvoid mutating
SharedDefaults.storein this test.The new
removeObject(forKey: pinnedLanguageId)makes this test permanently alter one more global key, and it still never snapshots/restoresenabledLanguageIds. That makes the assertion order-dependent on whatever prior tests left in shared defaults. SinceKeyboardViewModelalready accepts injectedUserDefaults, use a per-test suite here instead.Safer pattern
`@Test` func testReloadLanguageResetsShiftState() { - let viewModel = KeyboardViewModel(shouldPersistSettings: false) + let suiteName = "test.reloadLanguageResetsShiftState.\(UUID().uuidString)" + let defaults = UserDefaults(suiteName: suiteName)! + defer { defaults.removePersistentDomain(forName: suiteName) } + defaults.set("en_US", forKey: SettingsKey.selectedLanguageId.rawValue) + LanguageSettings.saveEnabledLanguageIds(["en_US", "de_DE"], to: defaults) + + let viewModel = KeyboardViewModel(userDefaults: defaults, shouldPersistSettings: false) viewModel.bindActionHandler { _ in } @@ - let store = SharedDefaults.store + let store = defaults let currentId = viewModel.currentLocale().identifier let newId = (currentId == "en_US") ? "de_DE" : "en_US" store.set(newId, forKey: SettingsKey.selectedLanguageId.rawValue) store.removeObject(forKey: SettingsKey.pinnedLanguageId.rawValue)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wurstfingerTests/AutoCapitalizationTests.swift` around lines 223 - 247, This test must stop mutating the global SharedDefaults.store; instead create a per-test UserDefaults suite (e.g., a temporary suiteName or an in-memory UserDefaults) and seed it with the necessary keys (SettingsKey.selectedLanguageId and any enabledLanguageIds/pinnedLanguageId state you need) then construct KeyboardViewModel with that injected UserDefaults (use the KeyboardViewModel initializer that accepts injected UserDefaults rather than SharedDefaults.store), call viewModel.reloadSettings(), and assert; finally clean up the temporary suite (remove persistent domain) so no global state is changed.
♻️ Duplicate comments (1)
wurstfingerKeyboard/KeyboardViewModel.swift (1)
271-293:⚠️ Potential issue | 🟠 Major
reloadLanguage()should not reapply startup pin logic on every settings refresh.This path runs for generic
UserDefaultsreloads, not just keyboard startup. RecomputinglanguageIdfrompinnedLanguageIdhere means a user who temporarily switched away from the pinned language can get snapped back to the pin when any unrelated setting changes (for exampletoggleUtilityColumnPosition()). It also still bypasses the full normalization inLanguageSettings(userDefaults:), so stale/empty enabled lists are only partially repaired here.Suggested direction
-private func reloadLanguage() { - if let raw = LanguageSettings.loadEnabledLanguageIds(from: sharedDefaults) { - let valid = raw.filter { LanguageConfig.language(withId: $0) != nil } - if !valid.isEmpty { - enabledLanguageIds = valid - } - } - - // Pinned language overrides last-used on keyboard appear - let pinned = sharedDefaults.string(forKey: SettingsKey.pinnedLanguageId.rawValue) - let languageId = if let pinned, enabledLanguageIds.contains(pinned), - LanguageConfig.language(withId: pinned) != nil { - pinned - } else { - sharedDefaults.string(forKey: SettingsKey.selectedLanguageId.rawValue) - ?? LanguageSettings.detectSystemLanguage() - } +private func reloadLanguage(applyStartupOverride: Bool = false) { + let settings = LanguageSettings(userDefaults: sharedDefaults) + enabledLanguageIds = settings.enabledLanguageIds + let languageId = applyStartupOverride ? settings.startupLanguageId : settings.selectedLanguageId if languageId != locale.identifier { applyLanguage(languageId) if shouldPersistSettings { sharedDefaults.set(languageId, forKey: SettingsKey.selectedLanguageId.rawValue) } } }Then call
reloadLanguage(applyStartupOverride: true)only from the actual startup/appear flow.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wurstfingerKeyboard/KeyboardViewModel.swift` around lines 271 - 293, reloadLanguage currently reapplies the startup "pinned" override on every UserDefaults refresh which can snap users back to a pinned language; change reloadLanguage to accept a Bool parameter (e.g., applyStartupOverride: Bool = false) and only consult SettingsKey.pinnedLanguageId and apply the pinned override when applyStartupOverride is true; keep the existing enabledLanguageIds normalization by continuing to use LanguageSettings.loadEnabledLanguageIds(from:) (or construct LanguageSettings(userDefaults:) to get full normalization) and only fall back to selectedLanguageId/LanguageSettings.detectSystemLanguage() when not applying the startup override; update the keyboard startup/appear flow to call reloadLanguage(applyStartupOverride: true) while leaving other UserDefaults reload triggers to call reloadLanguage() (default false); relevant symbols: reloadLanguage, LanguageSettings.loadEnabledLanguageIds / LanguageSettings(userDefaults:), sharedDefaults, SettingsKey.pinnedLanguageId, SettingsKey.selectedLanguageId, applyLanguage, shouldPersistSettings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@wurstfingerKeyboard/KeyboardViewModel.swift`:
- Around line 524-528: The computed property currentLanguageLabel duplicates
locale-sensitive uppercasing logic and should delegate to the centralized
implementation in LanguageSettings instead of recomputing from locale; replace
the body of currentLanguageLabel to return the shared/appropriate
LanguageSettings property or helper (e.g., LanguageSettings.shared.shortLabel or
LanguageSettings.shortLanguageLabel(for:)) that already performs locale-aware
uppercasing (handles de_DE and other locales) and remove the manual
uppercased(with: locale) logic here.
- Around line 317-319: The switchToNextLanguage() branch currently writes nextId
into sharedDefaults unconditionally; change it to respect the
shouldPersistSettings flag by only calling sharedDefaults.set(nextId, forKey:
SettingsKey.selectedLanguageId.rawValue) when shouldPersistSettings is true,
while still calling applyLanguage(nextId) when nextId != currentId. Locate the
conditional around nextId/currentId in switchToNextLanguage() and add the guard
around the sharedDefaults write (leave the comparison and applyLanguage call
intact).
---
Outside diff comments:
In `@wurstfingerTests/AutoCapitalizationTests.swift`:
- Around line 223-247: This test must stop mutating the global
SharedDefaults.store; instead create a per-test UserDefaults suite (e.g., a
temporary suiteName or an in-memory UserDefaults) and seed it with the necessary
keys (SettingsKey.selectedLanguageId and any enabledLanguageIds/pinnedLanguageId
state you need) then construct KeyboardViewModel with that injected UserDefaults
(use the KeyboardViewModel initializer that accepts injected UserDefaults rather
than SharedDefaults.store), call viewModel.reloadSettings(), and assert; finally
clean up the temporary suite (remove persistent domain) so no global state is
changed.
---
Duplicate comments:
In `@wurstfingerKeyboard/KeyboardViewModel.swift`:
- Around line 271-293: reloadLanguage currently reapplies the startup "pinned"
override on every UserDefaults refresh which can snap users back to a pinned
language; change reloadLanguage to accept a Bool parameter (e.g.,
applyStartupOverride: Bool = false) and only consult
SettingsKey.pinnedLanguageId and apply the pinned override when
applyStartupOverride is true; keep the existing enabledLanguageIds normalization
by continuing to use LanguageSettings.loadEnabledLanguageIds(from:) (or
construct LanguageSettings(userDefaults:) to get full normalization) and only
fall back to selectedLanguageId/LanguageSettings.detectSystemLanguage() when not
applying the startup override; update the keyboard startup/appear flow to call
reloadLanguage(applyStartupOverride: true) while leaving other UserDefaults
reload triggers to call reloadLanguage() (default false); relevant symbols:
reloadLanguage, LanguageSettings.loadEnabledLanguageIds /
LanguageSettings(userDefaults:), sharedDefaults, SettingsKey.pinnedLanguageId,
SettingsKey.selectedLanguageId, applyLanguage, shouldPersistSettings.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8d7a149a-757e-4b9c-b576-b3de7095c3cc
📒 Files selected for processing (3)
wurstfingerKeyboard/KeyboardViewModel.swiftwurstfingerTests/AutoCapitalizationTests.swiftwurstfingerTests/LanguageSettingsTests.swift
Comment on lines
+317
to
+319
| if nextId != currentId { | ||
| sharedDefaults.set(nextId, forKey: SettingsKey.selectedLanguageId.rawValue) | ||
| applyLanguage(nextId) |
There was a problem hiding this comment.
Honor shouldPersistSettings when cycling languages.
The init/reload paths already suppress selectedLanguageId writes for non-persisting view models, but switchToNextLanguage() still writes unconditionally. That leaks cycled state into shared defaults from tests and defeats the isolation this flag is meant to provide.
Small fix
if nextId != currentId {
- sharedDefaults.set(nextId, forKey: SettingsKey.selectedLanguageId.rawValue)
+ if shouldPersistSettings {
+ sharedDefaults.set(nextId, forKey: SettingsKey.selectedLanguageId.rawValue)
+ }
applyLanguage(nextId)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@wurstfingerKeyboard/KeyboardViewModel.swift` around lines 317 - 319, The
switchToNextLanguage() branch currently writes nextId into sharedDefaults
unconditionally; change it to respect the shouldPersistSettings flag by only
calling sharedDefaults.set(nextId, forKey:
SettingsKey.selectedLanguageId.rawValue) when shouldPersistSettings is true,
while still calling applyLanguage(nextId) when nextId != currentId. Locate the
conditional around nextId/currentId in switchToNextLanguage() and add the guard
around the sharedDefaults write (leave the comparison and applyLanguage call
intact).
Comment on lines
+524
to
+528
| /// Short uppercase label for the current language, e.g. "EN", "RU" | ||
| var currentLanguageLabel: String { | ||
| let lang = locale.language.languageCode?.identifier ?? "" | ||
| return lang.uppercased(with: locale) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Centralize the language-label formatter.
LanguageSettings already exposes this derived label. Keeping a second locale-sensitive uppercase implementation here recreates the drift that caused the casing bug in the first place. Prefer a shared helper/property instead of recomputing it from locale.
As per coding guidelines, ensure locale-aware uppercasing (German: de_DE).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@wurstfingerKeyboard/KeyboardViewModel.swift` around lines 524 - 528, The
computed property currentLanguageLabel duplicates locale-sensitive uppercasing
logic and should delegate to the centralized implementation in LanguageSettings
instead of recomputing from locale; replace the body of currentLanguageLabel to
return the shared/appropriate LanguageSettings property or helper (e.g.,
LanguageSettings.shared.shortLabel or LanguageSettings.shortLanguageLabel(for:))
that already performs locale-aware uppercasing (handles de_DE and other locales)
and remove the manual uppercased(with: locale) logic here.
- Enable users to select multiple keyboard languages in settings and cycle through them by swiping right on the globe key. - The language indicator appears on the globe key when more than one language is enabled. - Existing single-language users are seamlessly migrated.
- Fix nested Button in LanguageRow: use contentShape + onTapGesture for row tap, keep only inner Button for toggle (cl445 cl445#1) - Make enabledLanguageIds @published in KeyboardViewModel so globe key label updates when enabled list changes (cl445 cl445#2) - Use injected UserDefaults consistently in KeyboardViewModel init instead of mixing .shared and raw defaults (cl445 cl445#3) - Make enabledLanguageIds private(set) in LanguageSettings to enforce mutation through toggleLanguage/selectLanguage (cl445 cl445#4) - Deduplicate cycling logic: extract static nextLanguageId(after:in:) on LanguageSettings, call from both instance method and ViewModel (cl445 cl445#6) - Replace JSON encode/decode with native UserDefaults stringArray for enabled language persistence (cl445 cl445#7) Made-with: Cursor
- Add import Combine for ObservableObject/@published - Fix mutation order in selectLanguage/toggleLanguage to prevent transient state where selectedLanguageId is outside enabledLanguageIds - Use locale-aware uppercased(with:) in currentLanguageLabel - Use LanguageSettings normalization in reloadLanguage to filter stale IDs Made-with: Cursor
…rrides startup When the keyboard opens with a pinned default language, selectedLanguageId in UserDefaults still held the last-used language from the previous session. This caused the first globe swipe-right to cycle back to the already-displayed pinned language (a no-op), requiring a second swipe to actually switch.
The init was using selectedLanguage instead of startupLanguageId, causing the keyboard to briefly boot in the wrong language when a pin was set. Guard selectedLanguageId sync writes behind shouldPersistSettings to avoid test interference. Add ViewModel-level integration test for pinned startup.
… architecture Adapt multi-language features (swipe-right on globe key to cycle languages, language label overlay, enabledLanguageIds persistence) to work with the new data-driven keyboard pipeline introduced on develop.
kiensh
force-pushed
the
feature/support-multi-language
branch
from
caf65ab to
818a6ec
Compare
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
2 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.
Closes #91
Summary
Enable users to select multiple keyboard languages in settings and cycle through them by swiping right on the globe key, without leaving the keyboard.
enabledLanguageIdsto shared UserDefaults for multi-language persistenceLanguageSettingswith enabled list, toggle, cycling, and migration from single-languageLanguageSelectionViewas multi-select UI with active/enabled distinctionswitchToNextLanguage()toKeyboardViewModel, triggered by globe swipe-rightLanguageConfigrequires zero changes elsewhereTest plan
Summary by CodeRabbit
New Features
UI/UX Improvements
Tests