Introduce auto sizing for all app bar title components #891

mannodermaus · 2024-08-30T23:49:51Z

Issue

close When the text and display size is increased, the text in AnimatedTextTopAppBar may be partially hidden. #757

Overview (Required)

Introduce AutoSizeText, a component that automatically finds the correct size to fit a text into a box
- The maximum font size is defined by the style of the text, but it will shrink if necessary
- It uses a binary search to find the biggest size that fits in a given number of lines
Apply it to all titles in every top app bar of the app
- The custom app bar of TimetableScreen
- AnimatedTextTopAppBar (fix for the main tabs on the home screen)
- AnimatedMediumTopAppBar (fix for contributor, sponsor, staff, settings screens)

Movie (Optional)

Before

Tabs (Large Font)	Tabs (Normal Font)	Contributor Screen
before_1725061103.mp4	before2_1725061142.mp4	before3_1725061201.mp4

After

Tabs (Large Font)	Tabs (Normal Font)	Contributor Screen
after_1725060593.mp4	after2_1725060638.mp4	after3_1725060970.mp4

github-actions · 2024-08-30T23:51:31Z

Detekt check failed. Please run ./gradlew detekt --auto-correct to fix the issues.

takahirom · 2024-09-01T08:02:51Z

.../src/commonMain/kotlin/io/github/droidkaigi/confsched/designsystem/component/AutoSizeText.kt

+ * of the text size, use [TextUnit.Unspecified] for that parameter.)
+ *
+ * Adapted from Alchitry on Stack Overflow, licensed under CC BY-SA 4.0:
+ * https://stackoverflow.com/a/71416530


It's tough to know if this is safe. I think we might have to publish our app code under CC BY-SA 4.0 as well, which makes using our app's code difficult.

That's fair. I can reimplement the same solution by hand, but with a different API. Would you prefer that, or shall we skip the adjustments for bigger font sizes?

We might need to reimplement this. I wish we had a library for it. 😅
If there isn't a licensing issue, I'll leave it up to you to implement this, as you are reliable.

I reimplemented AutoSizeText with the same idea, but without コピペ, in 24a66c0

takahirom · 2024-09-02T11:55:34Z

We introduced the MediumTopAppBar. Does it affect this PR?
#753

mannodermaus · 2024-09-02T12:03:28Z

Yes, I believe it does. Let me rebase and do another pass on this!

mannodermaus · 2024-09-02T12:14:40Z

@takahirom Adjusted the MediumTopAppBar in 0eae2c3.

	Normal (no change)		Large
Before
After

takahirom · 2024-09-02T12:50:16Z

.../src/commonMain/kotlin/io/github/droidkaigi/confsched/designsystem/component/AutoSizeText.kt

