Skip to content
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): Increase size of spacebar text #9954

Merged
merged 3 commits into from
Nov 13, 2023

Conversation

ermshiperete
Copy link
Contributor

@ermshiperete ermshiperete commented Nov 7, 2023

Fixes #7940.

The caption on the spacebar used to be 60% (0.6em). This change increases it to 100%.

One reason for the sometimes small text on the spacebar described in #7940 besides the too small font size on kmw-spacebar-caption was that the calculation of the ideal font size added double padding: it added two pixels of
padding to the height, but also multiplied the height value by 0.9 which effectively added more padding. This change removes the 2 pixel padding from the calculation.

Note that it is still possible for the text on the spacebar to become unreasonably small if the keyboard/language name is too long. However, that is outside of the scope of this PR.

Screenshot from 2023-11-07 19-26-53

User Testing

TEST_EMULATOR:

  • Go to the "Prediction - robust testing" test page in Chrome.
  • Launch developer mode and set it to emulate an iPhone SE.
  • Set the emulation scale to better match the relative size to a physical handheld iPhone SE.
  • verify that the keyboard name on the spacebar has a reasonable size

TEST_HARDWARE:

  • Install Keyman on an Android or iPhone device
  • Open the Keyman app and install a Keyman keyboard
  • verify that the keyboard name on the spacebar has a reasonable size

TEST_WEB:

  • Go to the "Prediction - robust testing" test page in Chrome.
  • verify that the keyboard name on the spacebar has a reasonable size

The caption on the spacebar used to be 60%. This change increases it to
100%, but that might be sometimes too big. On the other hand, 80% still
seemed to be too small.

Fixes #7940.
@keymanapp-test-bot
Copy link

keymanapp-test-bot bot commented Nov 7, 2023

User Test Results

Test specification and instructions

  • TEST_EMULATOR (PASSED): Retested as per Marc's suggestion and here is my observation: 1. Opened test page and set it to emulation mode in Chrome browser. 2. Verified that the keyboard name on the space has a reasonable size. (notes)
  • TEST_HARDWARE (PASSED): Installed PR build in the Android Mobile device (Samsung Galaxy A23 5g) and here is my observation: 1. Verified that the keyboard name on the spacebar has a reasonable size. (notes)
  • TEST_WEB (PASSED): 1. Opened the "Prediction - robust testing" test page in Chrome browser. 2. Set it to emulation mode. 3. Verified that the keyboard name on the space has a reasonable size. (notes)

Test Artifacts

@keymanapp-test-bot keymanapp-test-bot bot added this to the A17S25 milestone Nov 7, 2023
@mcdurdin
Copy link
Member

mcdurdin commented Nov 7, 2023

This change increases it to 100%, but that might be sometimes too big. On the other hand, 80% still seemed to be too small.

I think we need to verify this across a range of keyboards and devices, because I expect that this is going to be too big for some of them. So we might need a cleverer solution, such as scaling it to fit?

@mcdurdin
Copy link
Member

mcdurdin commented Nov 7, 2023

I'm not also 100% sure that this is the entire cause of the tiny font size issue. The rest of the keyboard in the #7940 shows text at a sensible size, and it's only the spacebar which is mis-scaled. But all of the text elements on the keyboard use em scaling, so they should all be consistent in scale to each other?

@ermshiperete ermshiperete marked this pull request as draft November 7, 2023 06:41
@ermshiperete
Copy link
Contributor Author

Ah, you're right. It gets dynamically changed. Would have been too easy 😀.

One reason for the sometimes small text on the spacebar besides the
too small font size on `kmw-spacebar-caption` was that the calculation
of the ideal font size added double padding: it added two pixels of
padding to the height, but also multiplied the height value by 0.9 which
effectively added more padding. This change removes the 2 pixel padding
from the calculation.

The existing calculation varies the text size to fit in the available
width of the spacebar, so calculating with font size of 1em is reasonable
and better than the previous 0.6em. The previous value would lead to even
an smaller font size for long texts, far below the size that would be
sufficient to fit the text.
@ermshiperete
Copy link
Contributor Author

