Remove completion from Navigator APIs (#488)

mickael-menu · qnga · web-flow · commit d335bde3ea22 · 2024-04-16T09:12:37.000+02:00
Co-authored-by: qnga &lt;32197639+qnga@users.noreply.github.com&gt;
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/readium/navigators/media/audio/src/main/java/org/readium/navigator/media/audio/AudioNavigator.kt b/readium/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> =

Original file line number	Diff line number	Diff line change
`@@ -591,7 +591,7 @@ public class EpubNavigatorFragment internal constructor(`
`591`	`591`	`}`
`592`	`592`
`593`	`593`	`@OptIn(DelicateReadiumApi::class)`
`594`		`- override fun go(locator: Locator, animated: Boolean, completion: () -> Unit): Boolean {`
	`594`	`+ override fun go(locator: Locator, animated: Boolean): Boolean {`
`595`	`595`	`@Suppress("NAME_SHADOWING")`
`596`	`596`	`val locator = publication.normalizeLocator(locator)`
`597`	`597`
`@@ -636,9 +636,9 @@ public class EpubNavigatorFragment internal constructor(`
`636`	`636`	`return true`
`637`	`637`	`}`
`638`	`638`
`639`		`- override fun go(link: Link, animated: Boolean, completion: () -> Unit): Boolean {`
	`639`	`+ override fun go(link: Link, animated: Boolean): Boolean {`
`640`	`640`	`val locator = publication.locatorFromLink(link) ?: return false`
`641`		`- return go(locator, animated, completion)`
	`641`	`+ return go(locator, animated)`
`642`	`642`	`}`
`643`	`643`
`644`	`644`	`private fun run(commands: List<RunScriptCommand>) {`
`@@ -846,9 +846,9 @@ public class EpubNavigatorFragment internal constructor(`
`846`	`846`	`?.let { publication.get(it) }`
`847`	`847`	`}`
`848`	`848`
`849`		`- override fun goForward(animated: Boolean, completion: () -> Unit): Boolean {`
	`849`	`+ override fun goForward(animated: Boolean): Boolean {`
`850`	`850`	`if (publication.metadata.presentation.layout == EpubLayout.FIXED) {`
`851`		`- return goToNextResource(jump = false, animated = animated, completion)`
	`851`	`+ return goToNextResource(jump = false, animated = animated)`
`852`	`852`	`}`
`853`	`853`
`854`	`854`	`val webView = currentReflowablePageFragment?.webView ?: return false`
`@@ -860,13 +860,12 @@ public class EpubNavigatorFragment internal constructor(`
`860`	`860`	`ReadingProgression.RTL ->`
`861`	`861`	`webView.scrollLeft(animated)`
`862`	`862`	`}`
`863`		`- lifecycleScope.launch { completion() }`
`864`	`863`	`return true`
`865`	`864`	`}`
`866`	`865`
`867`		`- override fun goBackward(animated: Boolean, completion: () -> Unit): Boolean {`
	`866`	`+ override fun goBackward(animated: Boolean): Boolean {`
`868`	`867`	`if (publication.metadata.presentation.layout == EpubLayout.FIXED) {`
`869`		`- return goToPreviousResource(jump = false, animated = animated, completion)`
	`868`	`+ return goToPreviousResource(jump = false, animated = animated)`
`870`	`869`	`}`
`871`	`870`
`872`	`871`	`val webView = currentReflowablePageFragment?.webView ?: return false`
`@@ -878,11 +877,10 @@ public class EpubNavigatorFragment internal constructor(`
`878`	`877`	`ReadingProgression.RTL ->`
`879`	`878`	`webView.scrollRight(animated)`
`880`	`879`	`}`
`881`		`- lifecycleScope.launch { completion() }`
`882`	`880`	`return true`
`883`	`881`	`}`
`884`	`882`
`885`		`- private fun goToNextResource(jump: Boolean, animated: Boolean, completion: () -> Unit = {}): Boolean {`
	`883`	`+ private fun goToNextResource(jump: Boolean, animated: Boolean): Boolean {`
`886`	`884`	`val adapter = resourcePager.adapter ?: return false`
`887`	`885`	`if (resourcePager.currentItem >= adapter.count - 1) {`
`888`	`886`	`return false`
`@@ -902,11 +900,10 @@ public class EpubNavigatorFragment internal constructor(`
`902`	`900`	`}`
`903`	`901`	`}`
`904`	`902`
`905`		`- viewLifecycleOwner.lifecycleScope.launch { completion() }`
`906`	`903`	`return true`
`907`	`904`	`}`
`908`	`905`
`909`		`- private fun goToPreviousResource(jump: Boolean, animated: Boolean, completion: () -> Unit = {}): Boolean {`
	`906`	`+ private fun goToPreviousResource(jump: Boolean, animated: Boolean): Boolean {`
`910`	`907`	`if (resourcePager.currentItem <= 0) {`
`911`	`908`	`return false`
`912`	`909`	`}`
`@@ -925,7 +922,6 @@ public class EpubNavigatorFragment internal constructor(`
`925`	`922`	`}`
`926`	`923`	`}`
`927`	`924`
`928`		`- viewLifecycleOwner.lifecycleScope.launch { completion() }`
`929`	`925`	`return true`
`930`	`926`	`}`
`931`	`927`
Original file line number	Diff line number	Diff line change
`@@ -188,7 +188,7 @@ public class MediaSessionNavigator(`
`188`	`188`	`}`
`189`	`189`
`190`	`190`	`@OptIn(DelicateReadiumApi::class)`
`191`		`- override fun go(locator: Locator, animated: Boolean, completion: () -> Unit): Boolean {`
	`191`	`+ override fun go(locator: Locator, animated: Boolean): Boolean {`
`192`	`192`	`if (!isActive) return false`
`193`	`193`
`194`	`194`	`@Suppress("NAME_SHADOWING")`
`@@ -202,30 +202,27 @@ public class MediaSessionNavigator(`
`202`	`202`	`putParcelable("locator", locator)`
`203`	`203`	`}`
`204`	`204`	`)`
`205`		`- completion()`
`206`	`205`	`return true`
`207`	`206`	`}`
`208`	`207`
`209`		`- override fun go(link: Link, animated: Boolean, completion: () -> Unit): Boolean {`
	`208`	`+ override fun go(link: Link, animated: Boolean): Boolean {`
`210`	`209`	`val locator = publication.locatorFromLink(link) ?: return false`
`211`		`- return go(locator, animated, completion)`
	`210`	`+ return go(locator, animated)`
`212`	`211`	`}`
`213`	`212`
`214`	`213`	`@Suppress("UNUSED_PARAMETER")`
`215`		`- public fun goForward(animated: Boolean = true, completion: () -> Unit = {}): Boolean {`
	`214`	`+ public fun goForward(animated: Boolean = true): Boolean {`
`216`	`215`	`if (!isActive) return false`
`217`	`216`
`218`	`217`	`seekRelative(skipForwardInterval)`
`219`		`- completion()`
`220`	`218`	`return true`
`221`	`219`	`}`
`222`	`220`
`223`	`221`	`@Suppress("UNUSED_PARAMETER")`
`224`		`- public fun goBackward(animated: Boolean = true, completion: () -> Unit = {}): Boolean {`
	`222`	`+ public fun goBackward(animated: Boolean = true): Boolean {`
`225`	`223`	`if (!isActive) return false`
`226`	`224`
`227`	`225`	`seekRelative(-skipBackwardInterval)`
`228`		`- completion()`
`229`	`226`	`return true`
`230`	`227`	`}`
`231`	`228`