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

Favorite session date text #574

Merged
merged 13 commits into from
Feb 9, 2020
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ class BottomSheetSessionsFragment : DaggerFragment() {
binding.sessionRecycler.addItemDecoration(
SessionsItemDecoration(
groupAdapter,
requireContext()
requireContext(),
args.page == SessionPage.Favorite
)
)
binding.sessionRecycler.addOnScrollListener(object : RecyclerView.OnScrollListener() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,8 @@ class SessionItem @AssistedInject constructor(
imageRequestDisposables.forEach { it.dispose() }
}

fun startSessionDate(): String = session.startDayText

fun startSessionTime(): String = session.startTimeText

fun title(): LocaledString = session.title
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,15 @@ import io.github.droidkaigi.confsched2020.util.AndroidRAttr

class SessionsItemDecoration(
private val adapter: GroupAdapter<*>,
private val context: Context
private val context: Context,
private val isShowDate: Boolean
Copy link
Contributor

Choose a reason for hiding this comment

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

isShowDate looks ambiguous because it's shadowed due to here and isShowDateText is also defined.

How about visibleSessionDate for here?

) : RecyclerView.ItemDecoration() {

private val res: Resources = context.resources
private val textPaint by lazy {
Paint().apply {
isAntiAlias = true
textAlign = Paint.Align.CENTER
textAlign = Paint.Align.LEFT
textSize = sessionTimeTextSizeInPx
color = context.getThemeColor(AndroidRAttr.textColorHint)
}
Expand All @@ -36,6 +37,9 @@ class SessionsItemDecoration(
private val sessionTimeTextMarginTopInPx by lazy {
res.getDimensionPixelSize(R.dimen.session_time_text_margin_top).toFloat()
}
private val sessionTimeTextMarginStartInPx by lazy {
res.getDimensionPixelSize(R.dimen.session_time_text_margin_start).toFloat()
}

override fun onDrawOver(c: Canvas, parent: RecyclerView, state: RecyclerView.State) {
super.onDrawOver(c, parent, state)
Expand All @@ -44,7 +48,14 @@ class SessionsItemDecoration(
if (position == RecyclerView.NO_POSITION) return

val sessionItem = adapter.getItem(position) as SessionItem
val startTimeText = calcTimeText(position, view)
val isShowDateText = if (position > 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The following looks clearer.

Suggested change
val isShowDateText = if (position > 0) {
val shouldShowDateText = visibleSessionDate && if (position > 0) {

val lastSessionItem = adapter.getItem(position - 1) as SessionItem
sessionItem.startSessionDate() != lastSessionItem.startSessionDate()
} else {
true
}
val startDateTimeText =
calcDateTimeText(position, view, this.isShowDate && isShowDateText)

// we need least first session's label, skip to check if time label is same as last item on first item.
if (position > 0 && index > 0) {
Expand All @@ -54,34 +65,94 @@ class SessionsItemDecoration(
}
}

if (startDateTimeText.dateText != null) {
c.drawText(
startDateTimeText.dateText.value,
startDateTimeText.dateText.positionX,
startDateTimeText.dateText.positionY,
textPaint
)
}

c.drawText(
startTimeText.value,
startTimeText.positionX,
startTimeText.positionY,
startDateTimeText.startTimeText.value,
startDateTimeText.startTimeText.positionX,
startDateTimeText.startTimeText.positionY,
textPaint
)
}
}

private fun calcTimeText(position: Int, view: View): StartTimeText {
private fun calcDateTimeText(
Copy link
Contributor

Choose a reason for hiding this comment

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

Great work. 👀

Copy link
Contributor

Choose a reason for hiding this comment

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

I felt it would be nicer if this method could be split into calcTimeText and calcDateText with offset calculation when I glanced, but it would be kinda heavy work. So it's okay but we are welcoming additional improvements anytime.

position: Int,
view: View,
isShowDate: Boolean
): StartDateTimeText {
val sessionItem = adapter.getItem(position) as SessionItem
val nextSessionItem = if (position < adapter.itemCount - 1) {
adapter.getItem(position + 1) as SessionItem
} else null

var positionY =
view.top.coerceAtLeast(sessionTimeTextMarginTopInPx.toInt()) +
sessionTimeTextMarginTopInPx + sessionTimeTextSizeInPx
// session date text
val dateText = if (isShowDate) {

var sessionDateTextPositionY =
view.top.coerceAtLeast(sessionTimeTextMarginTopInPx.toInt()) +
sessionTimeTextMarginTopInPx + sessionTimeTextSizeInPx

if (sessionItem.startSessionTime() != nextSessionItem?.startSessionTime()) {
sessionDateTextPositionY =
sessionDateTextPositionY.coerceAtMost(view.bottom.toFloat())
}

DateText(
value = sessionItem.startSessionDate(),
positionX = sessionTimeTextMarginStartInPx,
positionY = sessionDateTextPositionY.coerceAtMost(
view.bottom.toFloat() -
sessionTimeTextMarginTopInPx -
sessionTimeTextSizeInPx
)
)
} else null

val sessionTimeTextMarginTop = if (isShowDate) {
sessionTimeTextMarginTopInPx + sessionTimeTextSizeInPx + sessionTimeTextMarginTopInPx
} else {
sessionTimeTextMarginTopInPx
}

// session time text
var sessionTimeTextPositionY =
(view.top + sessionTimeTextMarginTop.toInt() - sessionTimeTextMarginTopInPx.toInt()).coerceAtLeast(
Copy link
Collaborator

Choose a reason for hiding this comment

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

🚫 Exceeded max line length (100) (cannot be auto-corrected)

sessionTimeTextMarginTop.toInt()
) +
sessionTimeTextMarginTopInPx + sessionTimeTextSizeInPx

if (sessionItem.startSessionTime() != nextSessionItem?.startSessionTime()) {
positionY = positionY.coerceAtMost(view.bottom.toFloat())
sessionTimeTextPositionY = sessionTimeTextPositionY.coerceAtMost(view.bottom.toFloat())
}
return StartTimeText(

val startTimeText = StartTimeText(
value = sessionItem.startSessionTime(),
positionX = (sessionTimeSpaceInPx / 2).toFloat(),
positionY = positionY
positionX = sessionTimeTextMarginStartInPx,
positionY = sessionTimeTextPositionY
)

return StartDateTimeText(dateText, startTimeText)
}

private data class StartDateTimeText(
val dateText: DateText?,
val startTimeText: StartTimeText
)

private data class DateText(
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this almost the same class as StartTimeText? then it would be enough to have only one class like PositionalText.

val value: String,
val positionX: Float,
val positionY: Float
)

private data class StartTimeText(
val value: String,
val positionX: Float,
Expand Down
1 change: 1 addition & 0 deletions feature/session/src/main/res/values/dimens.xml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
<dimen name="session_time_text_size">16sp</dimen>
<dimen name="session_time_space">76dp</dimen>
<dimen name="session_time_text_margin_top">8dp</dimen>
<dimen name="session_time_text_margin_start">16dp</dimen>
<dimen name="session_item_margin_bottom">16dp</dimen>
<dimen name="session_item_speaker_margin">8dp</dimen>
<dimen name="session_filter_chip_vertical_margin">8dp</dimen>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ sealed class Session(
open val room: Room,
open val isFavorited: Boolean
) {
val startDayText by lazy { startTime.toOffset(defaultTimeZoneOffset()).format("yyyy.M.d") }
val startDayText by lazy { startTime.toOffset(defaultTimeZoneOffset()).format("M/d") }
Copy link
Contributor

Choose a reason for hiding this comment

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

Make sense 😄


val startTimeText by lazy { startTime.toOffset(defaultTimeZoneOffset()).format("HH:mm") }

Expand Down