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

Make session grid item title font size variable #563

Merged
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -72,6 +100,7 @@ fun TimetableGridItem(
shape = RoundedCornerShape(4.dp),
)
.width(192.dp)
.height(height)
.clickable {
onTimetableItemClick(timetableItem)
}
Expand All @@ -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)) {
Expand All @@ -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),
Expand All @@ -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() }
Copy link
Member

@takahirom takahirom Aug 17, 2023

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 👀

Copy link
Contributor Author

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 👀
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.

Copy link
Member

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 👍

Copy link
Contributor Author

@shxun6934 shxun6934 Aug 17, 2023

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

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
Copy link
Contributor Author

@shxun6934 shxun6934 Aug 16, 2023

Choose a reason for hiding this comment

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

[Ask For Option]

I set up the logic as follows.

  1. I checked the height of the Box that displays the session title. (Lines 170-178 of the code)
  2. 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.
  3. 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.🙇‍♂️


val oneLine = 1
val minFontSize = 9.sp
return when {
displayTitleHeight <= 0 -> minFontSize
rows <= oneLine || displayTitleHeight > actualTitleHeight -> textStyle.fontSize
else -> minFontSize
}
}

@Preview
@Composable
fun PreviewTimetableGridItem() {
Expand All @@ -132,6 +204,36 @@ fun PreviewTimetableGridItem() {
TimetableGridItem(
timetableItem = Session.fake(),
onTimetableItemClick = {},
gridItemHeightPx = 350,
)
}
}
}

@Preview
@Composable
fun PreviewTimetableGridLongTitleItem() {
Copy link
Member

@takahirom takahirom Aug 16, 2023

Choose a reason for hiding this comment

The 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? 🙏

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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!

Copy link
Member

Choose a reason for hiding this comment

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

@shxun6934 Thanks for your quick action!
It might be a reply after tomorrow(today) morning 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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,
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,11 @@ fun TimetableGrid(
timetableState = timetableGridState,
modifier = modifier,
contentPadding = PaddingValues(16.dp),
) { timetableItem ->
) { timetableItem, itemHeightPx ->
TimetableGridItem(
timetableItem = timetableItem,
onTimetableItemClick = onTimetableItemClick,
gridItemHeightPx = itemHeightPx,
)
}
}
Expand All @@ -92,13 +93,9 @@ fun TimetableGrid(
timetableState: TimetableState,
modifier: Modifier = Modifier,
contentPadding: PaddingValues = PaddingValues(),
content: @Composable (TimetableItem) -> Unit,
content: @Composable (TimetableItem, Int) -> Unit,
) {
val coroutineScope = rememberCoroutineScope()
val itemProvider = itemProvider({ timetable.timetableItems.size }) { index ->
val timetableItemWithFavorite = timetable.contents[index]
content(timetableItemWithFavorite.timetableItem)
}
val density = timetableState.density
val verticalScale = timetableState.screenScaleState.verticalScale
val timetableLayout = remember(timetable, verticalScale) {
Expand All @@ -117,6 +114,12 @@ fun TimetableGrid(
val linePxSize = with(timetableState.density) { TimetableSizes.lineStrokeSize.toPx() }
val layoutDirection = LocalLayoutDirection.current

val itemProvider = itemProvider({ timetable.timetableItems.size }) { index ->
val timetableItemWithFavorite = timetable.contents[index]
val itemHeightPx = timetableLayout.timetableLayouts[index].height
content(timetableItemWithFavorite.timetableItem, itemHeightPx)
}

LazyLayout(
modifier = modifier
.padding(
Expand Down Expand Up @@ -246,10 +249,11 @@ fun TimetablePreview() {
modifier = Modifier.fillMaxSize(),
timetable = Timetable.fake(),
timetableState = timetableState,
) { timetableItem ->
) { timetableItem, itemHeightPx ->
TimetableGridItem(
timetableItem = timetableItem,
onTimetableItemClick = {},
gridItemHeightPx = itemHeightPx,
)
}
}
Expand Down
Loading