Merge pull request #892 from Automattic/update/automotive-blank-skips

Stop the Automotive set skip settings accepting a blank value
Automattic · Apr 19, 2023 · 645891c · 645891c
2 parents 9b627fc + 78dbc4a
commit 645891c
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 30 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,8 @@
 *   Bug Fixes:
     *   Fixed accessibility content desctiption for episode list
          ([#890](https://github.com/Automattic/pocket-casts-android/issues/890)).
+    *   Improved the validation of the Automotive skip forward and back time settings
+         ([#890](https://github.com/Automattic/pocket-casts-android/pull/892)).
 
 7.37
 -----

diff --git a/...tive/src/main/java/au/com/shiftyjelly/pocketcasts/AutomotiveSettingsPreferenceFragment.kt b/...tive/src/main/java/au/com/shiftyjelly/pocketcasts/AutomotiveSettingsPreferenceFragment.kt
@@ -1,6 +1,5 @@
 package au.com.shiftyjelly.pocketcasts
 
-import android.content.SharedPreferences
 import android.os.Bundle
 import android.view.View
 import androidx.lifecycle.LiveData
@@ -26,7 +25,7 @@ import javax.inject.Inject
 import au.com.shiftyjelly.pocketcasts.localization.R as LR
 
 @AndroidEntryPoint
-class AutomotiveSettingsPreferenceFragment : PreferenceFragmentCompat(), SharedPreferences.OnSharedPreferenceChangeListener, Observer<RefreshState> {
+class AutomotiveSettingsPreferenceFragment : PreferenceFragmentCompat(), Observer<RefreshState> {
 
     @Inject lateinit var settings: Settings
     @Inject lateinit var podcastManager: PodcastManager
@@ -81,6 +80,30 @@ class AutomotiveSettingsPreferenceFragment : PreferenceFragmentCompat(), SharedP
                 }
                 .toLiveData()
         refreshObservable?.observe(viewLifecycleOwner, this)
+
+        preferenceSkipForward?.setOnPreferenceChangeListener { _, newValue ->
+            val value = newValue.toString().toIntOrNull() ?: 0
+            if (value > 0) {
+                settings.setSkipForwardInSec(value)
+                settings.setSkipForwardNeedsSync(true)
+                changeSkipTitles()
+                true
+            } else {
+                false
+            }
+        }
+
+        preferenceSkipBackward?.setOnPreferenceChangeListener { _, newValue ->
+            val value = newValue.toString().toIntOrNull() ?: 0
+            if (value > 0) {
+                settings.setSkipBackwardInSec(value)
+                settings.setSkipBackNeedsSync(true)
+                changeSkipTitles()
+                true
+            } else {
+                false
+            }
+        }
     }
 
     override fun onDestroyView() {
@@ -108,32 +131,10 @@ class AutomotiveSettingsPreferenceFragment : PreferenceFragmentCompat(), SharedP
         preferenceRefreshNow?.summary = status
     }
 
-    override fun onResume() {
-        super.onResume()
-        preferenceScreen.sharedPreferences?.registerOnSharedPreferenceChangeListener(this)
-    }
-
-    override fun onPause() {
-        super.onPause()
-        preferenceScreen.sharedPreferences?.unregisterOnSharedPreferenceChangeListener(this)
-    }
-
     private fun changeSkipTitles() {
         val skipForwardSummary = resources.getStringPluralSeconds(settings.getSkipForwardInSecs())
         preferenceSkipForward?.summary = skipForwardSummary
         val skipBackwardSummary = resources.getStringPluralSeconds(settings.getSkipBackwardInSecs())
         preferenceSkipBackward?.summary = skipBackwardSummary
     }
-
-    override fun onSharedPreferenceChanged(sharedPreferences: SharedPreferences, key: String) {
-        if (Settings.PREFERENCE_SKIP_FORWARD == key || Settings.PREFERENCE_SKIP_BACKWARD == key) {
-            changeSkipTitles()
-            settings.updateSkipValues()
-
-            when (key) {
-                Settings.PREFERENCE_SKIP_FORWARD -> settings.setSkipForwardNeedsSync(true)
-                Settings.PREFERENCE_SKIP_BACKWARD -> settings.setSkipBackNeedsSync(true)
-            }
-        }
-    }
 }
diff --git a/...services/preferences/src/main/java/au/com/shiftyjelly/pocketcasts/preferences/Settings.kt b/...services/preferences/src/main/java/au/com/shiftyjelly/pocketcasts/preferences/Settings.kt
@@ -317,7 +317,6 @@ interface Settings {
     fun getSkipForwardInMs(): Long
     fun getSkipBackwardInSecs(): Int
     fun getSkipBackwardInMs(): Long
-    fun updateSkipValues()
 
     fun getLastScreenOpened(): String?
     fun setLastScreenOpened(screenId: String)

diff --git a/...ices/preferences/src/main/java/au/com/shiftyjelly/pocketcasts/preferences/SettingsImpl.kt b/...ices/preferences/src/main/java/au/com/shiftyjelly/pocketcasts/preferences/SettingsImpl.kt
@@ -208,11 +208,6 @@ class SettingsImpl @Inject constructor(
         setBoolean(Settings.PREFERENCE_SKIP_BACK_NEEDS_SYNC, value)
     }
 
-    override fun updateSkipValues() {
-        skipBackwardInSecsObservable.accept(getSkipBackwardInSecs())
-        skipForwardInSecsObservable.accept(getSkipForwardInSecs())
-    }
-
     override fun getMarketingOptIn(): Boolean {
         return getBoolean(Settings.PREFERENCE_MARKETING_OPT_IN, false)
     }