Skip to content

Commit

Permalink
refactor: Use same patterns as Notifications* implementation (#1222)
Browse files Browse the repository at this point in the history
The modifications to the Notifications* classes highlighted different
(and better) ways of writing the code that manages status timelines.
Follow those practices here.

Changes include:

- Move `pachliAccountId` in to `IStatusViewData` so the adapter does not
need to be initialised with the information. This allows the parameter
to be removed from functions that operate on `IStatusViewData`, and the
adapter does not need to be marked `lateinit`.

- Convert Fragment/ViewModel communication to use the `uiResult`
pattern instead of separate `uiSuccess` and `uiError`.

- Show a `LinearProgressIndicator` when refreshing the list.

- Restore the reading position more smoothly by responding when the
first page of results is loaded.

- Save the reading position to `RemoteKeyEntity` instead of a dedicated
property in `AccountEntity`.

- Fixed queries for returning the row number of a notification or
status in the database.

Fixes #238, #872, #928, #1190
  • Loading branch information
nikclayton authored Jan 23, 2025
1 parent 05c68f6 commit 7bf322c
Show file tree
Hide file tree
Showing 62 changed files with 2,836 additions and 1,395 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,28 +35,26 @@ open class FilterableStatusViewHolder<T : IStatusViewData>(
var matchedFilter: Filter? = null

override fun setupWithStatus(
pachliAccountId: Long,
viewData: T,
listener: StatusActionListener<T>,
statusDisplayOptions: StatusDisplayOptions,
payloads: Any?,
) {
super.setupWithStatus(pachliAccountId, viewData, listener, statusDisplayOptions, payloads)
setupFilterPlaceholder(pachliAccountId, viewData, listener)
super.setupWithStatus(viewData, listener, statusDisplayOptions, payloads)
setupFilterPlaceholder(viewData, listener)
}

private fun setupFilterPlaceholder(
pachliAccountId: Long,
status: T,
viewData: T,
listener: StatusActionListener<T>,
) {
if (status.contentFilterAction !== FilterAction.WARN) {
if (viewData.contentFilterAction !== FilterAction.WARN) {
matchedFilter = null
setPlaceholderVisibility(false)
return
}

status.actionable.filtered?.find { it.filter.filterAction === NetworkFilterAction.WARN }?.let { result ->
viewData.actionable.filtered?.find { it.filter.filterAction === NetworkFilterAction.WARN }?.let { result ->
this.matchedFilter = result.filter
setPlaceholderVisibility(true)

Expand All @@ -71,10 +69,10 @@ open class FilterableStatusViewHolder<T : IStatusViewData>(
binding.statusFilteredPlaceholder.statusFilterLabel.text = label

binding.statusFilteredPlaceholder.statusFilterShowAnyway.setOnClickListener {
listener.clearContentFilter(pachliAccountId, status)
listener.clearContentFilter(viewData)
}
binding.statusFilteredPlaceholder.statusFilterEditFilter.setOnClickListener {
listener.onEditFilterById(pachliAccountId, result.filter.id)
listener.onEditFilterById(viewData.pachliAccountId, result.filter.id)
}
} ?: {
matchedFilter = null
Expand Down
35 changes: 11 additions & 24 deletions app/src/main/java/app/pachli/adapter/StatusBaseViewHolder.kt
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,6 @@ abstract class StatusBaseViewHolder<T : IStatusViewData> protected constructor(
}

protected fun setSpoilerAndContent(
pachliAccountId: Long,
viewData: T,
statusDisplayOptions: StatusDisplayOptions,
listener: StatusActionListener<T>,
Expand All @@ -165,7 +164,6 @@ abstract class StatusBaseViewHolder<T : IStatusViewData> protected constructor(
setContentWarningButtonText(expanded)
contentWarningButton.setOnClickListener {
toggleExpandedState(
pachliAccountId,
viewData,
true,
!expanded,
Expand Down Expand Up @@ -197,19 +195,17 @@ abstract class StatusBaseViewHolder<T : IStatusViewData> protected constructor(
}

protected open fun toggleExpandedState(
pachliAccountId: Long,
viewData: T,
sensitive: Boolean,
expanded: Boolean,
statusDisplayOptions: StatusDisplayOptions,
listener: StatusActionListener<T>,
) {
contentWarningDescription.invalidate()
listener.onExpandedChange(pachliAccountId, viewData, expanded)
listener.onExpandedChange(viewData, expanded)
setContentWarningButtonText(expanded)
setTextVisible(sensitive, expanded, viewData, statusDisplayOptions, listener)
setupCard(
pachliAccountId,
viewData,
expanded,
statusDisplayOptions.cardViewMode,
Expand Down Expand Up @@ -466,7 +462,6 @@ abstract class StatusBaseViewHolder<T : IStatusViewData> protected constructor(
}

protected fun setMediaPreviews(
pachliAccountId: Long,
viewData: T,
attachments: List<Attachment>,
sensitive: Boolean,
Expand Down Expand Up @@ -498,7 +493,7 @@ abstract class StatusBaseViewHolder<T : IStatusViewData> protected constructor(
} else {
imageView.foreground = null
}
setAttachmentClickListener(pachliAccountId, viewData, imageView, listener, i, attachment, true)
setAttachmentClickListener(viewData, imageView, listener, i, attachment, true)
if (sensitive) {
sensitiveMediaWarning.setText(R.string.post_sensitive_media_title)
} else {
Expand All @@ -509,13 +504,13 @@ abstract class StatusBaseViewHolder<T : IStatusViewData> protected constructor(
descriptionIndicator.visibility =
if (hasDescription && showingContent) View.VISIBLE else View.GONE
sensitiveMediaShow.setOnClickListener { v: View ->
listener.onContentHiddenChange(pachliAccountId, viewData, false)
listener.onContentHiddenChange(viewData, false)
v.visibility = View.GONE
sensitiveMediaWarning.visibility = View.VISIBLE
descriptionIndicator.visibility = View.GONE
}
sensitiveMediaWarning.setOnClickListener { v: View ->
listener.onContentHiddenChange(pachliAccountId, viewData, true)
listener.onContentHiddenChange(viewData, true)
v.visibility = View.GONE
sensitiveMediaShow.visibility = View.VISIBLE
descriptionIndicator.visibility = if (hasDescription) View.VISIBLE else View.GONE
Expand All @@ -530,7 +525,6 @@ abstract class StatusBaseViewHolder<T : IStatusViewData> protected constructor(
}

protected fun setMediaLabel(
pachliAccountId: Long,
viewData: T,
attachments: List<Attachment>,
sensitive: Boolean,
Expand All @@ -548,15 +542,14 @@ abstract class StatusBaseViewHolder<T : IStatusViewData> protected constructor(
// Set the icon next to the label.
val drawableId = attachments[0].iconResource()
mediaLabel.setCompoundDrawablesRelativeWithIntrinsicBounds(drawableId, 0, 0, 0)
setAttachmentClickListener(pachliAccountId, viewData, mediaLabel, listener, i, attachment, false)
setAttachmentClickListener(viewData, mediaLabel, listener, i, attachment, false)
} else {
mediaLabel.visibility = View.GONE
}
}
}

private fun setAttachmentClickListener(
pachliAccountId: Long,
viewData: T,
view: View,
listener: StatusActionListener<T>,
Expand All @@ -566,7 +559,7 @@ abstract class StatusBaseViewHolder<T : IStatusViewData> protected constructor(
) {
view.setOnClickListener { v: View? ->
if (sensitiveMediaWarning.visibility == View.VISIBLE) {
listener.onContentHiddenChange(pachliAccountId, viewData, true)
listener.onContentHiddenChange(viewData, true)
} else {
listener.onViewMedia(viewData, index, if (animateTransition) v else null)
}
Expand All @@ -584,7 +577,6 @@ abstract class StatusBaseViewHolder<T : IStatusViewData> protected constructor(
}

protected fun setupButtons(
pachliAccountId: Long,
viewData: T,
listener: StatusActionListener<T>,
accountId: String,
Expand All @@ -594,7 +586,7 @@ abstract class StatusBaseViewHolder<T : IStatusViewData> protected constructor(
avatar.setOnClickListener(profileButtonClickListener)
displayName.setOnClickListener(profileButtonClickListener)
replyButton.setOnClickListener {
listener.onReply(pachliAccountId, viewData)
listener.onReply(viewData)
}
reblogButton?.setEventListener { _: SparkButton?, buttonState: Boolean ->
// return true to play animation
Expand Down Expand Up @@ -683,7 +675,6 @@ abstract class StatusBaseViewHolder<T : IStatusViewData> protected constructor(
}

open fun setupWithStatus(
pachliAccountId: Long,
viewData: T,
listener: StatusActionListener<T>,
statusDisplayOptions: StatusDisplayOptions,
Expand Down Expand Up @@ -715,7 +706,6 @@ abstract class StatusBaseViewHolder<T : IStatusViewData> protected constructor(
val sensitive = actionable.sensitive
if (statusDisplayOptions.mediaPreviewEnabled && hasPreviewableAttachment(attachments)) {
setMediaPreviews(
pachliAccountId,
viewData,
attachments,
sensitive,
Expand All @@ -731,28 +721,26 @@ abstract class StatusBaseViewHolder<T : IStatusViewData> protected constructor(
mediaLabel.visibility = View.GONE
}
} else {
setMediaLabel(pachliAccountId, viewData, attachments, sensitive, listener, viewData.isShowingContent)
setMediaLabel(viewData, attachments, sensitive, listener, viewData.isShowingContent)
// Hide all unused views.
mediaPreview.visibility = View.GONE
hideSensitiveMediaWarning()
}
setupCard(
pachliAccountId,
viewData,
viewData.isExpanded,
statusDisplayOptions.cardViewMode,
statusDisplayOptions,
listener,
)
setupButtons(
pachliAccountId,
viewData,
listener,
actionable.account.id,
statusDisplayOptions,
)
setRebloggingEnabled(actionable.rebloggingAllowed(), actionable.visibility)
setSpoilerAndContent(pachliAccountId, viewData, statusDisplayOptions, listener)
setSpoilerAndContent(viewData, statusDisplayOptions, listener)
setContentDescriptionForStatus(viewData, statusDisplayOptions)

// Workaround for RecyclerView 1.0.0 / androidx.core 1.0.0
Expand Down Expand Up @@ -876,7 +864,6 @@ abstract class StatusBaseViewHolder<T : IStatusViewData> protected constructor(
}

protected fun setupCard(
pachliAccountId: Long,
viewData: T,
expanded: Boolean,
cardViewMode: CardViewMode,
Expand All @@ -898,13 +885,13 @@ abstract class StatusBaseViewHolder<T : IStatusViewData> protected constructor(
cardView.bind(card, viewData.actionable.sensitive, statusDisplayOptions, false) { card, target ->
if (target == PreviewCardView.Target.BYLINE) {
card.authors?.firstOrNull()?.account?.id?.let {
context.startActivity(AccountActivityIntent(context, pachliAccountId, it))
context.startActivity(AccountActivityIntent(context, viewData.pachliAccountId, it))
}
return@bind
}

if (card.kind == PreviewCardKind.PHOTO && card.embedUrl.isNotEmpty() && target == PreviewCardView.Target.IMAGE) {
context.startActivity(ViewMediaActivityIntent(context, pachliAccountId, viewData.actionable.account.username, card.embedUrl))
context.startActivity(ViewMediaActivityIntent(context, viewData.pachliAccountId, viewData.actionable.account.username, card.embedUrl))
return@bind
}

Expand Down
10 changes: 4 additions & 6 deletions app/src/main/java/app/pachli/adapter/StatusDetailedViewHolder.kt
Original file line number Diff line number Diff line change
Expand Up @@ -106,26 +106,24 @@ class StatusDetailedViewHolder(
}

override fun setupWithStatus(
pachliAccountId: Long,
viewData: StatusViewData,
listener: StatusActionListener<StatusViewData>,
statusDisplayOptions: StatusDisplayOptions,
payloads: Any?,
) {
// We never collapse statuses in the detail view
val uncollapsedStatus =
val uncollapsedViewdata =
if (viewData.isCollapsible && viewData.isCollapsed) viewData.copy(isCollapsed = false) else viewData
super.setupWithStatus(pachliAccountId, uncollapsedStatus, listener, statusDisplayOptions, payloads)
super.setupWithStatus(uncollapsedViewdata, listener, statusDisplayOptions, payloads)
setupCard(
pachliAccountId,
uncollapsedStatus,
uncollapsedViewdata,
viewData.isExpanded,
CardViewMode.FULL_WIDTH,
statusDisplayOptions,
listener,
) // Always show card for detailed status
if (payloads == null) {
val (_, _, _, _, _, _, _, _, _, _, reblogsCount, favouritesCount) = uncollapsedStatus.actionable
val (_, _, _, _, _, _, _, _, _, _, reblogsCount, favouritesCount) = uncollapsedViewdata.actionable
if (!statusDisplayOptions.hideStats) {
setReblogAndFavCount(viewData, reblogsCount, favouritesCount, listener)
} else {
Expand Down
13 changes: 5 additions & 8 deletions app/src/main/java/app/pachli/adapter/StatusViewHolder.kt
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ open class StatusViewHolder<T : IStatusViewData>(
) : StatusBaseViewHolder<T>(root ?: binding.root) {

override fun setupWithStatus(
pachliAccountId: Long,
viewData: T,
listener: StatusActionListener<T>,
statusDisplayOptions: StatusDisplayOptions,
Expand All @@ -50,7 +49,7 @@ open class StatusViewHolder<T : IStatusViewData>(
if (payloads == null) {
val sensitive = !TextUtils.isEmpty(viewData.actionable.spoilerText)
val expanded = viewData.isExpanded
setupCollapsedState(pachliAccountId, viewData, sensitive, expanded, listener)
setupCollapsedState(viewData, sensitive, expanded, listener)
val reblogging = viewData.rebloggingStatus
if (reblogging == null || viewData.contentFilterAction === FilterAction.WARN) {
statusInfo.hide()
Expand All @@ -70,7 +69,7 @@ open class StatusViewHolder<T : IStatusViewData>(
statusFavouritesCount.visible(statusDisplayOptions.showStatsInline)
setFavouritedCount(viewData.actionable.favouritesCount)
setReblogsCount(viewData.actionable.reblogsCount)
super.setupWithStatus(pachliAccountId, viewData, listener, statusDisplayOptions, payloads)
super.setupWithStatus(viewData, listener, statusDisplayOptions, payloads)
}

private fun setRebloggedByDisplayName(
Expand Down Expand Up @@ -108,7 +107,6 @@ open class StatusViewHolder<T : IStatusViewData>(
}

private fun setupCollapsedState(
pachliAccountId: Long,
viewData: T,
sensitive: Boolean,
expanded: Boolean,
Expand All @@ -117,7 +115,7 @@ open class StatusViewHolder<T : IStatusViewData>(
/* input filter for TextViews have to be set before text */
if (viewData.isCollapsible && (!sensitive || expanded)) {
buttonToggleContent.setOnClickListener {
listener.onContentCollapsedChange(pachliAccountId, viewData, !viewData.isCollapsed)
listener.onContentCollapsedChange(viewData, !viewData.isCollapsed)
}
buttonToggleContent.show()
if (viewData.isCollapsed) {
Expand All @@ -139,15 +137,14 @@ open class StatusViewHolder<T : IStatusViewData>(
}

override fun toggleExpandedState(
pachliAccountId: Long,
viewData: T,
sensitive: Boolean,
expanded: Boolean,
statusDisplayOptions: StatusDisplayOptions,
listener: StatusActionListener<T>,
) {
setupCollapsedState(pachliAccountId, viewData, sensitive, expanded, listener)
super.toggleExpandedState(pachliAccountId, viewData, sensitive, expanded, statusDisplayOptions, listener)
setupCollapsedState(viewData, sensitive, expanded, listener)
super.toggleExpandedState(viewData, sensitive, expanded, statusDisplayOptions, listener)
}

companion object {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import app.pachli.core.data.model.StatusDisplayOptions
import app.pachli.interfaces.StatusActionListener

class ConversationAdapter(
private val pachliAccountId: Long,
private var statusDisplayOptions: StatusDisplayOptions,
private val listener: StatusActionListener<ConversationViewData>,
) : PagingDataAdapter<ConversationViewData, ConversationViewHolder>(CONVERSATION_COMPARATOR) {
Expand Down Expand Up @@ -54,7 +53,7 @@ class ConversationAdapter(
payloads: List<Any>,
) {
getItem(position)?.let { conversationViewData ->
holder.setupWithConversation(pachliAccountId, conversationViewData, payloads.firstOrNull())
holder.setupWithConversation(conversationViewData, payloads.firstOrNull())
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,12 @@ data class ConversationViewData(
val lastStatus: StatusViewData,
) : IStatusViewData by lastStatus {
companion object {
fun from(conversationEntity: ConversationEntity) = ConversationViewData(
fun from(pachliAccountId: Long, conversationEntity: ConversationEntity) = ConversationViewData(
id = conversationEntity.id,
order = conversationEntity.order,
accounts = conversationEntity.accounts,
unread = conversationEntity.unread,
lastStatus = StatusViewData.from(conversationEntity.lastStatus),
lastStatus = StatusViewData.from(pachliAccountId, conversationEntity.lastStatus),
)
}
}
Loading

0 comments on commit 7bf322c

Please sign in to comment.