-
-
Notifications
You must be signed in to change notification settings - Fork 103
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
fix(web): ios popup positioning and style #6383
Conversation
Fixes #5929. Fixes #6381. This fixes four separate issues with the longpress popups and key previews: 1. The key preview was offset by a couple of pixels (unreported issue). 2. The background 'shim' did not cover the key caps, which caused them to look startlingly white when the rest of the background greyed down (#5929). 3. The popup menu was incorrectly wrapping to two rows due to sizing model mismatches (#6381). 4. The popup menu would be realigned incorrectly for the in-app keyboard, when moving it to keep it in the keyboard area (#5929).
User Test ResultsTest specification and instructions
Test Artifacts |
Hmm, the positioning is certainly better than before. But it doesn't look quite right on KeymanWeb in iOS and Keyman for iOS. So will have another round. Note that the problem is that Safari's CSS is diverging from Chrome here. I haven't identified what is triggering the difference yet. Key Preview, Keyman for iOSLongpress, Keyman for iOSKey Preview, KeymanWeb on Safari / iOSLongpress, KeymanWeb on Safari / iOSFinal issue: some keys are misaligned, only in Safari, not in Chrome: |
Font size adjustment needed an override for the key preview to prevent it using the base key font size as a starting point.
On Safari, some keys were vertically misaligned, due to different default vertical-align.
Adjusts calculations for key preview and ensures we use box-sizing: content-box in all circumstances, to make it consistent across all platforms.
Adjusts positioning calculations for longpress popups and corresponding styles to make it consistent across all platforms, particularly iOS.
So I've done another round of tweaks to the positioning to try and get it working better. I have tested locally on my test build with Chrome on Windows (emulation and in developer frame), and in Safari on iOS, but not yet in iOS app. But this sadly means retesting everything, so sorry @bharanidharanj and @darcywong00! The screenshots were very helpful, please keep them coming. @keymanapp-test-bot retest all |
Yes, this doesn't look right... 😢 TEST_IOS FAILED |
Okay, I've fixed up the issues with the callout positioning on iOS. @keymanapp-test-bot retest TEST_IOS |
Adjusts the dimensions to use pixel boundaries to prevent antialiasing for some of the key preview and longpress popups, and adjusts horizontal positioning of key preview to center more precisely, for iOS.
Please note that the longpress menu overlapping the e key is not a bug, but forced on us because iOS does not allow our longpress menu to be outside the keyboard frame. But I'm still seeing some artifacts and alignment issues on your screenshots @bharanidharanj. I've highlighted a few of them from TEST_IOS; the TEST_WEB_IOS screenshots show the same issues. Neither the key preview nor the longpress are perfect.' I tested on the iPhone SE 3rd Generation 15.4 Simulator and things looked pretty much pixel perfect there. However @bharanidharanj tested on iPhone 13 Pro and here things did not look right. So now to find out why. Some examples:
|
Okay, I've tried again and tested locally on both iPhone SE 3rd gen and iPhone 13 Pro to cater to the different resolutions. We should probably test against both those devices and with more than one keyboard -- the subpixel calculations can mask issues on different resolutions. @keymanapp-test-bot retest TEST_IOS TEST_WEB_IOS |
@mcdurdin Okay. |
Good catch! I missed that when I was re-testing. Okay, I have put in a fix for that, can you retest please? @keymanapp-test-bot retest TEST_IOS |
@mcdurdin Okay. I will re-test it. Thanks. |
TEST_IOS (PASSED): I am comfortable with going ahead with that very minor artifact. Thank you for your attention to detail 😁 |
Changes in this pull request will be available for download in Keyman version 15.0.223-beta |
Fixes #5929.
Fixes #6381.
This fixes four separate issues with the longpress popups and key previews:
Longpress iOS (Chrome simulation):
Key preview iOS (Chrome simulation):
User Testing
We need to verify the behaviour of the longpress menu and the key preview. If possible, capture screenshots of both of these elements when running the tests, so we can compare.
Suggest using sil_euro_latin keyboard for this test, as it has many big longpress menus.
Test single-row long presses, such as long-press m, and multi-row, such as long-press g or e.
TEST_IOS: Verify that the in-app keyboard and system keyboard both display longpress menus and key previews correctly. Note that the longpress menu will be constrained to fit within the keyboard area on iOS, due to limitations with iOS system keyboards. Please test on iPhone 13 Pro and iPhone SE. Consider other devices also. Please test with 4 and 5 row keyboards.
TEST_ANDROID: Verify that the in-app keyboard and system keyboard both display longpress menus and key previews correctly.
TEST_WEB_IOS: Verify that the web keyboard in unminified/testing, on an iOS simulator or device, displays longpress menus and key previews correctly. Please test on iPhone 13 Pro and iPhone SE. Consider other devices also. Please test with 4 and 5 row keyboards.
TEST_WEB_ANDROID: Verify that the web keyboard in unminified/testing, on an Android emulator or device, displays longpress menus and key previews correctly.