Fix saving changes to statuses when editing (#3103)

* Fix saving changes to statuses when editing

With the previous code backing out of a status editing operation where changes
had been made  (whether it was editing an existing status, a scheduled status,
or a draft) would prompt the user to save the changes as a new draft.

See https://github.com/tuskyapp/Tusky/issues/2704 and
https://github.com/tuskyapp/Tusky/issues/2705 for more detail.

The fix:

- Create an enum to represent the four different kinds of edits that can
  happen
  - Editing a new status (i.e., composing it for the first time)
  - Editing a posted status
  - Editing a draft
  - Editing a scheduled status

- Store this in ComposeOptions, and set it appropriately everywhere
  ComposeOptions is created.

- Check the edit kind when backing out of ComposeActivity, and use this to
  show one of three different dialogs as appropriate so the user can:
  - Save as new draft or discard changes
  - Continue editing or discard changes
  - Update existing draft or discard changes

Also fix ComposeViewModel.didChange(), which erroneously reported false if the
old text started with the new text (e.g., if the old text was "hello, world"
and it was edited to "hello", didChange() would not consider that to be a
change).

Fixes https://github.com/tuskyapp/Tusky/issues/2704,
https://github.com/tuskyapp/Tusky/issues/2705

* Use orEmpty extension function
This commit is contained in:
Nik Clayton 2022-12-31 13:04:49 +01:00 committed by GitHub
parent 2e72fa0dfc
commit a5f479d79c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 112 additions and 30 deletions

View file

@ -811,9 +811,12 @@ class AccountActivity : BottomSheetActivity(), ActionButtonActivity, HasAndroidI
private fun mention() { private fun mention() {
loadedAccount?.let { loadedAccount?.let {
val options = if (viewModel.isSelf) { val options = if (viewModel.isSelf) {
ComposeActivity.ComposeOptions() ComposeActivity.ComposeOptions(kind = ComposeActivity.ComposeKind.NEW)
} else { } else {
ComposeActivity.ComposeOptions(mentionedUsernames = setOf(it.username)) ComposeActivity.ComposeOptions(
mentionedUsernames = setOf(it.username),
kind = ComposeActivity.ComposeKind.NEW
)
} }
val intent = ComposeActivity.startIntent(this, options) val intent = ComposeActivity.startIntent(this, options)
startActivity(intent) startActivity(intent)

View file

@ -1115,30 +1115,79 @@ class ComposeActivity :
val contentText = binding.composeEditField.text.toString() val contentText = binding.composeEditField.text.toString()
val contentWarning = binding.composeContentWarningField.text.toString() val contentWarning = binding.composeContentWarningField.text.toString()
if (viewModel.didChange(contentText, contentWarning)) { if (viewModel.didChange(contentText, contentWarning)) {
when (viewModel.composeKind) {
val warning = if (!viewModel.media.value.isEmpty()) { ComposeKind.NEW -> getSaveAsDraftOrDiscardDialog(contentText, contentWarning)
R.string.compose_save_draft_loses_media ComposeKind.EDIT_DRAFT -> getUpdateDraftOrDiscardDialog(contentText, contentWarning)
} else { ComposeKind.EDIT_POSTED -> getContinueEditingOrDiscardDialog()
R.string.compose_save_draft ComposeKind.EDIT_SCHEDULED -> getContinueEditingOrDiscardDialog()
} }.show()
AlertDialog.Builder(this)
.setMessage(warning)
.setPositiveButton(R.string.action_save) { _, _ ->
viewModel.stopUploads()
saveDraftAndFinish(contentText, contentWarning)
}
.setNegativeButton(R.string.action_delete) { _, _ ->
viewModel.stopUploads()
deleteDraftAndFinish()
}
.show()
} else { } else {
viewModel.stopUploads() viewModel.stopUploads()
finishWithoutSlideOutAnimation() finishWithoutSlideOutAnimation()
} }
} }
/**
* User is editing a new post, and can either save the changes as a draft or discard them.
*/
private fun getSaveAsDraftOrDiscardDialog(contentText: String, contentWarning: String): AlertDialog.Builder {
val warning = if (viewModel.media.value.isNotEmpty()) {
R.string.compose_save_draft_loses_media
} else {
R.string.compose_save_draft
}
return AlertDialog.Builder(this)
.setMessage(warning)
.setPositiveButton(R.string.action_save) { _, _ ->
viewModel.stopUploads()
saveDraftAndFinish(contentText, contentWarning)
}
.setNegativeButton(R.string.action_delete) { _, _ ->
viewModel.stopUploads()
deleteDraftAndFinish()
}
}
/**
* User is editing an existing draft, and can either update the draft with the new changes or
* discard them.
*/
private fun getUpdateDraftOrDiscardDialog(contentText: String, contentWarning: String): AlertDialog.Builder {
val warning = if (viewModel.media.value.isNotEmpty()) {
R.string.compose_save_draft_loses_media
} else {
R.string.compose_save_draft
}
return AlertDialog.Builder(this)
.setMessage(warning)
.setPositiveButton(R.string.action_save) { _, _ ->
viewModel.stopUploads()
saveDraftAndFinish(contentText, contentWarning)
}
.setNegativeButton(R.string.action_discard) { _, _ ->
viewModel.stopUploads()
finishWithoutSlideOutAnimation()
}
}
/**
* User is editing a post (scheduled, or posted), and can either go back to editing, or
* discard the changes.
*/
private fun getContinueEditingOrDiscardDialog(): AlertDialog.Builder {
return AlertDialog.Builder(this)
.setMessage(R.string.compose_unsaved_changes)
.setPositiveButton(R.string.action_continue_edit) { _, _ ->
// Do nothing, dialog will dismiss, user can continue editing
}
.setNegativeButton(R.string.action_discard) { _, _ ->
viewModel.stopUploads()
finishWithoutSlideOutAnimation()
}
}
private fun deleteDraftAndFinish() { private fun deleteDraftAndFinish() {
viewModel.deleteDraft() viewModel.deleteDraft()
finishWithoutSlideOutAnimation() finishWithoutSlideOutAnimation()
@ -1217,6 +1266,24 @@ class ComposeActivity :
} }
} }
/**
* Status' kind. This particularly affects how the status is handled if the user
* backs out of the edit.
*/
enum class ComposeKind {
/** Status is new */
NEW,
/** Editing a posted status */
EDIT_POSTED,
/** Editing a status started as an existing draft */
EDIT_DRAFT,
/** Editing an an existing scheduled status */
EDIT_SCHEDULED
}
@Parcelize @Parcelize
data class ComposeOptions( data class ComposeOptions(
// Let's keep fields var until all consumers are Kotlin // Let's keep fields var until all consumers are Kotlin
@ -1240,6 +1307,7 @@ class ComposeActivity :
var modifiedInitialState: Boolean? = null, var modifiedInitialState: Boolean? = null,
var language: String? = null, var language: String? = null,
var statusId: String? = null, var statusId: String? = null,
var kind: ComposeKind? = null
) : Parcelable ) : Parcelable
companion object { companion object {

View file

@ -95,6 +95,8 @@ class ComposeViewModel @Inject constructor(
val media: MutableStateFlow<List<QueuedMedia>> = MutableStateFlow(emptyList()) val media: MutableStateFlow<List<QueuedMedia>> = MutableStateFlow(emptyList())
val uploadError = MutableSharedFlow<Throwable>(replay = 0, extraBufferCapacity = 1, onBufferOverflow = BufferOverflow.DROP_OLDEST) val uploadError = MutableSharedFlow<Throwable>(replay = 0, extraBufferCapacity = 1, onBufferOverflow = BufferOverflow.DROP_OLDEST)
lateinit var composeKind: ComposeActivity.ComposeKind
// Used in ComposeActivity to pass state to result function when cropImage contract inflight // Used in ComposeActivity to pass state to result function when cropImage contract inflight
var cropImageItemOld: QueuedMedia? = null var cropImageItemOld: QueuedMedia? = null
@ -213,15 +215,8 @@ class ComposeViewModel @Inject constructor(
} }
fun didChange(content: String?, contentWarning: String?): Boolean { fun didChange(content: String?, contentWarning: String?): Boolean {
val textChanged = content.orEmpty() != startingText.orEmpty()
val textChanged = !( val contentWarningChanged = contentWarning.orEmpty() != startingContentWarning
content.isNullOrEmpty() ||
startingText?.startsWith(content.toString()) ?: false
)
val contentWarningChanged = showContentWarning.value &&
!contentWarning.isNullOrEmpty() &&
!startingContentWarning.startsWith(contentWarning.toString())
val mediaChanged = media.value.isNotEmpty() val mediaChanged = media.value.isNotEmpty()
val pollChanged = poll.value != null val pollChanged = poll.value != null
val didScheduledTimeChange = hasScheduledTimeChanged val didScheduledTimeChange = hasScheduledTimeChanged
@ -411,6 +406,8 @@ class ComposeViewModel @Inject constructor(
return return
} }
composeKind = composeOptions?.kind ?: ComposeActivity.ComposeKind.NEW
val preferredVisibility = accountManager.activeAccount!!.defaultPostPrivacy val preferredVisibility = accountManager.activeAccount!!.defaultPostPrivacy
val replyVisibility = composeOptions?.replyVisibility ?: Status.Visibility.UNKNOWN val replyVisibility = composeOptions?.replyVisibility ?: Status.Visibility.UNKNOWN

View file

@ -112,6 +112,7 @@ class DraftsActivity : BaseActivity(), DraftActionListener {
scheduledAt = draft.scheduledAt, scheduledAt = draft.scheduledAt,
language = draft.language, language = draft.language,
statusId = draft.statusId, statusId = draft.statusId,
kind = ComposeActivity.ComposeKind.EDIT_DRAFT
) )
bottomSheet.state = BottomSheetBehavior.STATE_HIDDEN bottomSheet.state = BottomSheetBehavior.STATE_HIDDEN
@ -149,6 +150,7 @@ class DraftsActivity : BaseActivity(), DraftActionListener {
scheduledAt = draft.scheduledAt, scheduledAt = draft.scheduledAt,
language = draft.language, language = draft.language,
statusId = draft.statusId, statusId = draft.statusId,
kind = ComposeActivity.ComposeKind.EDIT_DRAFT
) )
startActivity(ComposeActivity.startIntent(this, composeOptions)) startActivity(ComposeActivity.startIntent(this, composeOptions))

View file

@ -371,6 +371,7 @@ public class NotificationHelper {
composeOptions.setMentionedUsernames(mentionedUsernames); composeOptions.setMentionedUsernames(mentionedUsernames);
composeOptions.setModifiedInitialState(true); composeOptions.setModifiedInitialState(true);
composeOptions.setLanguage(actionableStatus.getLanguage()); composeOptions.setLanguage(actionableStatus.getLanguage());
composeOptions.setKind(ComposeActivity.ComposeKind.NEW);
Intent composeIntent = ComposeActivity.startIntent( Intent composeIntent = ComposeActivity.startIntent(
context, context,

View file

@ -128,7 +128,8 @@ class ScheduledStatusActivity : BaseActivity(), ScheduledStatusActionListener, I
inReplyToId = item.params.inReplyToId, inReplyToId = item.params.inReplyToId,
visibility = item.params.visibility, visibility = item.params.visibility,
scheduledAt = item.scheduledAt, scheduledAt = item.scheduledAt,
sensitive = item.params.sensitive sensitive = item.params.sensitive,
kind = ComposeActivity.ComposeKind.EDIT_SCHEDULED
) )
) )
startActivity(intent) startActivity(intent)

View file

@ -223,6 +223,7 @@ class SearchStatusesFragment : SearchFragment<StatusViewData.Concrete>(), Status
replyingStatusAuthor = actionableStatus.account.localUsername, replyingStatusAuthor = actionableStatus.account.localUsername,
replyingStatusContent = status.content.toString(), replyingStatusContent = status.content.toString(),
language = actionableStatus.language, language = actionableStatus.language,
kind = ComposeActivity.ComposeKind.NEW
) )
) )
bottomSheetActivity?.startActivityWithSlideInAnimation(intent) bottomSheetActivity?.startActivityWithSlideInAnimation(intent)
@ -481,6 +482,7 @@ class SearchStatusesFragment : SearchFragment<StatusViewData.Concrete>(), Status
sensitive = redraftStatus.sensitive, sensitive = redraftStatus.sensitive,
poll = redraftStatus.poll?.toNewPoll(status.createdAt), poll = redraftStatus.poll?.toNewPoll(status.createdAt),
language = redraftStatus.language, language = redraftStatus.language,
kind = ComposeActivity.ComposeKind.NEW
) )
) )
startActivity(intent) startActivity(intent)
@ -510,6 +512,7 @@ class SearchStatusesFragment : SearchFragment<StatusViewData.Concrete>(), Status
language = status.language, language = status.language,
statusId = source.id, statusId = source.id,
poll = status.poll?.toNewPoll(status.createdAt), poll = status.poll?.toNewPoll(status.createdAt),
kind = ComposeActivity.ComposeKind.EDIT_POSTED,
) )
startActivity(ComposeActivity.startIntent(requireContext(), composeOptions)) startActivity(ComposeActivity.startIntent(requireContext(), composeOptions))
}, },

