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 PostItem scaling #382

Merged
merged 3 commits into from
Feb 7, 2025

Conversation

eylulnc
Copy link
Contributor

@eylulnc eylulnc commented Feb 3, 2025

Problem Description

When the user adjusts the system font size (for accessibility or personal preference), certain text elements—especially those rendered via our custom MarkdownText composable—do not scale properly. This leads to inconsistent UI: standard Compose Text elements scale up/down, but the markdown text remains the same size.

Changes

  1. Fixed Markdown Text Scaling

    Replaced TypedValue.COMPLEX_UNIT_DIP with TypedValue.COMPLEX_UNIT_SP in our MarkdownText composable.
    Ensured that any text size set on the underlying TextView is in scale‐independent pixels (sp), so it respects the user’s font‐size setting.

  2. Removed Fixed DP Constraints

    Updated components (like HeadlineProfilePicture) that had hard‐coded dp sizes, allowing them to scale if desired or remain flexible in layout.

Steps for testing

Go to the device’s Settings → Display → Font size and select “Largest.” ( Also among other font settings)
Return to App and open a communication channel
Check in the channel Post Header is no longer clipped and the post content size is also updated according to the font size you arranged.
Check that the profile picture also resizes to align with the new header height.

Screenshots

Regular and Largest font size

Screenshot 2025-02-03 at 23 39 33 Screenshot 2025-02-03 at 23 39 00

@eylulnc eylulnc self-assigned this Feb 3, 2025
@eylulnc eylulnc linked an issue Feb 3, 2025 that may be closed by this pull request
@eylulnc eylulnc added the ready for review This PR can be reviewed label Feb 3, 2025
@eylulnc eylulnc changed the title Bugfix: Fix PostItem scaling issue Bugfix: Fix PostItem scaling Feb 3, 2025
Copy link

@ahbitaqu ahbitaqu left a comment

Choose a reason for hiding this comment

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

Nice addition! I am experiencing a rendering issue with the reactions though after resizing the font

grafik

@eylulnc
Copy link
Contributor Author

eylulnc commented Feb 4, 2025

Emoji's should not be cutted out now. The icon button on the other hand is not someth
ing that can be scaled like the text in this context. There is of course a work around (scale components height or size by multiplying the fontScale) but I do not find it sustainable.

Screenshot 2025-02-04 at 19 46 56 Screenshot 2025-02-04 at 19 46 41

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.

Changes and code lgtm. Thank you for tackling this! :D

Copy link
Contributor

@julian-wls julian-wls left a comment

Choose a reason for hiding this comment

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

Really nice, works as expected!
Code lgtm

@FelberMartin FelberMartin added ready to merge This PR can be merged and removed ready for review This PR can be reviewed ready to merge This PR can be merged labels Feb 7, 2025
@FelberMartin
Copy link
Collaborator

@eylulnc There is one merge conflict, once that one is fixed, feel free to add the "Ready to merge" label :)

…into bugfix/communication/fix-post-scaling

# Conflicts:
#	feature/metis/conversation/src/main/kotlin/de/tum/informatics/www1/artemis/native_app/feature/metis/conversation/ui/post/PostItem.kt
@eylulnc eylulnc added the ready to merge This PR can be merged label Feb 7, 2025
@FelberMartin FelberMartin merged commit 993b2c1 into develop Feb 7, 2025
5 checks passed
@FelberMartin FelberMartin deleted the bugfix/communication/fix-post-scaling branch February 7, 2025 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge This PR can be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Address font scaling issue in posts
4 participants