diff --git a/app/src/main/kotlin/com/maltaisn/notes/ui/edit/EditFragment.kt b/app/src/main/kotlin/com/maltaisn/notes/ui/edit/EditFragment.kt index 2808fba7..840f4c8d 100644 --- a/app/src/main/kotlin/com/maltaisn/notes/ui/edit/EditFragment.kt +++ b/app/src/main/kotlin/com/maltaisn/notes/ui/edit/EditFragment.kt @@ -28,7 +28,9 @@ import androidx.core.view.postDelayed import androidx.fragment.app.Fragment import androidx.navigation.fragment.findNavController import androidx.navigation.fragment.navArgs +import androidx.recyclerview.widget.DefaultItemAnimator import androidx.recyclerview.widget.LinearLayoutManager +import androidx.recyclerview.widget.RecyclerView import com.google.android.material.snackbar.Snackbar import com.maltaisn.notes.App import com.maltaisn.notes.hideKeyboard @@ -116,7 +118,25 @@ class EditFragment : Fragment(), Toolbar.OnMenuItemClickListener, ConfirmDialog. val layoutManager = LinearLayoutManager(context) rcv.adapter = adapter rcv.layoutManager = layoutManager - rcv.itemAnimator?.changeDuration = 0 + rcv.itemAnimator = object : DefaultItemAnimator() { + override fun animateAppearance( + viewHolder: RecyclerView.ViewHolder, + preLayoutInfo: ItemHolderInfo?, + postLayoutInfo: ItemHolderInfo + ): Boolean { + return if (preLayoutInfo != null && (preLayoutInfo.left != postLayoutInfo.left + || preLayoutInfo.top != postLayoutInfo.top) + ) { + // item move, handle normally + super.animateAppearance(viewHolder, preLayoutInfo, postLayoutInfo) + } else { + // do not animate new item appearance + // this is mainly to avoid animating the whole list when fragment view is recreated. + dispatchAddFinished(viewHolder) + false + } + } + } setupViewModelObservers(adapter) } diff --git a/app/src/main/kotlin/com/maltaisn/notes/ui/edit/EditViewModel.kt b/app/src/main/kotlin/com/maltaisn/notes/ui/edit/EditViewModel.kt index b3a14a9b..8997b8ba 100644 --- a/app/src/main/kotlin/com/maltaisn/notes/ui/edit/EditViewModel.kt +++ b/app/src/main/kotlin/com/maltaisn/notes/ui/edit/EditViewModel.kt @@ -52,6 +52,7 @@ import com.maltaisn.notes.ui.note.ShownDateField import com.maltaisn.notes.ui.send import com.squareup.inject.assisted.Assisted import com.squareup.inject.assisted.AssistedInject +import kotlinx.coroutines.Job import kotlinx.coroutines.NonCancellable import kotlinx.coroutines.launch import java.util.Collections @@ -59,7 +60,6 @@ import java.util.Date /** * View model for the edit note screen. - * TODO status, pinned, reminder could be livedata, except that livedata has nullable type... flow? */ class EditViewModel @AssistedInject constructor( private val notesRepository: NotesRepository, @@ -106,7 +106,15 @@ class EditViewModel @AssistedInject constructor( /** * The currently displayed list items created in [updateListItems]. - * While this list is mutable, any in place changes should be reported to the adapter! + * + * While this list is mutable, any in place changes should be reported to the adapter! This is used in the case + * of moving items, where the view model updates the list but the adapter already knows of the move. + * + * Note the in the case of list items, a specific item has no identity. Its position and its content + * can change at any time, so it can be associated with a stable ID. This is problematic for item diff callback + * and animations, which would rely on ID. Instead, the diff callback was made to rely on identity so that + * add/remove animations take place correctly. The recycler view was also set up to not animate item appearance, + * so that if the list is completely recreated, no animation will occur despite all items identity being lost. */ private var listItems: MutableList = mutableListOf() set(value) { @@ -182,6 +190,8 @@ class EditViewModel @AssistedInject constructor( private val isNoteInTrash: Boolean get() = status == NoteStatus.DELETED + private var updateNoteJob: Job? = null + init { if (KEY_NOTE_ID in savedStateHandle) { viewModelScope.launch { @@ -205,6 +215,10 @@ class EditViewModel @AssistedInject constructor( */ fun start(noteId: Long = Note.NO_ID, labelId: Long = Label.NO_ID) { viewModelScope.launch { + // If fragment was very briefly destroyed then recreated, it's possible that this job is launched + // before the job to save the note on fragment destruction is called. + updateNoteJob?.join() + val isFirstStart = (note == BLANK_NOTE) // Try to get note by ID with its labels. @@ -268,8 +282,8 @@ class EditViewModel @AssistedInject constructor( // Update note updateNote() - // NonCancellable to avoid save being cancelled if called right before view model destruction - viewModelScope.launch(NonCancellable) { + // NonCancellable to avoid save being cancelled if called right before fragment destruction + updateNoteJob = viewModelScope.launch(NonCancellable) { // Compare previously saved note from database with new one. // It is possible that note will be null here, if: // - Back arrow is clicked, saving note. @@ -288,6 +302,7 @@ class EditViewModel @AssistedInject constructor( } notesRepository.updateNote(note) + updateNoteJob = null } } } @@ -560,9 +575,6 @@ class EditViewModel @AssistedInject constructor( * [updateNote] might need to be called first for that data to be up-to-date. */ private fun updateListItems() { - // TODO avoid full list refresh somehow? - // - on note type change - // - on fragment view recreation listItems = createListItems() } diff --git a/app/src/main/kotlin/com/maltaisn/notes/ui/home/HomeViewModel.kt b/app/src/main/kotlin/com/maltaisn/notes/ui/home/HomeViewModel.kt index 08536c15..aebfcf6b 100644 --- a/app/src/main/kotlin/com/maltaisn/notes/ui/home/HomeViewModel.kt +++ b/app/src/main/kotlin/com/maltaisn/notes/ui/home/HomeViewModel.kt @@ -82,9 +82,7 @@ class HomeViewModel @AssistedInject constructor( get() = _showEmptyTrashDialogEvent init { - viewModelScope.launch { - restoreState() - } + restoreState() } fun setDestination(destination: HomeDestination) { diff --git a/app/src/main/kotlin/com/maltaisn/notes/ui/labels/LabelViewModel.kt b/app/src/main/kotlin/com/maltaisn/notes/ui/labels/LabelViewModel.kt index 7477e13a..d5cce05b 100644 --- a/app/src/main/kotlin/com/maltaisn/notes/ui/labels/LabelViewModel.kt +++ b/app/src/main/kotlin/com/maltaisn/notes/ui/labels/LabelViewModel.kt @@ -34,8 +34,6 @@ import com.squareup.inject.assisted.AssistedInject import kotlinx.coroutines.Job import kotlinx.coroutines.flow.collect import kotlinx.coroutines.launch -import kotlinx.coroutines.yield -import java.util.concurrent.atomic.AtomicBoolean class LabelViewModel @AssistedInject constructor( private val labelsRepository: LabelsRepository, @@ -109,17 +107,16 @@ class LabelViewModel @AssistedInject constructor( private var renamingLabel = false private var labelsListJob: Job? = null - - private val stateRestored = AtomicBoolean(false) + private var restoreStateJob: Job? = null init { - viewModelScope.launch { + restoreStateJob = viewModelScope.launch { // Restore state noteIds = savedStateHandle.get>(KEY_NOTE_IDS).orEmpty() selectedLabelIds += savedStateHandle.get>(KEY_SELECTED_IDS).orEmpty() selectedLabels += selectedLabelIds.mapNotNull { labelsRepository.getLabelById(it) } renamingLabel = savedStateHandle[KEY_RENAMING_LABEL] ?: false - stateRestored.set(true) + restoreStateJob = null } } @@ -148,9 +145,7 @@ class LabelViewModel @AssistedInject constructor( labelsRepository.getAllLabelsByUsage().collect { labels -> // Since state restoration is suspending, ensure that state is properly restored // (e.g. selection) before setting list items, or selection may be lost. - while (!stateRestored.get()) { - yield() - } + restoreStateJob?.join() if (renamingLabel) { // List was updated after renaming label, this can only be due to label rename. @@ -295,5 +290,4 @@ class LabelViewModel @AssistedInject constructor( private const val KEY_SELECTED_IDS = "selected_ids" private const val KEY_RENAMING_LABEL = "renaming_label" } - } diff --git a/app/src/main/kotlin/com/maltaisn/notes/ui/note/NoteViewModel.kt b/app/src/main/kotlin/com/maltaisn/notes/ui/note/NoteViewModel.kt index 980c0554..7ebc5330 100644 --- a/app/src/main/kotlin/com/maltaisn/notes/ui/note/NoteViewModel.kt +++ b/app/src/main/kotlin/com/maltaisn/notes/ui/note/NoteViewModel.kt @@ -38,10 +38,9 @@ import com.maltaisn.notes.ui.note.adapter.NoteItem import com.maltaisn.notes.ui.note.adapter.NoteListItem import com.maltaisn.notes.ui.note.adapter.NoteListLayoutMode import com.maltaisn.notes.ui.send +import kotlinx.coroutines.Job import kotlinx.coroutines.launch -import kotlinx.coroutines.yield import java.util.Date -import java.util.concurrent.atomic.AtomicBoolean /** * This view model provides common behavior for home and search view models. @@ -132,7 +131,7 @@ abstract class NoteViewModel( val showDeleteConfirmEvent: LiveData> get() = _showDeletedForeverConfirmEvent - private var stateRestored = AtomicBoolean(false) + private var restoreStateJob: Job? = null init { // Initialize list layout to saved value. @@ -145,19 +144,19 @@ abstract class NoteViewModel( * Notice that state restoration is suspending, so when initializing the child view model, * [waitForRestoredState] must be called to wait for state restoration to be complete. */ - protected open suspend fun restoreState() { - // Restore saved selected notes - selectedNoteIds += savedStateHandle.get>(KEY_SELECTED_IDS) - .orEmpty().toMutableSet() - _selectedNotes += selectedNoteIds.mapNotNull { notesRepository.getNoteById(it) } - updateNoteSelection() - stateRestored.set(true) + protected open fun restoreState() { + restoreStateJob = viewModelScope.launch { + // Restore saved selected notes + selectedNoteIds += savedStateHandle.get>(KEY_SELECTED_IDS) + .orEmpty().toMutableSet() + _selectedNotes += selectedNoteIds.mapNotNull { notesRepository.getNoteById(it) } + updateNoteSelection() + restoreStateJob = null + } } protected suspend fun waitForRestoredState() { - while (!stateRestored.get()) { - yield() - } + restoreStateJob?.join() } /** diff --git a/app/src/main/kotlin/com/maltaisn/notes/ui/notification/NotificationActivity.kt b/app/src/main/kotlin/com/maltaisn/notes/ui/notification/NotificationActivity.kt index 61cbad70..87a1c529 100644 --- a/app/src/main/kotlin/com/maltaisn/notes/ui/notification/NotificationActivity.kt +++ b/app/src/main/kotlin/com/maltaisn/notes/ui/notification/NotificationActivity.kt @@ -18,12 +18,10 @@ package com.maltaisn.notes.ui.notification import android.content.Intent import android.os.Bundle -import android.util.Log import androidx.appcompat.app.AppCompatActivity import androidx.core.app.NotificationManagerCompat import androidx.navigation.findNavController import com.maltaisn.notes.App -import com.maltaisn.notes.TAG import com.maltaisn.notes.model.entity.Note import com.maltaisn.notes.navigateSafe import com.maltaisn.notes.receiver.AlarmReceiver @@ -68,7 +66,6 @@ class NotificationActivity : AppCompatActivity() { when (intent.action) { INTENT_ACTION_POSTPONE -> { val noteId = intent.getLongExtra(AlarmReceiver.EXTRA_NOTE_ID, Note.NO_ID) - Log.d(TAG, "note ID for postpone is $noteId") NotificationManagerCompat.from(this).cancel(noteId.toInt()) viewModel.onPostponeClicked(noteId) } diff --git a/app/src/main/kotlin/com/maltaisn/notes/ui/reminder/ReminderDialog.kt b/app/src/main/kotlin/com/maltaisn/notes/ui/reminder/ReminderDialog.kt index b68cca2f..a0ec2706 100644 --- a/app/src/main/kotlin/com/maltaisn/notes/ui/reminder/ReminderDialog.kt +++ b/app/src/main/kotlin/com/maltaisn/notes/ui/reminder/ReminderDialog.kt @@ -19,7 +19,6 @@ package com.maltaisn.notes.ui.reminder import android.app.Dialog import android.content.DialogInterface import android.os.Bundle -import android.util.Log import android.view.LayoutInflater import androidx.appcompat.app.AlertDialog import androidx.core.view.isVisible @@ -28,7 +27,6 @@ import androidx.navigation.fragment.findNavController import androidx.navigation.fragment.navArgs import com.google.android.material.dialog.MaterialAlertDialogBuilder import com.maltaisn.notes.App -import com.maltaisn.notes.TAG import com.maltaisn.notes.contains import com.maltaisn.notes.navigateSafe import com.maltaisn.notes.setMaxWidth @@ -67,7 +65,6 @@ class ReminderDialog : DialogFragment(), RecurrenceListCallback, RecurrencePicke override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) (requireContext().applicationContext as App).appComponent.inject(this) - Log.d(TAG, "Creating ReminderDialog fragment") } override fun onCreateDialog(savedInstanceState: Bundle?): Dialog {