View file

@ -45,6 +45,7 @@ import com.keylesspalace.tusky.PostLookupFallbackBehavior
import com.keylesspalace.tusky.R import com.keylesspalace.tusky.R
import com.keylesspalace.tusky.StatusListActivity.Companion.newHashtagIntent import com.keylesspalace.tusky.StatusListActivity.Companion.newHashtagIntent
import com.keylesspalace.tusky.ViewMediaActivity.Companion.newIntent import com.keylesspalace.tusky.ViewMediaActivity.Companion.newIntent
import com.keylesspalace.tusky.components.compose.ComposeActivity
import com.keylesspalace.tusky.components.compose.ComposeActivity.Companion.startIntent import com.keylesspalace.tusky.components.compose.ComposeActivity.Companion.startIntent
import com.keylesspalace.tusky.components.compose.ComposeActivity.ComposeOptions import com.keylesspalace.tusky.components.compose.ComposeActivity.ComposeOptions
import com.keylesspalace.tusky.components.report.ReportActivity.Companion.getIntent import com.keylesspalace.tusky.components.report.ReportActivity.Companion.getIntent
@ -135,6 +136,7 @@ abstract class SFragment : Fragment(), Injectable {
replyingStatusAuthor = account.localUsername, replyingStatusAuthor = account.localUsername,
replyingStatusContent = actionableStatus.content.parseAsMastodonHtml().toString(), replyingStatusContent = actionableStatus.content.parseAsMastodonHtml().toString(),
language = actionableStatus.language, language = actionableStatus.language,
kind = ComposeActivity.ComposeKind.NEW
) )
val intent = startIntent(requireContext(), composeOptions) val intent = startIntent(requireContext(), composeOptions)
@ -411,6 +413,7 @@ abstract class SFragment : Fragment(), Injectable {
modifiedInitialState = true, modifiedInitialState = true,
language = sourceStatus.language, language = sourceStatus.language,
poll = sourceStatus.poll?.toNewPoll(sourceStatus.createdAt), poll = sourceStatus.poll?.toNewPoll(sourceStatus.createdAt),
kind = ComposeActivity.ComposeKind.NEW
) )
startActivity(startIntent(requireContext(), composeOptions)) startActivity(startIntent(requireContext(), composeOptions))
} }
@ -437,6 +440,7 @@ abstract class SFragment : Fragment(), Injectable {
language = status.language, language = status.language,
statusId = source.id, statusId = source.id,
poll = status.poll?.toNewPoll(status.createdAt), poll = status.poll?.toNewPoll(status.createdAt),
kind = ComposeActivity.ComposeKind.EDIT_POSTED
) )
startActivity(startIntent(requireContext(), composeOptions)) startActivity(startIntent(requireContext(), composeOptions))
}, },

View file

@ -109,6 +109,8 @@
<string name="action_delete">Delete</string> <string name="action_delete">Delete</string>
<string name="action_delete_conversation">Delete conversation</string> <string name="action_delete_conversation">Delete conversation</string>
<string name="action_delete_and_redraft">Delete and re-draft</string> <string name="action_delete_and_redraft">Delete and re-draft</string>
<string name="action_discard">Discard changes</string>
<string name="action_continue_edit">Continue editing</string>
<string name="action_send">TOOT</string> <string name="action_send">TOOT</string>
<string name="action_send_public">TOOT!</string> <string name="action_send_public">TOOT!</string>
<string name="action_retry">Retry</string> <string name="action_retry">Retry</string>
@ -443,6 +445,7 @@
<string name="lock_account_label_description">Requires you to manually approve followers</string> <string name="lock_account_label_description">Requires you to manually approve followers</string>
<string name="compose_save_draft">Save draft?</string> <string name="compose_save_draft">Save draft?</string>
<string name="compose_save_draft_loses_media">Save draft? (Attachments will be uploaded again when you restore the draft.)</string> <string name="compose_save_draft_loses_media">Save draft? (Attachments will be uploaded again when you restore the draft.)</string>
<string name="compose_unsaved_changes">You have unsaved changes.</string>
<string name="send_post_notification_title">Sending post…</string> <string name="send_post_notification_title">Sending post…</string>
<string name="send_post_notification_error_title">Error sending post</string> <string name="send_post_notification_error_title">Error sending post</string>
<string name="send_post_notification_channel_name">Sending Posts</string> <string name="send_post_notification_channel_name">Sending Posts</string>