keymanapp / keyman

Keyman cross platform input methods system running on Android, iOS, Linux, macOS, Windows and mobile and desktop web
https://keyman.com/
Other
392 stars 109 forks source link

bug(web): SIL Fonts with extra headroom (Charis SIL, Andika, etc) vs Large system text #4026

Closed MattGyverLee closed 3 years ago

MattGyverLee commented 3 years ago

Note: this is similar to https://github.com/keymanapp/keyboards/issues/1399, but I solved that with custom CSS (as described below). I'm closing the keyboard-specific one and opening this general one.

Describe the bug I recently saw this on a user's phone (the user had the font set to XL in android phone settings on his Chinese-made phone), and this screenshot is from Android Studio Emulation on a Pixel 4 (system font size XL):

image

I realize that it may not always be possible to support all text sizes or stacked diacritics, but the space is there to display these if vertically centered properly.

I'm using Andika (Africa Subset), and I know that most SIL fonts have large [ginormous] line spacing to allow for a stack of diacritics.

I'm trying to create Andika Africa Compact with a more reasonable spacing that might help this. While in this process, Victor from WSTech said this:

Your problem, however, seems more like a Keyman bug. Keyman should not be basing positioning 
on vertical ascender values. In a keyboard display context there should be no difference at all 
between normal and 'compact' versions. This could bite other projects, too, including ones 
where there is no option for a 'compact' font variant.

Please file a bug report with the Keyman team.

To Reproduce

  1. Set your system font size to Extra Large on an android device (virtual or physical)
  2. Choose Charis SIL, Andika, or Doulos SIL as the OSK font.
  3. Run this keyboard on Android device or emulator.
  4. Notice that descenders are pushed off of the bottom of each key because of the excessive headroom.

Expected behavior I expect descenders to show on the keys, even if the font has "unusual" line spacing.

image

Keyman for Android:

**IOS*** I don't know if this affects iOS, but it is likely to.

Keyboard

Related context In a related issue, after setting the system font size, I had to reinstall the app/keyboard after setting android settings to get the font-size to change in Keyman (I didn't try rebooting). Keyman might need another refresh hook for system font size changes.

Suggested fix This is a conflict between .kmw-key-span's vertical align: middle and a font with extra headroom (intended to show stacking diacritics). For things to display correctly, you must: a. Use a "Compact" font, with default 1.0 line spacing. (I'm trying to build Andika Africa Compact for my case, but this requires using FontUtils to typetune the subset manually.) b. or override the line-spacing of your font in keyboard CSS. c. put a fix in kmwosk.css

Adding

.kmw-key span {
    ...
    line-spacing: 1;
}

to the Keyboard CSS or kmwosk.css solves the descender issues, but might cut off the top if someone added many stacking diacritics above a key, They would then have to override that specific key with a smaller font size.

MattGyverLee commented 3 years ago

The SIL Tchad and Burkina Keyboards are among the keyboards that should be affected by this.