Remove completion from Navigator APIs (readium#488)

Co-authored-by: qnga <32197639+qnga@users.noreply.github.com>
marshal1919 · Apr 16, 2024 · d335bde · d335bde
1 parent 27d64b1
commit d335bde
Show file tree

Hide file tree

Showing 11 changed files with 57 additions and 62 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,7 +4,14 @@ All notable changes to this project will be documented in this file. Take a look
 
 **Warning:** Features marked as *experimental* may change or be removed in a future release without notice. Use with caution.
 
-<!-- ## [Unreleased] -->
+## [Unreleased]
+
+### Deprecated
+
+#### Navigator
+
+* All the `completion` parameters of the `Navigator` APIs are removed.
+
 
 ## [3.0.0-alpha.2]
 

diff --git a/readium/navigator-media2/src/main/java/org/readium/navigator/media2/MediaNavigator.kt b/readium/navigator-media2/src/main/java/org/readium/navigator/media2/MediaNavigator.kt
@@ -371,28 +371,25 @@ public class MediaNavigator private constructor(
      * Compatibility
      */
 
-    private fun launchAndRun(runnable: suspend () -> Unit, callback: () -> Unit) =
-        coroutineScope.launch { runnable() }.invokeOnCompletion { callback() }
-
-    override fun go(locator: Locator, animated: Boolean, completion: () -> Unit): Boolean {
-        launchAndRun({ go(locator) }, completion)
+    override fun go(locator: Locator, animated: Boolean): Boolean {
+        coroutineScope.launch { go(locator) }
         return true
     }
 
-    override fun go(link: Link, animated: Boolean, completion: () -> Unit): Boolean {
-        launchAndRun({ go(link) }, completion)
+    override fun go(link: Link, animated: Boolean): Boolean {
+        coroutineScope.launch { go(link) }
         return true
     }
 
     @Suppress("UNUSED_PARAMETER")
-    public fun goForward(animated: Boolean, completion: () -> Unit): Boolean {
-        launchAndRun({ goForward() }, completion)
+    public fun goForward(animated: Boolean): Boolean {
+        coroutineScope.launch { goForward() }
         return true
     }
 
     @Suppress("UNUSED_PARAMETER")
-    public fun goBackward(animated: Boolean, completion: () -> Unit): Boolean {
-        launchAndRun({ goBackward() }, completion)
+    public fun goBackward(animated: Boolean): Boolean {
+        coroutineScope.launch { goBackward() }
         return true
     }
 

diff --git a/readium/navigator/src/main/java/org/readium/r2/navigator/Navigator.kt b/readium/navigator/src/main/java/org/readium/r2/navigator/Navigator.kt
@@ -41,12 +41,12 @@ public interface Navigator {
     /**
      * Moves to the position in the publication corresponding to the given [Locator].
      */
-    public fun go(locator: Locator, animated: Boolean = false, completion: () -> Unit = {}): Boolean
+    public fun go(locator: Locator, animated: Boolean = false): Boolean
 
     /**
      * Moves to the position in the publication targeted by the given link.
      */
-    public fun go(link: Link, animated: Boolean = false, completion: () -> Unit = {}): Boolean
+    public fun go(link: Link, animated: Boolean = false): Boolean
 
     public interface Listener {
 

diff --git a/readium/navigator/src/main/java/org/readium/r2/navigator/R2BasicWebView.kt b/readium/navigator/src/main/java/org/readium/r2/navigator/R2BasicWebView.kt
@@ -65,8 +65,8 @@ internal open class R2BasicWebView(context: Context, attrs: AttributeSet) : WebV
         fun onKey(event: KeyEvent): Boolean = false
         fun onDecorationActivated(id: DecorationId, group: String, rect: RectF, point: PointF): Boolean = false
         fun onProgressionChanged() {}
-        fun goForward(animated: Boolean = false, completion: () -> Unit = {}): Boolean = false
-        fun goBackward(animated: Boolean = false, completion: () -> Unit = {}): Boolean = false
+        fun goForward(animated: Boolean = false): Boolean = false
+        fun goBackward(animated: Boolean = false): Boolean = false
 
         /**
          * Returns the custom [ActionMode.Callback] to be used with the text selection menu.

diff --git a/readium/navigator/src/main/java/org/readium/r2/navigator/VisualNavigator.kt b/readium/navigator/src/main/java/org/readium/r2/navigator/VisualNavigator.kt
@@ -161,10 +161,10 @@ public interface OverflowableNavigator : VisualNavigator {
     /**
      * Moves to the next content portion (eg. page) in the reading progression direction.
      */
-    public fun goForward(animated: Boolean = false, completion: () -> Unit = {}): Boolean
+    public fun goForward(animated: Boolean = false): Boolean
 
     /**
      * Moves to the previous content portion (eg. page) in the reading progression direction.
      */
-    public fun goBackward(animated: Boolean = false, completion: () -> Unit = {}): Boolean
+    public fun goBackward(animated: Boolean = false): Boolean
 }
diff --git a/readium/navigator/src/main/java/org/readium/r2/navigator/epub/EpubNavigatorFragment.kt b/readium/navigator/src/main/java/org/readium/r2/navigator/epub/EpubNavigatorFragment.kt
@@ -591,7 +591,7 @@ public class EpubNavigatorFragment internal constructor(
     }
 
     @OptIn(DelicateReadiumApi::class)
-    override fun go(locator: Locator, animated: Boolean, completion: () -> Unit): Boolean {
+    override fun go(locator: Locator, animated: Boolean): Boolean {
         @Suppress("NAME_SHADOWING")
         val locator = publication.normalizeLocator(locator)
 
@@ -636,9 +636,9 @@ public class EpubNavigatorFragment internal constructor(
         return true
     }
 
-    override fun go(link: Link, animated: Boolean, completion: () -> Unit): Boolean {
+    override fun go(link: Link, animated: Boolean): Boolean {
         val locator = publication.locatorFromLink(link) ?: return false
-        return go(locator, animated, completion)
+        return go(locator, animated)
     }
 
     private fun run(commands: List<RunScriptCommand>) {
@@ -846,9 +846,9 @@ public class EpubNavigatorFragment internal constructor(
                 ?.let { publication.get(it) }
     }
 
-    override fun goForward(animated: Boolean, completion: () -> Unit): Boolean {
+    override fun goForward(animated: Boolean): Boolean {
         if (publication.metadata.presentation.layout == EpubLayout.FIXED) {
-            return goToNextResource(jump = false, animated = animated, completion)
+            return goToNextResource(jump = false, animated = animated)
         }
 
         val webView = currentReflowablePageFragment?.webView ?: return false
@@ -860,13 +860,12 @@ public class EpubNavigatorFragment internal constructor(
             ReadingProgression.RTL ->
                 webView.scrollLeft(animated)
         }
-        lifecycleScope.launch { completion() }
         return true
     }
 
-    override fun goBackward(animated: Boolean, completion: () -> Unit): Boolean {
+    override fun goBackward(animated: Boolean): Boolean {
         if (publication.metadata.presentation.layout == EpubLayout.FIXED) {
-            return goToPreviousResource(jump = false, animated = animated, completion)
+            return goToPreviousResource(jump = false, animated = animated)
         }
 
         val webView = currentReflowablePageFragment?.webView ?: return false
@@ -878,11 +877,10 @@ public class EpubNavigatorFragment internal constructor(
             ReadingProgression.RTL ->
                 webView.scrollRight(animated)
         }
-        lifecycleScope.launch { completion() }
         return true
     }
 
-    private fun goToNextResource(jump: Boolean, animated: Boolean, completion: () -> Unit = {}): Boolean {
+    private fun goToNextResource(jump: Boolean, animated: Boolean): Boolean {
         val adapter = resourcePager.adapter ?: return false
         if (resourcePager.currentItem >= adapter.count - 1) {
             return false
@@ -902,11 +900,10 @@ public class EpubNavigatorFragment internal constructor(
             }
         }
 
-        viewLifecycleOwner.lifecycleScope.launch { completion() }
         return true
     }
 
-    private fun goToPreviousResource(jump: Boolean, animated: Boolean, completion: () -> Unit = {}): Boolean {
+    private fun goToPreviousResource(jump: Boolean, animated: Boolean): Boolean {
         if (resourcePager.currentItem <= 0) {
             return false
         }
@@ -925,7 +922,6 @@ public class EpubNavigatorFragment internal constructor(
             }
         }
 
-        viewLifecycleOwner.lifecycleScope.launch { completion() }
         return true
     }
 

diff --git a/readium/navigator/src/main/java/org/readium/r2/navigator/image/ImageNavigatorFragment.kt b/readium/navigator/src/main/java/org/readium/r2/navigator/image/ImageNavigatorFragment.kt
@@ -183,7 +183,7 @@ public class ImageNavigatorFragment private constructor(
         _currentLocator.value = locator
     }
 
-    override fun go(locator: Locator, animated: Boolean, completion: () -> Unit): Boolean {
+    override fun go(locator: Locator, animated: Boolean): Boolean {
         @Suppress("NAME_SHADOWING")
         val locator = publication.normalizeLocator(locator)
 
@@ -197,12 +197,12 @@ public class ImageNavigatorFragment private constructor(
         return true
     }
 
-    override fun go(link: Link, animated: Boolean, completion: () -> Unit): Boolean {
+    override fun go(link: Link, animated: Boolean): Boolean {
         val locator = publication.locatorFromLink(link) ?: return false
-        return go(locator, animated, completion)
+        return go(locator, animated)
     }
 
-    override fun goForward(animated: Boolean, completion: () -> Unit): Boolean {
+    override fun goForward(animated: Boolean): Boolean {
         val current = resourcePager.currentItem
         if (requireActivity().layoutDirectionIsRTL()) {
             // The view has RTL layout
@@ -216,7 +216,7 @@ public class ImageNavigatorFragment private constructor(
         return current != resourcePager.currentItem
     }
 
-    override fun goBackward(animated: Boolean, completion: () -> Unit): Boolean {
+    override fun goBackward(animated: Boolean): Boolean {
         val current = resourcePager.currentItem
         if (requireActivity().layoutDirectionIsRTL()) {
             // The view has RTL layout

diff --git a/readium/navigator/src/main/java/org/readium/r2/navigator/media/MediaSessionNavigator.kt b/readium/navigator/src/main/java/org/readium/r2/navigator/media/MediaSessionNavigator.kt
@@ -188,7 +188,7 @@ public class MediaSessionNavigator(
     }
 
     @OptIn(DelicateReadiumApi::class)
-    override fun go(locator: Locator, animated: Boolean, completion: () -> Unit): Boolean {
+    override fun go(locator: Locator, animated: Boolean): Boolean {
         if (!isActive) return false
 
         @Suppress("NAME_SHADOWING")
@@ -202,30 +202,27 @@ public class MediaSessionNavigator(
                 putParcelable("locator", locator)
             }
         )
-        completion()
         return true
     }
 
-    override fun go(link: Link, animated: Boolean, completion: () -> Unit): Boolean {
+    override fun go(link: Link, animated: Boolean): Boolean {
         val locator = publication.locatorFromLink(link) ?: return false
-        return go(locator, animated, completion)
+        return go(locator, animated)
     }
 
     @Suppress("UNUSED_PARAMETER")
-    public fun goForward(animated: Boolean = true, completion: () -> Unit = {}): Boolean {
+    public fun goForward(animated: Boolean = true): Boolean {
         if (!isActive) return false
 
         seekRelative(skipForwardInterval)
-        completion()
         return true
     }
 
     @Suppress("UNUSED_PARAMETER")
-    public fun goBackward(animated: Boolean = true, completion: () -> Unit = {}): Boolean {
+    public fun goBackward(animated: Boolean = true): Boolean {
         if (!isActive) return false
 
         seekRelative(-skipBackwardInterval)
-        completion()
         return true
     }
 

diff --git a/readium/navigator/src/main/java/org/readium/r2/navigator/pdf/PdfNavigatorFragment.kt b/readium/navigator/src/main/java/org/readium/r2/navigator/pdf/PdfNavigatorFragment.kt
@@ -219,32 +219,30 @@ public class PdfNavigatorFragment<S : Configurable.Settings, P : Configurable.Pr
 
     override val currentLocator: StateFlow<Locator> get() = viewModel.currentLocator
 
-    override fun go(locator: Locator, animated: Boolean, completion: () -> Unit): Boolean {
+    override fun go(locator: Locator, animated: Boolean): Boolean {
         @Suppress("NAME_SHADOWING")
         val locator = publication.normalizeLocator(locator)
         listener?.onJumpToLocator(locator)
-        return goToPageIndex(locator.locations.pageIndex, animated, completion)
+        return goToPageIndex(locator.locations.pageIndex, animated)
     }
 
-    override fun go(link: Link, animated: Boolean, completion: () -> Unit): Boolean {
+    override fun go(link: Link, animated: Boolean): Boolean {
         val locator = publication.locatorFromLink(link) ?: return false
-        return go(locator, animated, completion)
+        return go(locator, animated)
     }
 
-    override fun goForward(animated: Boolean, completion: () -> Unit): Boolean {
+    override fun goForward(animated: Boolean): Boolean {
         val pageIndex = currentLocator.value.locations.pageIndex + 1
-        return goToPageIndex(pageIndex, animated, completion)
+        return goToPageIndex(pageIndex, animated)
     }
 
-    override fun goBackward(animated: Boolean, completion: () -> Unit): Boolean {
+    override fun goBackward(animated: Boolean): Boolean {
         val pageIndex = currentLocator.value.locations.pageIndex - 1
-        return goToPageIndex(pageIndex, animated, completion)
+        return goToPageIndex(pageIndex, animated)
     }
 
-    private fun goToPageIndex(pageIndex: Int, animated: Boolean, completion: () -> Unit): Boolean {
-        val success = documentFragment.goToPageIndex(pageIndex, animated = animated)
-        if (success) { completion() }
-        return success
+    private fun goToPageIndex(pageIndex: Int, animated: Boolean): Boolean {
+        return documentFragment.goToPageIndex(pageIndex, animated = animated)
     }
 
     // VisualNavigator

diff --git a/.../navigators/media/audio/src/main/java/org/readium/navigator/media/audio/AudioNavigator.kt b/.../navigators/media/audio/src/main/java/org/readium/navigator/media/audio/AudioNavigator.kt
@@ -151,7 +151,7 @@ public class AudioNavigator<S : Configurable.Settings, P : Configurable.Preferen
     override fun asMedia3Player(): Player =
         audioEngine.asPlayer()
 
-    override fun go(locator: Locator, animated: Boolean, completion: () -> Unit): Boolean {
+    override fun go(locator: Locator, animated: Boolean): Boolean {
         @Suppress("NAME_SHADOWING")
         val locator = publication.normalizeLocator(locator)
         val itemIndex = readingOrder.items.indexOfFirst { it.href == locator.href }
@@ -163,9 +163,9 @@ public class AudioNavigator<S : Configurable.Settings, P : Configurable.Preferen
         return true
     }
 
-    override fun go(link: Link, animated: Boolean, completion: () -> Unit): Boolean {
+    override fun go(link: Link, animated: Boolean): Boolean {
         val locator = publication.locatorFromLink(link) ?: return false
-        return go(locator, animated, completion)
+        return go(locator, animated)
     }
 
     private fun AudioEngine.State.toState(): MediaNavigator.State =

diff --git a/readium/navigators/media/tts/src/main/java/org/readium/navigator/media/tts/TtsNavigator.kt b/readium/navigators/media/tts/src/main/java/org/readium/navigator/media/tts/TtsNavigator.kt
@@ -152,14 +152,14 @@ public class TtsNavigator<S : TtsEngine.Settings, P : TtsEngine.Preferences<P>,
     override val currentLocator: StateFlow<Locator> =
         location.mapStateIn(coroutineScope) { it.tokenLocator ?: it.utteranceLocator }
 
-    override fun go(locator: Locator, animated: Boolean, completion: () -> Unit): Boolean {
+    override fun go(locator: Locator, animated: Boolean): Boolean {
         player.go(publication.normalizeLocator(locator))
         return true
     }
 
-    override fun go(link: Link, animated: Boolean, completion: () -> Unit): Boolean {
+    override fun go(link: Link, animated: Boolean): Boolean {
         val locator = publication.locatorFromLink(link) ?: return false
-        return go(locator, animated, completion)
+        return go(locator, animated)
     }
 
     override val settings: StateFlow<S> =