-
-
Notifications
You must be signed in to change notification settings - Fork 36
fix: dialog view state preservation #284
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
22113b0
198bbb4
fb9e246
eae0176
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -7,6 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 | |||
| ## [Unreleased] | ||||
|
|
||||
| ### Fixed | ||||
| - Fixed checklist dialog disappearing on screen rotation ([#744]) | ||||
| - Fixed inconsistent checklist sorting when the "Move checked items to the bottom" option is enabled ([#59]) | ||||
|
|
||||
| ## [1.6.0] - 2025-10-29 | ||||
|
|
@@ -105,6 +106,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 | |||
| [#178]: https://github.com/FossifyOrg/Notes/issues/178 | ||||
| [#190]: https://github.com/FossifyOrg/Notes/issues/190 | ||||
| [#201]: https://github.com/FossifyOrg/Notes/issues/201 | ||||
| [#744]: https://github.com/FossifyOrg/General-Discussion/issues/744 | ||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||
|
|
||||
| [Unreleased]: https://github.com/FossifyOrg/Notes/compare/1.6.0...HEAD | ||||
| [1.6.0]: https://github.com/FossifyOrg/Notes/compare/1.5.0...1.6.0 | ||||
|
|
||||
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
| @@ -0,0 +1,147 @@ | ||||
| package org.fossify.notes.dialogs | ||||
|
|
||||
| import android.app.Dialog | ||||
| import android.os.Bundle | ||||
| import android.view.View | ||||
| import android.view.WindowManager | ||||
| import androidx.appcompat.app.AlertDialog | ||||
| import androidx.appcompat.widget.AppCompatEditText | ||||
| import androidx.fragment.app.DialogFragment | ||||
| import org.fossify.notes.R | ||||
| import org.fossify.notes.databinding.DialogNewChecklistItemBinding | ||||
| import org.fossify.notes.databinding.ItemAddChecklistBinding | ||||
| import org.fossify.commons.extensions.getAlertDialogBuilder | ||||
| import org.fossify.commons.extensions.getContrastColor | ||||
| import org.fossify.commons.extensions.getProperPrimaryColor | ||||
| import org.fossify.commons.extensions.showKeyboard | ||||
| import org.fossify.commons.R as CommonsR | ||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can keep using the fully qualified R class name for this as that's the pattern everywhere in this project.
Suggested change
|
||||
|
|
||||
| class ChecklistItemDialogFragment : DialogFragment() { | ||||
|
|
||||
| private val activeInputFields = mutableListOf<AppCompatEditText>() | ||||
| private var binding: DialogNewChecklistItemBinding? = null | ||||
|
|
||||
| companion object { | ||||
| const val DIALOG_TAG = "ChecklistItemDialogFragment" | ||||
| const val REQUEST_KEY = "ChecklistItemRequest" | ||||
| const val RESULT_TEXT_KEY = "ResultText" | ||||
| const val RESULT_TASK_ID_KEY = "ResultTaskId" | ||||
|
|
||||
| private const val ARG_TEXT = "ArgText" | ||||
| private const val ARG_TASK_ID = "ArgTaskId" | ||||
| private const val SAVED_STATE_TEXTS = "SavedStateTexts" | ||||
|
Comment on lines
+25
to
+32
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IDs should use snake case for consistency. |
||||
|
|
||||
| fun newInstance(taskId: Int = -1, text: String = ""): ChecklistItemDialogFragment { | ||||
| val fragment = ChecklistItemDialogFragment() | ||||
| val args = Bundle() | ||||
| args.putInt(ARG_TASK_ID, taskId) | ||||
| args.putString(ARG_TEXT, text) | ||||
| fragment.arguments = args | ||||
| return fragment | ||||
| } | ||||
| } | ||||
|
|
||||
| override fun onCreateDialog(savedInstanceState: Bundle?): Dialog { | ||||
| val activity = requireActivity() | ||||
| val taskId = arguments?.getInt(ARG_TASK_ID) ?: -1 | ||||
|
|
||||
| binding = DialogNewChecklistItemBinding.inflate(activity.layoutInflater) | ||||
|
|
||||
| activeInputFields.clear() | ||||
|
|
||||
| // Restore rows | ||||
| if (savedInstanceState != null) { | ||||
| val savedTexts = savedInstanceState.getStringArrayList(SAVED_STATE_TEXTS) | ||||
| if (!savedTexts.isNullOrEmpty()) { | ||||
| savedTexts.forEach { text -> addNewRow(text) } | ||||
| } else { | ||||
| addNewRow("") | ||||
| } | ||||
| } else { | ||||
| val initialText = arguments?.getString(ARG_TEXT) ?: "" | ||||
| addNewRow(initialText) | ||||
| } | ||||
|
|
||||
| val isNewTaskMode = (taskId == -1) | ||||
| if (isNewTaskMode) { | ||||
| val contrastColor = activity.getProperPrimaryColor().getContrastColor() | ||||
| binding!!.addItem.setColorFilter(contrastColor) | ||||
|
|
||||
| binding!!.addItem.setOnClickListener { | ||||
| addNewRow("") | ||||
| } | ||||
| } else { | ||||
| binding!!.addItem.visibility = View.GONE | ||||
| binding!!.settingsAddChecklistTop.visibility = View.GONE | ||||
| } | ||||
|
|
||||
| val titleRes = if (isNewTaskMode) R.string.add_new_checklist_items else R.string.rename_note | ||||
|
|
||||
| val builder = activity.getAlertDialogBuilder() | ||||
| .setTitle(titleRes) | ||||
| .setView(binding!!.root) | ||||
| .setPositiveButton(CommonsR.string.ok, null) | ||||
| .setNegativeButton(CommonsR.string.cancel, null) | ||||
|
|
||||
| val dialog = builder.create() | ||||
| dialog.window?.setSoftInputMode(WindowManager.LayoutParams.SOFT_INPUT_STATE_VISIBLE) | ||||
|
|
||||
| dialog.setOnShowListener { | ||||
| if (activeInputFields.isNotEmpty()) { | ||||
| dialog.showKeyboard(activeInputFields.last()) | ||||
| } | ||||
|
|
||||
| val positiveButton = (dialog as AlertDialog).getButton(AlertDialog.BUTTON_POSITIVE) | ||||
| positiveButton.setOnClickListener { | ||||
| val combinedText = activeInputFields | ||||
| .map { it.text.toString().trim() } | ||||
| .filter { it.isNotEmpty() } | ||||
| .joinToString("\n") | ||||
|
|
||||
| if (combinedText.isNotEmpty()) { | ||||
| val resultBundle = Bundle().apply { | ||||
| putString(RESULT_TEXT_KEY, combinedText) | ||||
| putInt(RESULT_TASK_ID_KEY, taskId) | ||||
| } | ||||
| parentFragmentManager.setFragmentResult(REQUEST_KEY, resultBundle) | ||||
| dialog.dismiss() | ||||
| } else { | ||||
| dialog.dismiss() | ||||
| } | ||||
| } | ||||
| } | ||||
|
|
||||
| return dialog | ||||
| } | ||||
|
|
||||
| private fun addNewRow(text: String) { | ||||
| val rowBinding = ItemAddChecklistBinding.inflate(layoutInflater) | ||||
|
|
||||
| // We disable automatic state saving for this view. | ||||
| // This prevents Android from confusing the multiple EditTexts (which all share the same ID) | ||||
| // and overwriting our manually restored text with the last view's text. | ||||
| rowBinding.titleEditText.isSaveEnabled = false | ||||
|
|
||||
| rowBinding.titleEditText.setText(text) | ||||
|
|
||||
| if (text.isNotEmpty()) { | ||||
| rowBinding.titleEditText.setSelection(text.length) | ||||
| } | ||||
|
|
||||
| val inputField = rowBinding.titleEditText as AppCompatEditText | ||||
| activeInputFields.add(inputField) | ||||
|
|
||||
| binding?.checklistHolder?.addView(rowBinding.root) | ||||
| } | ||||
|
|
||||
| override fun onSaveInstanceState(outState: Bundle) { | ||||
| super.onSaveInstanceState(outState) | ||||
| val currentTexts = ArrayList(activeInputFields.map { it.text.toString() }) | ||||
| outState.putStringArrayList(SAVED_STATE_TEXTS, currentTexts) | ||||
| } | ||||
|
|
||||
| override fun onDestroyView() { | ||||
| super.onDestroyView() | ||||
| binding = null | ||||
| } | ||||
| } | ||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,8 +14,7 @@ import org.fossify.commons.helpers.ensureBackgroundThread | |
| import org.fossify.notes.activities.SimpleActivity | ||
| import org.fossify.notes.adapters.TasksAdapter | ||
| import org.fossify.notes.databinding.FragmentChecklistBinding | ||
| import org.fossify.notes.dialogs.EditTaskDialog | ||
| import org.fossify.notes.dialogs.NewChecklistItemDialog | ||
| import org.fossify.notes.dialogs.ChecklistItemDialogFragment | ||
| import org.fossify.notes.extensions.config | ||
| import org.fossify.notes.extensions.updateWidgets | ||
| import org.fossify.notes.helpers.NOTE_ID | ||
|
|
@@ -43,6 +42,26 @@ class TasksFragment : NoteFragment(), TasksActionListener { | |
| return binding.root | ||
| } | ||
|
|
||
| override fun onViewCreated(view: View, savedInstanceState: Bundle?) { | ||
| super.onViewCreated(view, savedInstanceState) | ||
|
|
||
| // Listen for results from the ChecklistItemDialogFragment | ||
| childFragmentManager.setFragmentResultListener(ChecklistItemDialogFragment.REQUEST_KEY, | ||
| viewLifecycleOwner) | ||
| { _, bundle -> | ||
| val text = bundle.getString(ChecklistItemDialogFragment.RESULT_TEXT_KEY) ?: return@setFragmentResultListener | ||
| val taskId = bundle.getInt(ChecklistItemDialogFragment.RESULT_TASK_ID_KEY, -1) | ||
|
|
||
| if (taskId == -1) { | ||
| // ID is -1, so we are adding a NEW item | ||
| addNewChecklistItems(text) | ||
| } else { | ||
| // ID exists, so we are EDITING an existing item | ||
| updateExistingTask(taskId, text) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| override fun onResume() { | ||
| super.onResume() | ||
| loadNoteById(noteId) | ||
|
|
@@ -137,30 +156,30 @@ class TasksFragment : NoteFragment(), TasksActionListener { | |
| setupLockedViews(this.toCommonBinding(), note!!) | ||
| } | ||
| } | ||
|
|
||
| private fun showNewItemDialog() { | ||
| NewChecklistItemDialog(activity as SimpleActivity, noteId) { titles -> | ||
| var currentMaxId = tasks.maxByOrNull { item -> item.id }?.id ?: 0 | ||
| val newItems = ArrayList<Task>() | ||
|
|
||
| titles.forEach { title -> | ||
| title.split("\n").map { it.trim() }.filter { it.isNotBlank() }.forEach { row -> | ||
| newItems.add(Task(currentMaxId + 1, System.currentTimeMillis(), row, false)) | ||
| currentMaxId++ | ||
| } | ||
| } | ||
| // Pass -1 to indicate a NEW item | ||
| ChecklistItemDialogFragment.newInstance(taskId = -1, text = "") | ||
| .show(childFragmentManager, ChecklistItemDialogFragment.DIALOG_TAG) | ||
| } | ||
|
|
||
| if (config?.addNewChecklistItemsTop == true) { | ||
| tasks.addAll(0, newItems) | ||
| } else { | ||
| tasks.addAll(newItems) | ||
| } | ||
| private fun addNewChecklistItems(text: String) { | ||
| var currentMaxId = tasks.maxByOrNull { item -> item.id }?.id ?: 0 | ||
| val newItems = ArrayList<Task>() | ||
|
|
||
| saveNote() | ||
| setupAdapter() | ||
| text.split("\n").map { it.trim() }.filter { it.isNotBlank() }.forEach { row -> | ||
| newItems.add(Task(currentMaxId + 1, System.currentTimeMillis(), row, false)) | ||
| currentMaxId++ | ||
| } | ||
|
|
||
| if (config?.addNewChecklistItemsTop == true) { | ||
| tasks.addAll(0, newItems) | ||
| } else { | ||
| tasks.addAll(newItems) | ||
| } | ||
| } | ||
|
|
||
| saveNote() | ||
| setupAdapter() | ||
| } | ||
| private fun prepareTaskItems(): List<NoteItem> { | ||
| return if (config?.moveDoneChecklistItems == true) { | ||
| mutableListOf<NoteItem>().apply { | ||
|
|
@@ -272,12 +291,17 @@ class TasksFragment : NoteFragment(), TasksActionListener { | |
| fun getTasks() = Gson().toJson(tasks) | ||
|
|
||
| override fun editTask(task: Task, callback: () -> Unit) { | ||
| EditTaskDialog(activity as SimpleActivity, task.title) { title -> | ||
| val editedTask = task.copy(title = title) | ||
| val index = tasks.indexOf(task) | ||
| tasks[index] = editedTask | ||
| ChecklistItemDialogFragment.newInstance(taskId = task.id, text = task.title) | ||
| .show(childFragmentManager, ChecklistItemDialogFragment.DIALOG_TAG) | ||
| } | ||
|
|
||
| private fun updateExistingTask(taskId: Int, newTitle: String) { | ||
| val taskIndex = tasks.indexOfFirst { it.id == taskId } | ||
| if (taskIndex != -1) { | ||
| val task = tasks[taskIndex] | ||
| val editedTask = task.copy(title = newTitle) | ||
| tasks[taskIndex] = editedTask | ||
| saveAndReload() | ||
| callback() | ||
| } | ||
| } | ||
|
Comment on lines
+298
to
306
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Usually, this is correct, but we don't need to reference the general issue, as it gets erased by the release automation scripts (yet to be fixed) unless the reference points to this exact repository.