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): touch-layout fontsize property use ⛲ #10504

Merged
merged 1 commit into from
Jan 26, 2024

Conversation

jahorton
Copy link
Contributor

@jahorton jahorton commented Jan 25, 2024

Fixes #10444 for standard use cases. To reach "all use cases" will take a bit more work.

User Testing

TEST_BALOCHI_PHONETIC: Using the Keyman app on an iPhone - preferably on an iPhone SE (emulated or real) - verify that key caps are sufficiently large.

  1. Install the balochi_phonetic keyboard.
  2. Load the balochi_phonetic keyboard.
  3. Verify that multiple keys have key caps spanning nearly the entire width of their respective keys.
    • Good keys to inspect:
      • 'g': گ
      • 'k': ک
      • 'e': ے
      • 't': ت

Example of the keyboard properly rendered:

image

How it looks on the current master, for comparison:

image

It may be wise to run the test a second time, with the latest alpha build, for comparison.

(This keyboard's touch-layout requests a font-size boost of +40%.)

@keymanapp-test-bot keymanapp-test-bot bot added has-user-test user-test-required User tests have not been completed labels Jan 25, 2024
@keymanapp-test-bot
Copy link

keymanapp-test-bot bot commented Jan 25, 2024

User Test Results

Test specification and instructions

  • TEST_BALOCHI_PHONETIC (PASSED): Tested with the attached PR build (Keyman 17.0.251-alpha-test-10504 via TestFlight) in the iPhone 13 Mobile device and here is my observation: 1. Verified that the multiple keys have key caps spanning nearly the entire width of their respective keys. (notes)

Test Artifacts

@keymanapp-test-bot keymanapp-test-bot bot added this to the A17S31 milestone Jan 25, 2024
@jahorton jahorton changed the title fix(web): touch-layout fontsize property use fix(web): touch-layout fontsize property use ⛲ Jan 25, 2024
let gs = this.kbdDiv.style;
let bs = b.style;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The choice of variable names here wasn't particularly helpful.

We want to leave the layer-group font-size alone here - that's where the touch-layout fontsize property is applied, and why it was being overwritten.

Copy link
Member

@mcdurdin mcdurdin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM in principle, I think that makes sense.

@bharanidharanj
Copy link

Test Results

  • TEST_BALOCHI_PHONETIC (PASSED): Tested with the attached PR build (Keyman 17.0.251-alpha-test-10504 via TestFlight) in the iPhone 13 Mobile device and here is my observation: 1. Verified that the multiple keys have key caps spanning nearly the entire width of their respective keys.

@keymanapp-test-bot keymanapp-test-bot bot removed the user-test-required User tests have not been completed label Jan 25, 2024
@jahorton jahorton merged commit 5c5f5ac into master Jan 26, 2024
16 checks passed
@jahorton jahorton deleted the fix/web/layout-fontsize branch January 26, 2024 03:44
@keyman-server
Copy link
Collaborator

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

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

Successfully merging this pull request may close these issues.

bug(web): layout font size not being properly applied
5 participants