-
-
Notifications
You must be signed in to change notification settings - Fork 110
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(ios): configurable keyboard height #12571
base: master
Are you sure you want to change the base?
Conversation
User Test ResultsTest specification and instructions
Test Artifacts
|
need to handle persistance to UserDefaults
506da20
to
221ba7b
Compare
viewWillTransition size argument does not reflect device bounds
updates constraints on CustomInputView only so that changes are isolated to the in-app keyboard and have no effect on the system keyboard
also avoid call to userdefaults unless key exists
first check UserDefaults for saved values if no saved values, determine default keyboard heights write default keyboard heights to UserDefaults
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just nits. Good work
private func determineDefaultKeyboardHeights() { | ||
// if no KeyboardScaleMap found for device, then default to 216.0 | ||
let portraitKeyboardScale = KeyboardScaleMap.getDeviceDefaultKeyboardScale(forPortrait: true) | ||
self.defaultPortraitHeight = Double(portraitKeyboardScale?.keyboardHeight ?? 216.0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
216.0 appears in a few places. Is it worth making it a constant somewhere?
if (self.isPortrait) { | ||
if (Storage.active.userDefaults.object(forKey: Key.portraitKeyboardHeight) != nil) { | ||
self.keyboardHeight = Storage.active.userDefaults.portraitKeyboardHeight | ||
let message = "applyKeyboardHeight, from UserDefaults loaded portrait value \(self.keyboardHeight)" | ||
os_log("%{public}s", log:KeymanEngineLogger.ui, type: .debug, message) | ||
} else { | ||
self.keyboardHeight = self.defaultPortraitHeight | ||
let message = "applyKeyboardHeight, portraitHeight not found in UserDefaults, using default value \(self.keyboardHeight)" | ||
os_log("%{public}s", log:KeymanEngineLogger.ui, type: .error, message) | ||
} | ||
} else { | ||
if (Storage.active.userDefaults.object(forKey: Key.portraitKeyboardHeight) != nil) { | ||
self.keyboardHeight = Storage.active.userDefaults.landscapeKeyboardHeight | ||
let message = "applyKeyboardHeight, from UserDefaults loaded landscape value \(self.keyboardHeight)" | ||
os_log("%{public}s", log:KeymanEngineLogger.ui, type: .debug, message) | ||
} else { | ||
self.keyboardHeight = self.defaultLandscapeHeight | ||
let message = "applyKeyboardHeight, landscapeHeight not found in UserDefaults, using default value \(self.keyboardHeight)" | ||
os_log("%{public}s", log:KeymanEngineLogger.ui, type: .error, message) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a fair bit of repetition between landscape and portrait orientation code. Could we merge the values into an array (0=portait, 1=landscape) or something like that? What do you think? Or an {x,y}?
Test ResultsI tested this issue with the attached "Keyman 18.0.130-alpha-test-12571" build(test flight) on the iOS 17.4, iPhone 13 physical device.
|
Allows the height of the Keyman keyboard to be adjusted by the user from within the Keyman Settings.
Fixes #5897
User Testing