+ * with the upper bound being defined by the font size of its [style].
+ */
+@Composable
+fun AutoSizeText(


Looks great!

takahirom · 2024-09-02T12:50:36Z

.../src/commonMain/kotlin/io/github/droidkaigi/confsched/designsystem/component/AutoSizeText.kt

+
+    // Keep decreasing the font size until the text fits in the box without exceeding the max lines
+    while (paragraph.didExceedMaxLines || maxHeight < paragraph.height.toDp() || maxWidth < paragraph.minIntrinsicWidth.toDp()) {
+        targetFontSize *= 0.95


If you are interested, we might be able to use binary search for this. 😄

Fun idea! Added in 612af1c

takahirom · 2024-09-02T12:51:22Z

Thank you for your comprehensive screenshots.
Could we also have a test using setFontScale?
https://github.com/DroidKaigi/conference-app-2024/blob/main/core/testing/src/main/java/io/github/droidkaigi/confsched/testing/robot/MiniRobots.kt#L158

mannodermaus · 2024-09-03T11:52:09Z

It took a bit to get the line count assertion for text nodes just right, but I managed to pull it off in e0b7d8b. From now on, we can assert the line count of any text based on expectations!

github-actions · 2024-09-03T12:06:07Z

Snapshot diff report

File name	Image
TimetableScreenTest[ TimetableScreen - wh en font scale is lar ge - it should show title in a single li ne]_compare.png

takahirom · 2024-09-03T15:29:23Z

.../src/commonMain/kotlin/io/github/droidkaigi/confsched/designsystem/component/AutoSizeText.kt

+        Text(
+            text = text,
+            color = color,
+            fontSize = calculateFontSize(text, style, color, textAlign, maxLines),


I'm not sure, but could we consider using remember{} caching for this? If so, it might lead to a open source library-level optimization. 👍

Sorry, I forgot that we can't use Composable function in remember lambda.👀

It should be possible to rearrange this to work. Let me think about this tonight!

Turns out that Paragraph() does not require any composable context. 👀 be532bb

…l main screens

This required a new type of matcher for asserting the line count of a text node

.../src/commonMain/kotlin/io/github/droidkaigi/confsched/designsystem/component/AutoSizeText.kt

takahirom · 2024-09-04T11:53:21Z

Looks awesome! I want to share this, and for the people who want to use this implementation, could you update the pull request content to match the current code?

…onfsched/designsystem/component/AutoSizeText.kt Co-authored-by: Takahiro Menju <[email protected]>

mannodermaus · 2024-09-04T22:53:10Z

I updated the description to reference the new implementation. @takahirom please let me know if you'd like me to change something else about the PR description. 🙇‍♂️

takahirom

Thank you for your valuable contribution!

mannodermaus force-pushed the fix/top-bar-text-break-in-large-font-size branch from f0f32a0 to f15a996 Compare August 30, 2024 23:50

github-actions bot temporarily deployed to deploygate-distribution August 30, 2024 23:54 Inactive

mannodermaus force-pushed the fix/top-bar-text-break-in-large-font-size branch from f15a996 to 596faa8 Compare August 30, 2024 23:55

github-actions bot temporarily deployed to deploygate-distribution August 30, 2024 23:59 Inactive

mannodermaus marked this pull request as ready for review August 31, 2024 00:12

takahirom reviewed Sep 1, 2024

View reviewed changes

takahirom added the Waiting for response or commit label Sep 1, 2024

github-actions bot removed the Waiting for response or commit label Sep 1, 2024

takahirom added the Waiting for response or commit label Sep 1, 2024

mannodermaus force-pushed the fix/top-bar-text-break-in-large-font-size branch from 596faa8 to 4572700 Compare September 2, 2024 09:47

github-actions bot removed the Waiting for response or commit label Sep 2, 2024

github-actions bot temporarily deployed to deploygate-distribution September 2, 2024 09:51 Inactive

mannodermaus requested a review from takahirom September 2, 2024 09:54

github-actions bot temporarily deployed to deploygate-distribution September 2, 2024 11:58 Inactive

github-actions bot temporarily deployed to deploygate-distribution September 2, 2024 12:15 Inactive

takahirom reviewed Sep 2, 2024

View reviewed changes

takahirom added the Waiting for response or commit label Sep 2, 2024

mannodermaus force-pushed the fix/top-bar-text-break-in-large-font-size branch from 0eae2c3 to b0817f9 Compare September 3, 2024 02:37

github-actions bot removed the Waiting for response or commit label Sep 3, 2024

github-actions bot temporarily deployed to deploygate-distribution September 3, 2024 02:41 Inactive

mannodermaus force-pushed the fix/top-bar-text-break-in-large-font-size branch from b0817f9 to e0b7d8b Compare September 3, 2024 11:51

mannodermaus requested a review from takahirom September 3, 2024 11:52

github-actions bot temporarily deployed to deploygate-distribution September 3, 2024 11:58 Inactive

takahirom added the awesome Label for project-applicable insights or remarkable technologies. label Sep 3, 2024

takahirom reviewed Sep 3, 2024

View reviewed changes

mannodermaus added 7 commits September 4, 2024 19:24

Introduce AutoSizeText component & use it for the top bar title of al…

50e7cdb

…l main screens

Apply AutoSizeText to other title bars in the app

0e3f70f

Apply AutoTextSize to the new AnimatedMediumTopAppBar

1d3ecb6

Find the best font size for AutoTextSize using binary search, if needed

a01e906

Add font scale test for AutoSizeText to timetable screen

9c82a2e

This required a new type of matcher for asserting the line count of a text node

Small comment fix

9b7456b

Remember the calculated font size to avoid unnecessary recalculation

be532bb

mannodermaus force-pushed the fix/top-bar-text-break-in-large-font-size branch from 285de21 to be532bb Compare September 4, 2024 10:24

mannodermaus requested a review from takahirom September 4, 2024 10:25

github-actions bot temporarily deployed to deploygate-distribution September 4, 2024 10:28 Inactive

takahirom reviewed Sep 4, 2024

View reviewed changes

.../src/commonMain/kotlin/io/github/droidkaigi/confsched/designsystem/component/AutoSizeText.kt Outdated Show resolved Hide resolved

Update core/designsystem/src/commonMain/kotlin/io/github/droidkaigi/c…

d0ad0ff

…onfsched/designsystem/component/AutoSizeText.kt Co-authored-by: Takahiro Menju <[email protected]>

github-actions bot temporarily deployed to deploygate-distribution September 4, 2024 22:54 Inactive

takahirom approved these changes Sep 5, 2024

View reviewed changes

takahirom merged commit e2b3d10 into DroidKaigi:main Sep 5, 2024
6 checks passed

mannodermaus deleted the fix/top-bar-text-break-in-large-font-size branch September 5, 2024 01:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce auto sizing for all app bar title components #891

Introduce auto sizing for all app bar title components #891

mannodermaus commented Aug 30, 2024 •

edited

Loading

github-actions bot commented Aug 30, 2024

takahirom Sep 1, 2024 •

edited

Loading

mannodermaus Sep 1, 2024

takahirom Sep 1, 2024

mannodermaus Sep 2, 2024

takahirom commented Sep 2, 2024

mannodermaus commented Sep 2, 2024

mannodermaus commented Sep 2, 2024

takahirom Sep 2, 2024

takahirom Sep 2, 2024

mannodermaus Sep 3, 2024

takahirom commented Sep 2, 2024

mannodermaus commented Sep 3, 2024

github-actions bot commented Sep 3, 2024

takahirom Sep 3, 2024

takahirom Sep 3, 2024

mannodermaus Sep 4, 2024

mannodermaus Sep 4, 2024

takahirom commented Sep 4, 2024

mannodermaus commented Sep 4, 2024

takahirom left a comment

Introduce auto sizing for all app bar title components #891

Introduce auto sizing for all app bar title components #891

Conversation

mannodermaus commented Aug 30, 2024 • edited Loading

Issue

Overview (Required)

Movie (Optional)

Before

After

github-actions bot commented Aug 30, 2024

takahirom Sep 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

takahirom commented Sep 2, 2024

mannodermaus commented Sep 2, 2024

mannodermaus commented Sep 2, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

takahirom commented Sep 2, 2024

mannodermaus commented Sep 3, 2024

github-actions bot commented Sep 3, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

takahirom commented Sep 4, 2024

mannodermaus commented Sep 4, 2024

takahirom left a comment

Choose a reason for hiding this comment

mannodermaus commented Aug 30, 2024 •

edited

Loading

takahirom Sep 1, 2024 •

edited

Loading