-
Notifications
You must be signed in to change notification settings - Fork 206
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
Make session grid item title font size variable #563
Changes from 8 commits
b2b8322
3f18973
1751ec4
d3f90ce
999450d
5497152
3063284
b071334
f8094f7
a1f29be
84578b6
4f3ee6b
97421f1
782d290
fe8b5d9
45b14ce
5bfb94f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,9 +24,14 @@ import androidx.compose.ui.Modifier | |
import androidx.compose.ui.draw.clip | ||
import androidx.compose.ui.graphics.Color | ||
import androidx.compose.ui.graphics.vector.rememberVectorPainter | ||
import androidx.compose.ui.platform.LocalDensity | ||
import androidx.compose.ui.platform.testTag | ||
import androidx.compose.ui.text.TextStyle | ||
import androidx.compose.ui.tooling.preview.Preview | ||
import androidx.compose.ui.unit.Density | ||
import androidx.compose.ui.unit.TextUnit | ||
import androidx.compose.ui.unit.dp | ||
import androidx.compose.ui.unit.sp | ||
import io.github.droidkaigi.confsched2023.designsystem.theme.KaigiTheme | ||
import io.github.droidkaigi.confsched2023.designsystem.theme.room_hall_a | ||
import io.github.droidkaigi.confsched2023.designsystem.theme.room_hall_b | ||
|
@@ -40,21 +45,31 @@ import io.github.droidkaigi.confsched2023.model.RoomIndex.Room4 | |
import io.github.droidkaigi.confsched2023.model.RoomIndex.Room5 | ||
import io.github.droidkaigi.confsched2023.model.TimetableItem | ||
import io.github.droidkaigi.confsched2023.model.TimetableItem.Session | ||
import io.github.droidkaigi.confsched2023.model.TimetableSpeaker | ||
import io.github.droidkaigi.confsched2023.model.fake | ||
import io.github.droidkaigi.confsched2023.model.type | ||
import io.github.droidkaigi.confsched2023.sessions.SessionsStrings.ScheduleIcon | ||
import io.github.droidkaigi.confsched2023.sessions.SessionsStrings.UserIcon | ||
import io.github.droidkaigi.confsched2023.sessions.section.TimetableSizes | ||
import io.github.droidkaigi.confsched2023.ui.previewOverride | ||
import io.github.droidkaigi.confsched2023.ui.rememberAsyncImagePainter | ||
import kotlinx.datetime.DateTimeUnit | ||
import kotlinx.datetime.minus | ||
import kotlin.math.roundToInt | ||
|
||
const val TimetableGridItemTestTag = "TimetableGridItem" | ||
|
||
@Composable | ||
fun TimetableGridItem( | ||
timetableItem: TimetableItem, | ||
onTimetableItemClick: (TimetableItem) -> Unit, | ||
gridItemHeightPx: Int, | ||
modifier: Modifier = Modifier, | ||
) { | ||
val localDensity = LocalDensity.current | ||
|
||
val speaker = timetableItem.speakers.firstOrNull() | ||
|
||
val backgroundColor = when (timetableItem.room.type) { | ||
Room1 -> room_hall_a | ||
Room2 -> room_hall_b | ||
|
@@ -63,6 +78,19 @@ fun TimetableGridItem( | |
Room5 -> room_hall_e | ||
else -> Color.White | ||
} | ||
val height = with(localDensity) { gridItemHeightPx.toDp() } | ||
val titleTextStyle = MaterialTheme.typography.labelLarge.let { | ||
check(it.fontSize.isSp) | ||
val titleFontSize = calculateTitleFontSize( | ||
textStyle = it, | ||
localDensity = localDensity, | ||
gridItemHeightPx = gridItemHeightPx, | ||
speaker = speaker, | ||
titleLength = timetableItem.title.currentLangTitle.length, | ||
) | ||
it.copy(fontSize = titleFontSize, color = Color.White) | ||
} | ||
|
||
Box(modifier.testTag(TimetableGridItemTestTag)) { | ||
Box( | ||
modifier = Modifier | ||
|
@@ -72,6 +100,7 @@ fun TimetableGridItem( | |
shape = RoundedCornerShape(4.dp), | ||
) | ||
.width(192.dp) | ||
.height(height) | ||
.clickable { | ||
onTimetableItemClick(timetableItem) | ||
} | ||
|
@@ -80,7 +109,7 @@ fun TimetableGridItem( | |
Column { | ||
Text( | ||
text = timetableItem.title.currentLangTitle, | ||
style = MaterialTheme.typography.labelLarge.copy(Color.White), | ||
style = titleTextStyle, | ||
) | ||
Spacer(modifier = Modifier.height(4.dp)) | ||
Row(modifier = Modifier.height(16.dp)) { | ||
|
@@ -99,7 +128,6 @@ fun TimetableGridItem( | |
Spacer(modifier = Modifier.height(16.dp)) | ||
|
||
// TODO: Dealing with more than one speaker | ||
val speaker = timetableItem.speakers.firstOrNull() | ||
if (speaker != null) { | ||
Row( | ||
modifier = Modifier.height(32.dp), | ||
|
@@ -124,6 +152,50 @@ fun TimetableGridItem( | |
} | ||
} | ||
|
||
/** | ||
* | ||
* Calculate the font size of the title by the height of the displayed session grid. | ||
* | ||
* @param textStyle session title text style. | ||
* @param localDensity local density. | ||
* @param gridItemHeightPx session grid item height. (unit is px.) | ||
* @param speaker session speaker. | ||
* @param titleLength session title length. | ||
* | ||
*/ | ||
private fun calculateTitleFontSize( | ||
textStyle: TextStyle, | ||
localDensity: Density, | ||
gridItemHeightPx: Int, | ||
speaker: TimetableSpeaker?, | ||
titleLength: Int, | ||
): TextUnit { | ||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Ask For Option] I set up the logic as follows.
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.🙇♂️ |
||
|
||
val oneLine = 1 | ||
val minFontSize = 9.sp | ||
return when { | ||
displayTitleHeight <= 0 -> minFontSize | ||
rows <= oneLine || displayTitleHeight > actualTitleHeight -> textStyle.fontSize | ||
else -> minFontSize | ||
} | ||
} | ||
|
||
@Preview | ||
@Composable | ||
fun PreviewTimetableGridItem() { | ||
|
@@ -132,6 +204,36 @@ fun PreviewTimetableGridItem() { | |
TimetableGridItem( | ||
timetableItem = Session.fake(), | ||
onTimetableItemClick = {}, | ||
gridItemHeightPx = 350, | ||
) | ||
} | ||
} | ||
} | ||
|
||
@Preview | ||
@Composable | ||
fun PreviewTimetableGridLongTitleItem() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can I see how the text becomes small? In this preview, we see normal-size text right? 🙏 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @shxun6934 Thanks for your quick action! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @takahirom @mhidaka the fix commit is b071334 A snapshot in GithubActions looks like this: Is there any problem here? |
||
val fake = Session.fake() | ||
|
||
val localDensity = LocalDensity.current | ||
val verticalScale = 1f | ||
|
||
val minutePx = with(localDensity) { TimetableSizes.minuteHeight.times(verticalScale).toPx() } | ||
val displayEndsAt = fake.endsAt.minus(1, DateTimeUnit.MINUTE) | ||
val height = ((displayEndsAt - fake.startsAt).inWholeMinutes * minutePx).roundToInt() | ||
|
||
KaigiTheme { | ||
Surface { | ||
TimetableGridItem( | ||
timetableItem = Session.fake().let { | ||
val longTitle = it.title.copy( | ||
jaTitle = it.title.jaTitle.repeat(2), | ||
enTitle = it.title.enTitle.repeat(2), | ||
) | ||
it.copy(title = longTitle) | ||
}, | ||
onTimetableItemClick = {}, | ||
gridItemHeightPx = height, | ||
) | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, making
private val titleToScheduleSpaceHeight = 4.dp
would be better 👍There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've made it possible to manage with constants! 🙇♂️
I am referring to TimetableSizes.
the fix commit is a1f29be