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

Bugfix: Fix emoji icon scaling #408

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

julian-wls
Copy link
Contributor

Problem Description

Currently, the icon for the add emoji button seems a bit too big and not aligned, see screenshot:
image

It looks fine when the text size is bigger than usual, but not with the normal scaling.

Changes

This PR changes the height of the emoji to use .sp to be affected from the font changes and therefore be aligned with the other emoji chips.

Steps for testing

Make sure it looks good before and after changing the font size.

@julian-wls julian-wls self-assigned this Feb 12, 2025
@julian-wls julian-wls changed the title Bugfix: Fixed emoji icon scaling Bugfix: Fix emoji icon scaling Feb 12, 2025
@julian-wls julian-wls added the ready for review This PR can be reviewed label Feb 12, 2025
Copy link
Collaborator

@FelberMartin FelberMartin left a comment

Choose a reason for hiding this comment

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

Nice change, there are two issues I noticed:

  1. For small font sizes, the emojis and counter seem to be not centerred anymore
    image
  2. For very large font sizes, the icon is still a little bit smaller than the emojis
    image

@julian-wls
Copy link
Contributor Author

julian-wls commented Feb 13, 2025

Nice change, there are two issues I noticed:

  1. For small font sizes, the emojis and counter seem to be not centered anymore
  1. For very large font sizes, the icon is still a little bit smaller than the emojis

Good catch, I managed to fix it for the very small font sizes.
For the large font sizes, I'm not quite sure, when selecting the largest it seems aligned correctly for me:
image

@FelberMartin
Copy link
Collaborator

For the large font sizes, I'm not quite sure, when selecting the largest it seems aligned correctly for me

Hm interesting, I re-tested on both foldable and physical device, and in both cases the icon size was off for the largest font size🤔

Recording.2025-02-13.112544.mp4

@julian-wls
Copy link
Contributor Author

For the large font sizes, I'm not quite sure, when selecting the largest it seems aligned correctly for me

Hm interesting, I re-tested on both foldable and physical device, and in both cases the icon size was off for the largest font size🤔

Recording.2025-02-13.112544.mp4

Oh, okay. I was using the font scaling in the accessibility options of the Android system settings app. There seem to be only 5 different text sizes, which all look good now.

@FelberMartin
Copy link
Collaborator

Uh good to know. I just checked in the Android settings, there are two different scales. Display size apparently controls the .dp scalings, and font size the .sp scaled elements.
image

@julian-wls
Copy link
Contributor Author

julian-wls commented Feb 17, 2025

Oh, I only had 4 steps on the font scale (API 33):

image

I tested on API 35, and it works with the ultra big font size now :)
The solution was to change the padding of the box to also use .sp.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for review This PR can be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants