Favorite session date text #574
Conversation
|
|
||
| // session time text | ||
| var sessionTimeTextPositionY = | ||
| (view.top + sessionTimeTextMarginTop.toInt() - sessionTimeTextMarginTopInPx.toInt()).coerceAtLeast( |
There was a problem hiding this comment.
| 🚫 | Exceeded max line length (100) (cannot be auto-corrected) |
CaptainPAG
changed the title
| @@ -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 | |||
There was a problem hiding this comment.
isShowDate looks ambiguous because it's shadowed due to here and isShowDateText is also defined.
How about visibleSessionDate for here?
| @@ -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) { | |||
There was a problem hiding this comment.
The following looks clearer.
| val isShowDateText = if (position > 0) { | |
| val shouldShowDateText = visibleSessionDate && if (position > 0) { |
| @@ -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") } | |||
| val startTimeText: StartTimeText | ||
| ) | ||
|
|
||
| private data class DateText( |
There was a problem hiding this comment.
Is this almost the same class as StartTimeText? then it would be enough to have only one class like PositionalText.
| textPaint | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| private fun calcTimeText(position: Int, view: View): StartTimeText { | ||
| private fun calcDateTimeText( |
There was a problem hiding this comment.
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.
|
@CaptainPAG If you have any questions. Please tell me 🙏 |
…to favorite-session-date-text
…to favorite-session-date-text # Conflicts: # feature/session/src/main/java/io/github/droidkaigi/confsched2020/session/ui/item/SessionItem.kt
|
@CaptainPAG |
| currentSessionItem.startSessionDate() == lastSessionItem.startSessionDate() | ||
| return when { | ||
| isSameDateWithLastItem -> false | ||
| else -> true |
There was a problem hiding this comment.
When multiple same time's favorite session has, date time text will dismiss on scrolling one item height in this code.😭
But, I was not be able to fix this problem. I'll issue for the problem once.
|
Your apk has been deployed to https://deploygate.com/distributions/621a358cc169f3b59aa946169198df451f816df3. Anyone can try your changes via the link. Generated by 🚫 Danger |
|
No issue was reported. Cool! Generated by 🚫 Danger |
| @@ -26,6 +26,8 @@ sealed class SessionPage : AndroidParcel { | |||
|
|
|||
| abstract val title: String | |||
|
|
|||
| fun visibleSessionDate() = this is Event || this is Favorite | |||
|
LGTM 👍 |
Issue
Overview (Required)
Screenshot