I think we need to verify this across a range of keyboards and devices, because I expect that this is going to be too big for some of them. So we might need a cleverer solution, such as scaling it to fit?

It already does downscaling so that the text will always fit in the available size. However, this can lead to the font size becoming unreasonably small if the language/keyboard name is too long, but that's a different issue.

@ermshiperete
Copy link
Contributor Author

I'm not also 100% sure that this is the entire cause of the tiny font size issue. The rest of the keyboard in the #7940 shows text at a sensible size, and it's only the spacebar which is mis-scaled. But all of the text elements on the keyboard use em scaling, so they should all be consistent in scale to each other?

I added another commit to address the underlying cause of #7940 and updated the PR description.

@ermshiperete ermshiperete marked this pull request as ready for review November 7, 2023 12:33
@bharanidharanj
Copy link

Test Results

  • TEST_EMULATOR (PASSED): Tested with the Keyman developer 17.0.207-alpha-test-9954 on Windows 10 OS and here is my observation: 1. Opened a Keyboard project. 2. Compiled it. Launched it in developer mode and set it to emulate an iPhone SE. 3. Verified that the keyboard name on the spacebar has a reasonable size.

..itransdevnagari keyboard

..silcameroonqwerty

..khmerangkor

  • TEST_HARDWARE (PASSED): Installed PR build in the Android Mobile device (Samsung Galaxy A23 5g) and here is my observation: 1. Verified that the keyboard name on the spacebar has a reasonable size.

  • TEST_WEB (PASSED): 1. Opened the "Prediction - robust testing" test page in Chrome browser. 2. Set it to emulation mode. 3. Verified that the keyboard name on the space has a reasonable size.

@keymanapp-test-bot keymanapp-test-bot bot removed the user-test-required User tests have not been completed label Nov 8, 2023
@mcdurdin
Copy link
Member

mcdurdin commented Nov 8, 2023

  • Launched it in developer mode and set it to emulate an iPhone SE.

@bharanidharanj, note that you need to reload the page after switching to Developer mode in Chrome (press Ctrl+R or F5). Otherwise, you get the desktop on screen keyboard in a phone form factor, which we can see is what has happened in your screenshots 😁.

@keymanapp-test-bot retest TEST_EMULATOR

@keymanapp-test-bot keymanapp-test-bot bot added the user-test-required User tests have not been completed label Nov 8, 2023
@bharanidharanj
Copy link

  • Launched it in developer mode and set it to emulate an iPhone SE.

@bharanidharanj, note that you need to reload the page after switching to Developer mode in Chrome (press Ctrl+R or F5). Otherwise, you get the desktop on screen keyboard in a phone form factor, which we can see is what has happened in your screenshots 😁.

@keymanapp-test-bot retest TEST_EMULATOR

Thanks for the Clarification, @mcdurdin . I will retest and add my comment.

@bharanidharanj
Copy link

Test Results

  • TEST_EMULATOR (PASSED): Retested as per Marc's suggestion and here is my observation: 1. Opened test page and set it to emulation mode in Chrome browser. 2. Verified that the keyboard name on the space has a reasonable size.

..English Keyboard

..English - EuroLatin(SIL) Keyboard

@keymanapp-test-bot keymanapp-test-bot bot removed the user-test-required User tests have not been completed label Nov 9, 2023
@mcdurdin mcdurdin modified the milestones: A17S25, A17S26 Nov 13, 2023
@ermshiperete ermshiperete merged commit 75a5d36 into master Nov 13, 2023
15 checks passed
@ermshiperete ermshiperete deleted the fix/web/7940_spacebar-text branch November 13, 2023 07:08
@keyman-server
Copy link
Collaborator

Changes in this pull request will be available for download in Keyman version 17.0.209-alpha

@MayuraVerma
Copy link
Contributor

MayuraVerma commented Nov 15, 2023

Also the labels for alpha/number special keys, shift key, return key and global icon and text size should be increased

image

image

@mcdurdin
Copy link
Member

Also the labels for alpha/number special keys, shift key, return key and global icon and text size should be increased

@MayuraVerma this needs to be a separate issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug(web): spacebar text sometimes overly small without good reason
5 participants