Fixes some filter creation bugs (#447)

- Fixes a case where when changing the modifier, the filter wouldn't be persisted - Fixes a case where when changing the modifier for a date filter, you wouldn't be able to change the date value - Fixes a bug when changing a modifier, but still selecting the same one would prevent changing the modifier again
damontecres · Oct 31, 2024 · 471e0ad · 471e0ad
1 parent 1d5075f
commit 471e0ad
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 16 deletions.
diff --git a/app/src/main/java/com/github/damontecres/stashapp/filter/picker/DatePickerFragment.kt b/app/src/main/java/com/github/damontecres/stashapp/filter/picker/DatePickerFragment.kt
@@ -54,11 +54,12 @@ class DatePickerFragment(
         createActionList(actions)
     }
 
-    override fun createActionList(actions: MutableList<GuidedAction>) {
+    override fun createActionList(actions: MutableList<GuidedAction>): List<GuidedAction> {
+        Log.v(TAG, "createActionList: actions.size=${actions.size}")
         val dateLong =
             if (value1 != null) {
                 try {
-                    format.parse(value1)?.time ?: Date().time
+                    format.parse(value1!!)?.time ?: Date().time
                 } catch (ex: ParseException) {
                     Log.w(TAG, "Parse error ($value1)", ex)
                     Date().time
@@ -117,6 +118,8 @@ class DatePickerFragment(
         }
 
         addStandardActions(actions, filterOption)
+
+        return actions
     }
 
     override fun parseAction(action: GuidedAction?): String? {
@@ -137,7 +140,7 @@ class DatePickerFragment(
                 value2 = Optional.presentIfNotNull(value2),
                 modifier = modifier,
             )
-        } else if (modifier == CriterionModifier.IS_NULL || modifier == CriterionModifier.NOT_NULL) {
+        } else if (modifier.isNullModifier()) {
             DateCriterionInput(value = "", modifier = modifier)
         } else {
             null

diff --git a/app/src/main/java/com/github/damontecres/stashapp/filter/picker/TwoValuePicker.kt b/app/src/main/java/com/github/damontecres/stashapp/filter/picker/TwoValuePicker.kt
@@ -1,6 +1,7 @@
 package com.github.damontecres.stashapp.filter.picker
 
 import android.os.Bundle
+import android.util.Log
 import android.widget.Toast
 import androidx.core.content.ContextCompat
 import androidx.leanback.widget.GuidanceStylist
@@ -43,7 +44,8 @@ abstract class TwoValuePicker<T, CriterionInput : Any>(
         )
     }
 
-    protected open fun createActionList(actions: MutableList<GuidedAction>) {
+    protected open fun createActionList(actions: MutableList<GuidedAction> = mutableListOf()): List<GuidedAction> {
+        Log.v(TAG, "createActionList: actions.size=${actions.size}")
         val modifierOptions = this.modifierOptions.map(::modifierAction)
         actions.add(
             GuidedAction.Builder(requireContext())
@@ -86,9 +88,12 @@ abstract class TwoValuePicker<T, CriterionInput : Any>(
         }
 
         addStandardActions(actions, filterOption)
+
+        return actions
     }
 
     override fun onGuidedActionEditedAndProceed(action: GuidedAction): Long {
+        Log.v(TAG, "onGuidedActionEditedAndProceed: ${action.id}")
         if (action.id == VALUE_1 || action.id == VALUE_2) {
             value1 = parseAction(if (action.id == VALUE_1) action else findActionById(VALUE_1))
             value2 = parseAction(if (action.id == VALUE_2) action else findActionById(VALUE_2))
@@ -121,27 +126,30 @@ abstract class TwoValuePicker<T, CriterionInput : Any>(
     }
 
     override fun onSubGuidedActionClicked(action: GuidedAction): Boolean {
+        Log.v(TAG, "onSubGuidedActionClicked: ${action.id}")
         if (action.id >= MODIFIER_OFFSET) {
-            modifier = CriterionModifier.entries[(action.id - MODIFIER_OFFSET).toInt()]
-            collapseSubActions()
-            val actions = mutableListOf<GuidedAction>()
-            createActionList(actions)
-            this.actions = actions
-            if (modifier.hasTwoValues() && (value1 == null || value2 == null)) {
-                enableFinish(false)
-            } else {
-                enableFinish(true)
+            val newModifier = CriterionModifier.entries[(action.id - MODIFIER_OFFSET).toInt()]
+            if (modifier != newModifier) {
+                modifier = newModifier
+                // Since the actions are going to be modified before this function returns to collapse the sub actions,
+                // Manually collapse the sub actions, not doing this results in a weird UI reset
+                collapseSubActions()
+                setActionsDiffCallback(null)
+                this.actions = createActionList()
+                if (value1 == null || modifier.hasTwoValues() && value2 == null) {
+                    enableFinish(false)
+                } else {
+                    enableFinish(true)
+                }
             }
         }
         return true
     }
 
     override fun onGuidedActionClicked(action: GuidedAction) {
+        Log.v(TAG, "onGuidedActionClicked: ${action.id}")
         if (action.id == GuidedAction.ACTION_ID_FINISH) {
-            val value1 = parseAction(findActionById(VALUE_1))
-            val value2 = parseAction(findActionById(VALUE_2))
             val newValue = createCriterionInput(value1, value2, modifier)
-
             viewModel.updateFilter(filterOption, newValue)
             parentFragmentManager.popBackStack()
         } else {
@@ -176,6 +184,8 @@ abstract class TwoValuePicker<T, CriterionInput : Any>(
     }
 
     companion object {
+        private const val TAG = "TwoValuePicker"
+
         const val VALUE_1 = 1L
         const val VALUE_2 = 2L
         const val MODIFIER = 3L