Skip to content
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

Remove completion from Navigator APIs #488

Merged
merged 2 commits into from
Apr 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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>) {
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
}
Expand All @@ -925,7 +922,6 @@ public class EpubNavigatorFragment internal constructor(
}
}

viewLifecycleOwner.lifecycleScope.launch { completion() }
return true
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand All @@ -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 =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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> =
Expand Down
Loading