Skip to content

Commit

Permalink
Don't use mutable state flows in UI (#4336)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Goooler
Goooler authored Mar 27, 2024
1 parent 3274bd2 commit a3d87de
Show file tree
Hide file tree
Showing 7 changed files with 117 additions and 85 deletions.
2 changes: 1 addition & 1 deletion app/src/main/java/com/keylesspalace/tusky/components/compose/ComposeActivity.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1040,7 +1040,7 @@ class ComposeActivity :

override fun onVisibilityChanged(visibility: Status.Visibility) {
composeOptionsBehavior.state = BottomSheetBehavior.STATE_COLLAPSED
viewModel.statusVisibility.value = visibility
viewModel.changeStatusVisibility(visibility)
}

@VisibleForTesting
Expand Down
102 changes: 59 additions & 43 deletions app/src/main/java/com/keylesspalace/tusky/components/compose/ComposeViewModel.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,9 @@ import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.SharedFlow
import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.asFlow
import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.flow.shareIn
import kotlinx.coroutines.flow.update
Expand Down Expand Up @@ -86,24 +88,34 @@ class ComposeViewModel @Inject constructor(
val emoji: SharedFlow<List<Emoji>> = instanceInfoRepo::getEmojis.asFlow()
.shareIn(viewModelScope, SharingStarted.Eagerly, replay = 1)

val markMediaAsSensitive: MutableStateFlow<Boolean> =
private val _markMediaAsSensitive =
MutableStateFlow(accountManager.activeAccount?.defaultMediaSensitivity ?: false)
val markMediaAsSensitive: StateFlow<Boolean> = _markMediaAsSensitive.asStateFlow()

val statusVisibility: MutableStateFlow<Status.Visibility> =
MutableStateFlow(Status.Visibility.UNKNOWN)
val showContentWarning: MutableStateFlow<Boolean> = MutableStateFlow(false)
val poll: MutableStateFlow<NewPoll?> = MutableStateFlow(null)
val scheduledAt: MutableStateFlow<String?> = MutableStateFlow(null)
private val _statusVisibility = MutableStateFlow(Status.Visibility.UNKNOWN)
val statusVisibility: StateFlow<Status.Visibility> = _statusVisibility.asStateFlow()

private val _showContentWarning = MutableStateFlow(false)
val showContentWarning: StateFlow<Boolean> = _showContentWarning.asStateFlow()

private val _poll = MutableStateFlow(null as NewPoll?)
val poll: StateFlow<NewPoll?> = _poll.asStateFlow()

private val _scheduledAt = MutableStateFlow(null as String?)
val scheduledAt: StateFlow<String?> = _scheduledAt.asStateFlow()

private val _media = MutableStateFlow(emptyList<QueuedMedia>())
val media: StateFlow<List<QueuedMedia>> = _media.asStateFlow()

val media: MutableStateFlow<List<QueuedMedia>> = MutableStateFlow(emptyList())
val uploadError =
MutableSharedFlow<Throwable>(
replay = 0,
extraBufferCapacity = 1,
onBufferOverflow = BufferOverflow.DROP_OLDEST
)

val closeConfirmation = MutableStateFlow(ConfirmationKind.NONE)
private val _closeConfirmation = MutableStateFlow(ConfirmationKind.NONE)
val closeConfirmation: StateFlow<ConfirmationKind> = _closeConfirmation.asStateFlow()

private lateinit var composeKind: ComposeKind

Expand All @@ -121,7 +133,7 @@ class ComposeViewModel @Inject constructor(
) {
try {
val (type, uri, size) = mediaUploader.prepareMedia(mediaUri, instanceInfo.first())
val mediaItems = media.value
val mediaItems = _media.value
if (type != QueuedMedia.Type.IMAGE &&
mediaItems.isNotEmpty() &&
mediaItems[0].type == QueuedMedia.Type.IMAGE
Expand All @@ -146,7 +158,7 @@ class ComposeViewModel @Inject constructor(
): QueuedMedia {
var stashMediaItem: QueuedMedia? = null

media.update { mediaList ->
_media.update { mediaList ->
val mediaItem = QueuedMedia(
localId = mediaUploader.getNewLocalMediaId(),
uri = uri,
Expand All @@ -173,7 +185,7 @@ class ComposeViewModel @Inject constructor(
mediaUploader
.uploadMedia(mediaItem, instanceInfo.first())
.collect { event ->
val item = media.value.find { it.localId == mediaItem.localId }
val item = _media.value.find { it.localId == mediaItem.localId }
?: return@collect
val newMediaItem = when (event) {
is UploadEvent.ProgressEvent ->
Expand All @@ -189,12 +201,12 @@ class ComposeViewModel @Inject constructor(
}
)
is UploadEvent.ErrorEvent -> {
media.update { mediaList -> mediaList.filter { it.localId != mediaItem.localId } }
_media.update { mediaList -> mediaList.filter { it.localId != mediaItem.localId } }
uploadError.emit(event.error)
return@collect
}
}
media.update { mediaList ->
_media.update { mediaList ->
mediaList.map { mediaItem ->
if (mediaItem.localId == newMediaItem.localId) {
newMediaItem
Expand All @@ -209,14 +221,18 @@ class ComposeViewModel @Inject constructor(
return mediaItem
}

fun changeStatusVisibility(visibility: Status.Visibility) {
_statusVisibility.value = visibility
}

private fun addUploadedMedia(
id: String,
type: QueuedMedia.Type,
uri: Uri,
description: String?,
focus: Attachment.Focus?
) {
media.update { mediaList ->
_media.update { mediaList ->
val mediaItem = QueuedMedia(
localId = mediaUploader.getNewLocalMediaId(),
uri = uri,
Expand All @@ -234,12 +250,12 @@ class ComposeViewModel @Inject constructor(

fun removeMediaFromQueue(item: QueuedMedia) {
mediaUploader.cancelUploadScope(item.localId)
media.update { mediaList -> mediaList.filter { it.localId != item.localId } }
_media.update { mediaList -> mediaList.filter { it.localId != item.localId } }
updateCloseConfirmation()
}

fun toggleMarkSensitive() {
this.markMediaAsSensitive.value = this.markMediaAsSensitive.value != true
this._markMediaAsSensitive.value = this._markMediaAsSensitive.value != true
}

fun updateContent(newContent: String?) {
Expand All @@ -253,12 +269,12 @@ class ComposeViewModel @Inject constructor(
}

private fun updateCloseConfirmation() {
val contentWarning = if (showContentWarning.value) {
val contentWarning = if (_showContentWarning.value) {
currentContentWarning
} else {
""
}
this.closeConfirmation.value = if (didChange(currentContent, contentWarning)) {
this._closeConfirmation.value = if (didChange(currentContent, contentWarning)) {
when (composeKind) {
ComposeKind.NEW -> if (isEmpty(currentContent, contentWarning)) {
ConfirmationKind.NONE
Expand All @@ -281,19 +297,19 @@ class ComposeViewModel @Inject constructor(
private fun didChange(content: String?, contentWarning: String?): Boolean {
val textChanged = content.orEmpty() != startingText.orEmpty()
val contentWarningChanged = contentWarning.orEmpty() != startingContentWarning
val mediaChanged = media.value.isNotEmpty()
val pollChanged = poll.value != null
val mediaChanged = _media.value.isNotEmpty()
val pollChanged = _poll.value != null
val didScheduledTimeChange = hasScheduledTimeChanged

return modifiedInitialState || textChanged || contentWarningChanged || mediaChanged || pollChanged || didScheduledTimeChange
}

private fun isEmpty(content: String?, contentWarning: String?): Boolean {
return !modifiedInitialState && (content.isNullOrBlank() && contentWarning.isNullOrBlank() && media.value.isEmpty() && poll.value == null)
return !modifiedInitialState && (content.isNullOrBlank() && contentWarning.isNullOrBlank() && _media.value.isEmpty() && _poll.value == null)
}

fun contentWarningChanged(value: Boolean) {
showContentWarning.value = value
_showContentWarning.value = value
contentWarningStateChanged = true
updateCloseConfirmation()
}
Expand All @@ -307,12 +323,12 @@ class ComposeViewModel @Inject constructor(
}

fun stopUploads() {
mediaUploader.cancelUploadScope(*media.value.map { it.localId }.toIntArray())
mediaUploader.cancelUploadScope(*_media.value.map { it.localId }.toIntArray())
}

fun shouldShowSaveDraftDialog(): Boolean {
// if any of the media files need to be downloaded first it could take a while, so show a loading dialog
return media.value.any { mediaValue ->
return _media.value.any { mediaValue ->
mediaValue.uri.scheme == "https"
}
}
Expand All @@ -321,7 +337,7 @@ class ComposeViewModel @Inject constructor(
val mediaUris: MutableList<String> = mutableListOf()
val mediaDescriptions: MutableList<String?> = mutableListOf()
val mediaFocus: MutableList<Attachment.Focus?> = mutableListOf()
for (item in media.value) {
for (item in _media.value) {
mediaUris.add(item.uri.toString())
mediaDescriptions.add(item.description)
mediaFocus.add(item.focus)
Expand All @@ -333,15 +349,15 @@ class ComposeViewModel @Inject constructor(
inReplyToId = inReplyToId,
content = content,
contentWarning = contentWarning,
sensitive = markMediaAsSensitive.value,
visibility = statusVisibility.value,
sensitive = _markMediaAsSensitive.value,
visibility = _statusVisibility.value,
mediaUris = mediaUris,
mediaDescriptions = mediaDescriptions,
mediaFocus = mediaFocus,
poll = poll.value,
poll = _poll.value,
failedToSend = false,
failedToSendAlert = false,
scheduledAt = scheduledAt.value,
scheduledAt = _scheduledAt.value,
language = postLanguage,
statusId = originalStatusId
)
Expand All @@ -356,7 +372,7 @@ class ComposeViewModel @Inject constructor(
api.deleteScheduledStatus(scheduledTootId!!)
}

val attachedMedia = media.value.map { item ->
val attachedMedia = _media.value.map { item ->
MediaToSend(
localId = item.localId,
id = item.id,
Expand All @@ -369,12 +385,12 @@ class ComposeViewModel @Inject constructor(
val tootToSend = StatusToSend(
text = content,
warningText = spoilerText,
visibility = statusVisibility.value.serverString(),
sensitive = attachedMedia.isNotEmpty() && (markMediaAsSensitive.value || showContentWarning.value),
visibility = _statusVisibility.value.serverString(),
sensitive = attachedMedia.isNotEmpty() && (_markMediaAsSensitive.value || _showContentWarning.value),
media = attachedMedia,
scheduledAt = scheduledAt.value,
scheduledAt = _scheduledAt.value,
inReplyToId = inReplyToId,
poll = poll.value,
poll = _poll.value,
replyingStatusContent = null,
replyingStatusAuthorUsername = null,
accountId = accountId,
Expand All @@ -389,7 +405,7 @@ class ComposeViewModel @Inject constructor(
}

private fun updateMediaItem(localId: Int, mutator: (QueuedMedia) -> QueuedMedia) {
media.update { mediaList ->
_media.update { mediaList ->
mediaList.map { mediaItem ->
if (mediaItem.localId == localId) {
mutator(mediaItem)
Expand Down Expand Up @@ -478,7 +494,7 @@ class ComposeViewModel @Inject constructor(
startingContentWarning = contentWarning
}
if (!contentWarningStateChanged) {
showContentWarning.value = !contentWarning.isNullOrBlank()
_showContentWarning.value = !contentWarning.isNullOrBlank()
}

// recreate media list
Expand Down Expand Up @@ -512,7 +528,7 @@ class ComposeViewModel @Inject constructor(
if (tootVisibility.num != Status.Visibility.UNKNOWN.num) {
startingVisibility = tootVisibility
}
statusVisibility.value = startingVisibility
_statusVisibility.value = startingVisibility
val mentionedUsernames = composeOptions?.mentionedUsernames
if (mentionedUsernames != null) {
val builder = StringBuilder()
Expand All @@ -524,13 +540,13 @@ class ComposeViewModel @Inject constructor(
startingText = builder.toString()
}

scheduledAt.value = composeOptions?.scheduledAt
_scheduledAt.value = composeOptions?.scheduledAt

composeOptions?.sensitive?.let { markMediaAsSensitive.value = it }
composeOptions?.sensitive?.let { _markMediaAsSensitive.value = it }

val poll = composeOptions?.poll
if (poll != null && composeOptions.mediaAttachments.isNullOrEmpty()) {
this.poll.value = poll
this._poll.value = poll
}
replyingStatusContent = composeOptions?.replyingStatusContent
replyingStatusAuthor = composeOptions?.replyingStatusAuthor
Expand All @@ -541,16 +557,16 @@ class ComposeViewModel @Inject constructor(
}

fun updatePoll(newPoll: NewPoll?) {
poll.value = newPoll
_poll.value = newPoll
updateCloseConfirmation()
}

fun updateScheduledAt(newScheduledAt: String?) {
if (newScheduledAt != scheduledAt.value) {
if (newScheduledAt != _scheduledAt.value) {
hasScheduledTimeChanged = true
}

scheduledAt.value = newScheduledAt
_scheduledAt.value = newScheduledAt
}

val editing: Boolean
Expand Down
Loading

0 comments on commit a3d87de

Please sign in to comment.