Make session grid item title font size variable #563
Conversation
shxun6934
changed the title
shxun6934
changed the title
| val actualTitleHeight = rows * fontSizePx | ||
|
|
||
| val oneLine = 1 | ||
| val minFontSize = 8.sp |
There was a problem hiding this comment.
[Help]
We really have to calculate the appropriate size according to the session title length.
However, since the logic did not come to mind, a fixed minimum value is set.
There was a problem hiding this comment.
I was able to assemble a similar logic, but there is room for improvement.
|
Hi @shxun6934! Codes seem to be unformatted. To resolve this issue, please run |
| val titleToScheduleSpaceHeight = with(localDensity) { 4.dp.toPx() } | ||
| val scheduleHeight = with(localDensity) { 16.dp.toPx() } | ||
| val scheduleToSpeakerSpaceHeight = with(localDensity) { 16.dp.toPx() } | ||
| val horizontalPadding = with(localDensity) { (12 * 2).dp.toPx() } | ||
|
|
||
| // The height of the title that should be displayed. | ||
| var displayTitleHeight = | ||
| gridItemHeightPx - titleToScheduleSpaceHeight - scheduleHeight - scheduleToSpeakerSpaceHeight - horizontalPadding | ||
| displayTitleHeight -= if (speaker != null) with(localDensity) { 32.dp.toPx() } else 0f | ||
|
|
||
| // Actual height of displayed title. | ||
| val boxWidthWithoutPadding = with(localDensity) { (192 - 12 * 2).dp.toPx() } | ||
| val fontSizePx = with(localDensity) { textStyle.fontSize.toPx() } | ||
| val textLengthInRow = (boxWidthWithoutPadding / fontSizePx).roundToInt() | ||
| val rows = titleLength / textLengthInRow | ||
| val actualTitleHeight = rows * fontSizePx |
There was a problem hiding this comment.
[Ask For Option]
I set up the logic as follows.
- I checked the height of the Box that displays the session title. (Lines 170-178 of the code)
- Based on the actual length of the session title, default font size, and width of the Box, I check whether it fits within the height I checked earlier.
- I set the default font size if it fits, or set the minimum font size if it doesn't fit.
It seems to be a little easier to calculate, but I couldn't come up with an opinion myself.
If you don't mind, I would appreciate it if you could give me your opinion.🙇♂️
|
|
||
| @Preview | ||
| @Composable | ||
| fun PreviewTimetableGridLongTitleItem() { |
There was a problem hiding this comment.
Can I see how the text becomes small? In this preview, we see normal-size text right? 🙏
There was a problem hiding this comment.
Thank you for pointing it out!
In my environment, I confirmed that the text size is small, but it seems that it is not so in the snapshot in GithubActions, so I will check it!
There was a problem hiding this comment.
@shxun6934 Thanks for your quick action!
It might be a reply after tomorrow(today) morning 😉
There was a problem hiding this comment.
@takahirom @mhidaka
Good morning!
the fix commit is b071334
A snapshot in GithubActions looks like this:
Is there any problem here?
…feature/447/resize_session_title_by_height
| speaker: TimetableSpeaker?, | ||
| titleLength: Int, | ||
| ): TextUnit { | ||
| val titleToScheduleSpaceHeight = with(localDensity) { 4.dp.toPx() } |
There was a problem hiding this comment.
Can we make it const like private val titleToScheduleSpaceHeight =..? to maintain this 👀
Or I'm thinking if we can use Layout() composable. Did you think it? This could be rather complex so I'm not sure it is better 👀
There was a problem hiding this comment.
Can we make it const like private val titleToScheduleSpaceHeight =..? to maintain this 👀
I'm thinking if we can use Layout() composable. Did you think it? This could be rather complex so I'm not sure it is better 👀
I see !
Layout composable can certainly be implemented, but I haven't touched it, so it looks like it will take a lot of time.
So, for the time being, we will make the size used for layout a constant while keeping the current implementation.
And, I will try a little with reference to the following documents and blogs.
There was a problem hiding this comment.
Yeah, making private val titleToScheduleSpaceHeight = 4.dp would be better 👍
There was a problem hiding this comment.
We've made it possible to manage with constants! 🙇♂️
I am referring to TimetableSizes.
the fix commit is a1f29be
…feature/447/resize_session_title_by_height
| gridItemHeightPx: Int, | ||
| speaker: TimetableSpeaker?, | ||
| titleLength: Int, | ||
| ): Pair<TextUnit, TextUnit> { |
There was a problem hiding this comment.
Made not only fontSize but also lineHeight variable.
| while (displayTitleHeight <= actualTitleHeight) { | ||
| if (fontResizePx <= minFontSizePx) { | ||
| fontResizePx = minFontSizePx | ||
| lineHeightResizePx = minLineHeightPx | ||
| break | ||
| } | ||
|
|
||
| fontResizePx -= with(localDensity) { 1.sp.toPx() } | ||
| val fontResize = with(localDensity) { fontResizePx.toSp() } | ||
| if (fontResize <= 12.sp) { | ||
| lineHeightResizePx = middleLineHeightPx | ||
| } else if (fontResize <= 10.sp) { | ||
| lineHeightResizePx = minLineHeightPx | ||
| } | ||
| actualTitleHeight = calculateTitleHeight( | ||
| fontSizePx = fontResizePx, | ||
| lineHeightPx = lineHeightResizePx, | ||
| titleLength = titleLength, | ||
| maxWidth = boxWidthWithoutPadding, | ||
| ) | ||
| } |
There was a problem hiding this comment.
Added logic to vary font size and lineHeight until it fits inside the Box.
reference
…feature/447/resize_session_title_by_height
| return fontSizePx + (lineHeightPx * (rows - 1f)) | ||
| } | ||
|
|
||
| object TimetableGridItemSizes { |
|
Oh, we have a conflict. Could you solve it? |
…feature/447/resize_session_title_by_height
Thank you for pointing out. Since the conflict has been resolved, please confirm. 🙇♂️ |
Issue
Overview (Required)
Links
Screenshot
en
Screen_recording_20230817_235512.mp4
Screen_recording_20230818_000041.mp4
ja
Screen_recording_20230817_235624.mp4
Screen_recording_20230817_235837.mp4