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

Font size wrong on All Chats header text #1572

Closed
kongo09 opened this issue Oct 14, 2023 · 7 comments · Fixed by #2073
Closed

Font size wrong on All Chats header text #1572

kongo09 opened this issue Oct 14, 2023 · 7 comments · Fixed by #2073
Assignees
Labels
O-Occasional Affects or can be seen by some users regularly or most users rarely S-Tolerable Low/no impact on users T-Defect Something isn't working: bugs, crashes, hangs and other reported problems

Comments

@kongo09
Copy link

kongo09 commented Oct 14, 2023

Steps to reproduce

  1. Start EX
  2. Slowly move roomlist up
  3. Observe how All Chats appears in the header

Outcome

What did you expect?

Text to fade in

What happened instead?

Text appears briefly in large font size, jumps to small and then fades in

screen-20231014-093024.2.mp4

Your phone model

Pixel 5

Operating system version

Android 14

Application version and app store

latest nightly

Homeserver

No response

Will you send logs?

No

Are you willing to provide a PR?

No

@kongo09 kongo09 added the T-Defect Something isn't working: bugs, crashes, hangs and other reported problems label Oct 14, 2023
@jmartinesp
Copy link
Member

That's just what the Android component ( MediumTopAppBar) does by default. To improve this we'd have to create our own component from scratch.

Here's a screenshot from a GIF found in the wild where the 2 states (collapsed and expanded) can be seen at the same time:

Image

@jmartinesp jmartinesp added S-Tolerable Low/no impact on users O-Occasional Affects or can be seen by some users regularly or most users rarely labels Oct 18, 2023
@kongo09
Copy link
Author

kongo09 commented Oct 18, 2023

It might not be so much about being seen at the same time, but more about the font size that starts big and then jumps to small.

@bmarty
Copy link
Member

bmarty commented Dec 19, 2023

Won't fix

@bmarty bmarty closed this as not planned Won't fix, can't repro, duplicate, stale Dec 19, 2023
@kongo09
Copy link
Author

kongo09 commented Dec 19, 2023

Not sure I can follow your reasoning. If I look in the documentation, there is not jumping around, the font size is always consistent.
https://developer.android.com/jetpack/compose/components/app-bars

appbar-scroll.mp4

Can you please take another look. Something else must be going on.

The legacy app doesn't exhibit this jumping problem either and the Android settings also don't show it.

MediumTopAppBar.mp4

@kongo09 kongo09 reopened this Dec 19, 2023
@kongo09
Copy link
Author

kongo09 commented Dec 19, 2023

@nadonomy or do I miss something from a design perspective?

@jmartinesp
Copy link
Member

jmartinesp commented Dec 20, 2023

Actually after taking another look, I think @kongo09 is right here: there is a bug that causes the title to change its size, introduced here.

We have to do this because the underlying Material 3 component doesn't allow us to customise the text styles used for large/small title other than modifying MaterialTheme.typography.headlineSmall and MaterialTheme.typography.titleLarge, which could affect other components, although I think (it needs some double checking) only dialog titles would be affected, so it might be something we can just do.

If we wanted to both have the right text styles for dialogs and not affect any other components we might be able to do this by providing a custom MaterialTheme with the material typographies changed using LocalCompositionProvider only for this component wrapping this component.

Ideally, the solution would be that Google would just allow us to override these values just for this component, but that needs a feature request, and for it to actually get done an released, so it would take quite some time even if they decided they actually want to allow it.

@nadonomy
Copy link

+1 to fixing this - this is plainly a visual bug. If it needs prioritising against other work that's one thing but this for sure shouldn't be a 'won't fix'.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-Occasional Affects or can be seen by some users regularly or most users rarely S-Tolerable Low/no impact on users T-Defect Something isn't working: bugs, crashes, hangs and other reported problems
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants