From a5f479d79cbae5fa567df53951146932a135ca47 Mon Sep 17 00:00:00 2001 From: Nik Clayton Date: Sat, 31 Dec 2022 13:04:49 +0100 Subject: [PATCH] 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 --- .../components/account/AccountActivity.kt | 7 +- .../components/compose/ComposeActivity.kt | 104 +++++++++++++++--- .../components/compose/ComposeViewModel.kt | 15 +-- .../tusky/components/drafts/DraftsActivity.kt | 2 + .../notifications/NotificationHelper.java | 1 + .../scheduled/ScheduledStatusActivity.kt | 3 +- .../fragments/SearchStatusesFragment.kt | 3 + .../keylesspalace/tusky/fragment/SFragment.kt | 4 + app/src/main/res/values/strings.xml | 3 + 9 files changed, 112 insertions(+), 30 deletions(-) diff --git a/app/src/main/java/com/keylesspalace/tusky/components/account/AccountActivity.kt b/app/src/main/java/com/keylesspalace/tusky/components/account/AccountActivity.kt index 0d3e05cb..b46d6124 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/account/AccountActivity.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/account/AccountActivity.kt @@ -811,9 +811,12 @@ class AccountActivity : BottomSheetActivity(), ActionButtonActivity, HasAndroidI private fun mention() { loadedAccount?.let { val options = if (viewModel.isSelf) { - ComposeActivity.ComposeOptions() + ComposeActivity.ComposeOptions(kind = ComposeActivity.ComposeKind.NEW) } else { - ComposeActivity.ComposeOptions(mentionedUsernames = setOf(it.username)) + ComposeActivity.ComposeOptions( + mentionedUsernames = setOf(it.username), + kind = ComposeActivity.ComposeKind.NEW + ) } val intent = ComposeActivity.startIntent(this, options) startActivity(intent) diff --git a/app/src/main/java/com/keylesspalace/tusky/components/compose/ComposeActivity.kt b/app/src/main/java/com/keylesspalace/tusky/components/compose/ComposeActivity.kt index 5f708b82..2fb2ff9a 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/compose/ComposeActivity.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/compose/ComposeActivity.kt @@ -1115,30 +1115,79 @@ class ComposeActivity : val contentText = binding.composeEditField.text.toString() val contentWarning = binding.composeContentWarningField.text.toString() if (viewModel.didChange(contentText, contentWarning)) { - - val warning = if (!viewModel.media.value.isEmpty()) { - R.string.compose_save_draft_loses_media - } else { - R.string.compose_save_draft - } - - AlertDialog.Builder(this) - .setMessage(warning) - .setPositiveButton(R.string.action_save) { _, _ -> - viewModel.stopUploads() - saveDraftAndFinish(contentText, contentWarning) - } - .setNegativeButton(R.string.action_delete) { _, _ -> - viewModel.stopUploads() - deleteDraftAndFinish() - } - .show() + when (viewModel.composeKind) { + ComposeKind.NEW -> getSaveAsDraftOrDiscardDialog(contentText, contentWarning) + ComposeKind.EDIT_DRAFT -> getUpdateDraftOrDiscardDialog(contentText, contentWarning) + ComposeKind.EDIT_POSTED -> getContinueEditingOrDiscardDialog() + ComposeKind.EDIT_SCHEDULED -> getContinueEditingOrDiscardDialog() + }.show() } else { viewModel.stopUploads() 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() { viewModel.deleteDraft() 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 data class ComposeOptions( // Let's keep fields var until all consumers are Kotlin @@ -1240,6 +1307,7 @@ class ComposeActivity : var modifiedInitialState: Boolean? = null, var language: String? = null, var statusId: String? = null, + var kind: ComposeKind? = null ) : Parcelable companion object { diff --git a/app/src/main/java/com/keylesspalace/tusky/components/compose/ComposeViewModel.kt b/app/src/main/java/com/keylesspalace/tusky/components/compose/ComposeViewModel.kt index e697fad6..75cad529 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/compose/ComposeViewModel.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/compose/ComposeViewModel.kt @@ -95,6 +95,8 @@ class ComposeViewModel @Inject constructor( val media: MutableStateFlow> = MutableStateFlow(emptyList()) val uploadError = MutableSharedFlow(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 var cropImageItemOld: QueuedMedia? = null @@ -213,15 +215,8 @@ class ComposeViewModel @Inject constructor( } fun didChange(content: String?, contentWarning: String?): Boolean { - - val textChanged = !( - content.isNullOrEmpty() || - startingText?.startsWith(content.toString()) ?: false - ) - - val contentWarningChanged = showContentWarning.value && - !contentWarning.isNullOrEmpty() && - !startingContentWarning.startsWith(contentWarning.toString()) + val textChanged = content.orEmpty() != startingText.orEmpty() + val contentWarningChanged = contentWarning.orEmpty() != startingContentWarning val mediaChanged = media.value.isNotEmpty() val pollChanged = poll.value != null val didScheduledTimeChange = hasScheduledTimeChanged @@ -411,6 +406,8 @@ class ComposeViewModel @Inject constructor( return } + composeKind = composeOptions?.kind ?: ComposeActivity.ComposeKind.NEW + val preferredVisibility = accountManager.activeAccount!!.defaultPostPrivacy val replyVisibility = composeOptions?.replyVisibility ?: Status.Visibility.UNKNOWN diff --git a/app/src/main/java/com/keylesspalace/tusky/components/drafts/DraftsActivity.kt b/app/src/main/java/com/keylesspalace/tusky/components/drafts/DraftsActivity.kt index ba0c215d..7cdcd259 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/drafts/DraftsActivity.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/drafts/DraftsActivity.kt @@ -112,6 +112,7 @@ class DraftsActivity : BaseActivity(), DraftActionListener { scheduledAt = draft.scheduledAt, language = draft.language, statusId = draft.statusId, + kind = ComposeActivity.ComposeKind.EDIT_DRAFT ) bottomSheet.state = BottomSheetBehavior.STATE_HIDDEN @@ -149,6 +150,7 @@ class DraftsActivity : BaseActivity(), DraftActionListener { scheduledAt = draft.scheduledAt, language = draft.language, statusId = draft.statusId, + kind = ComposeActivity.ComposeKind.EDIT_DRAFT ) startActivity(ComposeActivity.startIntent(this, composeOptions)) diff --git a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationHelper.java b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationHelper.java index af70cce2..41853cc1 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationHelper.java +++ b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationHelper.java @@ -371,6 +371,7 @@ public class NotificationHelper { composeOptions.setMentionedUsernames(mentionedUsernames); composeOptions.setModifiedInitialState(true); composeOptions.setLanguage(actionableStatus.getLanguage()); + composeOptions.setKind(ComposeActivity.ComposeKind.NEW); Intent composeIntent = ComposeActivity.startIntent( context, diff --git a/app/src/main/java/com/keylesspalace/tusky/components/scheduled/ScheduledStatusActivity.kt b/app/src/main/java/com/keylesspalace/tusky/components/scheduled/ScheduledStatusActivity.kt index 43ce346d..ade93322 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/scheduled/ScheduledStatusActivity.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/scheduled/ScheduledStatusActivity.kt @@ -128,7 +128,8 @@ class ScheduledStatusActivity : BaseActivity(), ScheduledStatusActionListener, I inReplyToId = item.params.inReplyToId, visibility = item.params.visibility, scheduledAt = item.scheduledAt, - sensitive = item.params.sensitive + sensitive = item.params.sensitive, + kind = ComposeActivity.ComposeKind.EDIT_SCHEDULED ) ) startActivity(intent) diff --git a/app/src/main/java/com/keylesspalace/tusky/components/search/fragments/SearchStatusesFragment.kt b/app/src/main/java/com/keylesspalace/tusky/components/search/fragments/SearchStatusesFragment.kt index b261ce31..103bcfc5 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/search/fragments/SearchStatusesFragment.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/search/fragments/SearchStatusesFragment.kt @@ -223,6 +223,7 @@ class SearchStatusesFragment : SearchFragment(), Status replyingStatusAuthor = actionableStatus.account.localUsername, replyingStatusContent = status.content.toString(), language = actionableStatus.language, + kind = ComposeActivity.ComposeKind.NEW ) ) bottomSheetActivity?.startActivityWithSlideInAnimation(intent) @@ -481,6 +482,7 @@ class SearchStatusesFragment : SearchFragment(), Status sensitive = redraftStatus.sensitive, poll = redraftStatus.poll?.toNewPoll(status.createdAt), language = redraftStatus.language, + kind = ComposeActivity.ComposeKind.NEW ) ) startActivity(intent) @@ -510,6 +512,7 @@ class SearchStatusesFragment : SearchFragment(), Status language = status.language, statusId = source.id, poll = status.poll?.toNewPoll(status.createdAt), + kind = ComposeActivity.ComposeKind.EDIT_POSTED, ) startActivity(ComposeActivity.startIntent(requireContext(), composeOptions)) }, diff --git a/app/src/main/java/com/keylesspalace/tusky/fragment/SFragment.kt b/app/src/main/java/com/keylesspalace/tusky/fragment/SFragment.kt index 9b217f84..32808cce 100644 --- a/app/src/main/java/com/keylesspalace/tusky/fragment/SFragment.kt +++ b/app/src/main/java/com/keylesspalace/tusky/fragment/SFragment.kt @@ -45,6 +45,7 @@ import com.keylesspalace.tusky.PostLookupFallbackBehavior import com.keylesspalace.tusky.R import com.keylesspalace.tusky.StatusListActivity.Companion.newHashtagIntent 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.ComposeOptions import com.keylesspalace.tusky.components.report.ReportActivity.Companion.getIntent @@ -135,6 +136,7 @@ abstract class SFragment : Fragment(), Injectable { replyingStatusAuthor = account.localUsername, replyingStatusContent = actionableStatus.content.parseAsMastodonHtml().toString(), language = actionableStatus.language, + kind = ComposeActivity.ComposeKind.NEW ) val intent = startIntent(requireContext(), composeOptions) @@ -411,6 +413,7 @@ abstract class SFragment : Fragment(), Injectable { modifiedInitialState = true, language = sourceStatus.language, poll = sourceStatus.poll?.toNewPoll(sourceStatus.createdAt), + kind = ComposeActivity.ComposeKind.NEW ) startActivity(startIntent(requireContext(), composeOptions)) } @@ -437,6 +440,7 @@ abstract class SFragment : Fragment(), Injectable { language = status.language, statusId = source.id, poll = status.poll?.toNewPoll(status.createdAt), + kind = ComposeActivity.ComposeKind.EDIT_POSTED ) startActivity(startIntent(requireContext(), composeOptions)) }, diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 6fcc16b0..2a66137f 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -109,6 +109,8 @@ Delete Delete conversation Delete and re-draft + Discard changes + Continue editing TOOT TOOT! Retry @@ -443,6 +445,7 @@ Requires you to manually approve followers Save draft? Save draft? (Attachments will be uploaded again when you restore the draft.) + You have unsaved changes. Sending post… Error sending post Sending Posts