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

key press popup text cut off #744

Merged
merged 5 commits into from
Apr 25, 2024
Merged

Conversation

codokie
Copy link
Contributor

@codokie codokie commented Apr 23, 2024

I changed the gravity of the key preview view and it seems to fix #712, #166 on my 393dpi phone.
I've also tried it out in android studio with simulated displays of 560dpi (pixel 7 pro) and xhdpi (small phone) and it still looks right without cutoffs of high characters such as 'p', 'g' or '['

@Helium314
Copy link
Owner

Thanks!
But the preview can have issues with letters with diacritics now...
E.g. Ü is cut off on top when using the same display settings that would previously cut off j on bottom.

@Helium314
Copy link
Owner

Helium314 commented Apr 23, 2024

Maybe setTextAndScaleX should actually check and scale the Y axis too? (though not when using this name)

@Helium314
Copy link
Owner

Really it does that on your phone? on mine Ü still seems whole following the change.

On the normal setting I use (S4 Mini, "display size" in LineageOS settings set to small) the dots are cut off half way. When setting to smaller, even normal uppercase letters are cut off on top.

But it should be possible to fix this quirk by other means

Maybe setting / scaling the text size? But I remember this has some quirks as well.

@Helium314
Copy link
Owner

Helium314 commented Apr 23, 2024

I had actually mean to dynamically scale the text size, only when necessary.
Because my impression is that it's always possible to find some device that has issues...

Anyway, I had a look at the different "display size" settings, and the issue is that the preview container scales with this setting, while keyboard and letter size does not: (smaller "display size" resp. higher dpi on top)
test

I think the preferable solution would be to make preview size independent of that setting, but so far I don't know how to properly choose the default size. (condsider that it should work in landscape mode too, and for tablets)
Maybe it would be enough to divide the size of the KeyPreviewView by screen density and maybe multiply with a constant factor?

display size smallest width density densityDpi preview background width
large 320 dp 1.675 279 59 px
default 360 dp 1.5 240 53 px
small 400 dp 1.35 216 47 px
smaller 449 dp 1.2 192 42 px

display size is from display settings, smallest width from developer settings, density and densityDpi from DisplayMetrics

@Helium314
Copy link
Owner

Works perfectly for me, and even gets rid of some code and a config parameter. Thanks!

The width still depends on that "display size" setting, but that's already the case and a rather minor issue.

@Helium314 Helium314 merged commit dc4cdea into Helium314:main Apr 25, 2024
1 check passed
@codokie codokie deleted the label-cutoff branch April 26, 2024 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

letters in key preview popups doesn't fit
2 participants