mirror of
https://github.com/chmouel/liseur.git
synced 2026-10-19 09:13:36 +00:00
fix: turn every page the same way
Turning a page while the reading menu was open used a different animation from turning it with the menu closed: the page slid across instead of lifting, took longer, and the page number at the bottom only caught up once the slide had finished. The lift needs a photograph of the page to animate it, and it cannot take one while the menu is covering the page, so it handed the turn to a slide instead. It now goes straight to the next page, which is where the lift ends up anyway. The app also took no notice of the system's "Remove animations" accessibility setting. It now reads it, so a reader who has turned animations off for the whole device gets plain, instant page turns without having to find the matching setting inside the app as well. Changing it takes effect in an already open book. Dragging a page across with a finger still works, since that follows the hand rather than playing something at the reader. Closes #201
This commit is contained in:
parent
2e888bf249
commit
8e47d7819a
8 changed files with 339 additions and 21 deletions
|
|
@ -295,6 +295,28 @@ enum class PageTurnStyle(val id: String) {
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The style a turn actually uses, once the screen and the system have
|
||||
* had their say.
|
||||
*
|
||||
* Two facts overrule the reader's choice, and they are not the same
|
||||
* fact even though they give the same answer. [eInk] is what the panel
|
||||
* can draw: a photograph moved across electronic paper arrives as a
|
||||
* trail of half-erased pages. [motionRemoved] is the reader telling
|
||||
* Android they do not want animations, which Liseur should not make
|
||||
* them say twice.
|
||||
*
|
||||
* They are kept apart because things downstream ask about one and not
|
||||
* the other — the curl a drag draws is refused on e-paper and kept for
|
||||
* a reader who merely turned animations off, since a page following a
|
||||
* thumb is that thumb moving and not the app playing something at them.
|
||||
*/
|
||||
fun pageTurnStyleOnScreen(
|
||||
chosen: PageTurnStyle,
|
||||
eInk: Boolean,
|
||||
motionRemoved: Boolean,
|
||||
): PageTurnStyle = if (eInk || motionRemoved) PageTurnStyle.NONE else chosen
|
||||
|
||||
/**
|
||||
* Which notch the auto-scroll slider sits on, as it is stored.
|
||||
*
|
||||
|
|
|
|||
|
|
@ -125,6 +125,7 @@ import com.chmouel.liseur.reader.footnotes.FootnoteLayout
|
|||
import com.chmouel.liseur.data.settings.FooterMode
|
||||
import com.chmouel.liseur.data.settings.ColumnMode
|
||||
import com.chmouel.liseur.data.settings.PageTurnStyle
|
||||
import com.chmouel.liseur.data.settings.pageTurnStyleOnScreen
|
||||
import com.chmouel.liseur.reader.progress.ResourceAnchor
|
||||
import com.chmouel.liseur.data.settings.ReadingFont
|
||||
import com.chmouel.liseur.data.settings.ReaderPrefs
|
||||
|
|
@ -177,6 +178,7 @@ import com.chmouel.liseur.reader.progress.RestorePoint
|
|||
import com.chmouel.liseur.reader.progress.ScrollProgression
|
||||
import com.chmouel.liseur.reader.search.SearchScreen
|
||||
import com.chmouel.liseur.ui.LocalEInk
|
||||
import com.chmouel.liseur.ui.rememberMotionRemoved
|
||||
import com.chmouel.liseur.ui.eink.EInkDisplay
|
||||
import com.chmouel.liseur.ui.BusyIndicator
|
||||
import com.chmouel.liseur.ui.contentWidthCap
|
||||
|
|
@ -804,16 +806,31 @@ fun ReaderScreen(
|
|||
val pageTurnEffect = remember { PageTurnEffectState(effectScope) }
|
||||
val viewedImageNow by rememberUpdatedState(viewedImage)
|
||||
val openingImageNow by rememberUpdatedState(openingImage)
|
||||
// The sliding page is a snapshot dragged across the screen, which is
|
||||
// the one thing electronic paper cannot draw: it arrives as a trail
|
||||
// of half-erased pages. The instant jump is what e-paper wants
|
||||
// anyway, and it overrules whatever is set here rather than being
|
||||
// one more thing to set. Taps and drags read the style from here, so
|
||||
// the two ways of turning a page cannot disagree about which drags
|
||||
// are Readium's; the curl a drag draws under the other styles is
|
||||
// refused on e-paper separately, since NONE cannot say why it is set.
|
||||
// Two things overrule the reader's choice here, and only here, so
|
||||
// that every motion a turn can make reads one answer: the tap, the
|
||||
// volume key, the drag claim, the glide a scrolled book makes and
|
||||
// the endpaper all ask this lambda. Taps and drags reading the same
|
||||
// style is what stops the two ways of turning a page disagreeing
|
||||
// about which drags are Readium's.
|
||||
//
|
||||
// Electronic paper is the first: a snapshot dragged across such a
|
||||
// screen arrives as a trail of half-erased pages, so the instant
|
||||
// jump is what the panel wants anyway. The second is a reader who
|
||||
// turned animations off in Android, which Liseur should not make
|
||||
// them say a second time in its own settings (#201).
|
||||
//
|
||||
// The curl a drag draws is refused on e-paper separately, since NONE
|
||||
// cannot say why it is set, and the two reasons want different
|
||||
// answers: a page following a thumb is the thumb moving, which a
|
||||
// reader who removed animations never asked to stop.
|
||||
val motionRemoved = rememberMotionRemoved()
|
||||
val motionRemovedNow by rememberUpdatedState(motionRemoved)
|
||||
val turnStyle: () -> PageTurnStyle = {
|
||||
if (eInkNow) PageTurnStyle.NONE else prefsFlow.value.pageTurnStyle
|
||||
pageTurnStyleOnScreen(
|
||||
chosen = prefsFlow.value.pageTurnStyle,
|
||||
eInk = eInkNow,
|
||||
motionRemoved = motionRemovedNow,
|
||||
)
|
||||
}
|
||||
val pageTurner = remember {
|
||||
PageTurner(
|
||||
|
|
|
|||
|
|
@ -86,10 +86,9 @@ fun PageTurnOverlay(state: PageTurnEffectState, modifier: Modifier = Modifier) {
|
|||
/**
|
||||
* Performs page turns for taps and volume keys. Which of the three
|
||||
* motions a turn uses is [style]'s answer: the lifted snapshot, the
|
||||
* navigator's own slide, or no motion at all. The lifted one still
|
||||
* falls back to the plain slide when the snapshot cannot be taken —
|
||||
* the chrome is up, a turn is already running, the view has no size
|
||||
* yet.
|
||||
* navigator's own slide, or no motion at all. The lifted one falls back
|
||||
* to the instant jump when the snapshot cannot be taken — the chrome is
|
||||
* up, a turn is already running, the view has no size yet.
|
||||
*
|
||||
* A scrolled book is a different movement altogether, and [isScrolling]
|
||||
* says so: see [scrollScreenful].
|
||||
|
|
@ -376,9 +375,16 @@ class PageTurner(
|
|||
if (win == null || isEffectSuppressed() || effect.isRunning ||
|
||||
view.width <= 0 || view.height <= 0
|
||||
) {
|
||||
// Rapid taps while a turn is animating jump instantly to
|
||||
// keep up with the reader's pace.
|
||||
navigate(nav, forward, animated = !effect.isRunning, token = token)
|
||||
// No photograph, so no lifted page, but the turn still has
|
||||
// to happen. It jumps rather than borrowing the navigator's
|
||||
// slide: what the lift gives the reader is the next page
|
||||
// now, and the part that cannot be drawn is the departing
|
||||
// one flying off. Standing a different style's motion in
|
||||
// its place made the same key turn the page two ways
|
||||
// depending on whether the menu was up, and left the footer
|
||||
// trailing the thumb, because Readium publishes its locator
|
||||
// when its scroller stops (#201).
|
||||
navigate(nav, forward, animated = false, token = token)
|
||||
return
|
||||
}
|
||||
copyPage(win, view) { bitmap ->
|
||||
|
|
|
|||
92
app/src/main/kotlin/com/chmouel/liseur/ui/SystemMotion.kt
Normal file
92
app/src/main/kotlin/com/chmouel/liseur/ui/SystemMotion.kt
Normal file
|
|
@ -0,0 +1,92 @@
|
|||
package com.chmouel.liseur.ui
|
||||
|
||||
import android.content.ContentResolver
|
||||
import android.database.ContentObserver
|
||||
import android.net.Uri
|
||||
import android.os.Handler
|
||||
import android.os.Looper
|
||||
import android.provider.Settings
|
||||
import androidx.compose.runtime.Composable
|
||||
import androidx.compose.runtime.DisposableEffect
|
||||
import androidx.compose.runtime.getValue
|
||||
import androidx.compose.runtime.mutableStateOf
|
||||
import androidx.compose.runtime.remember
|
||||
import androidx.compose.runtime.setValue
|
||||
import androidx.compose.ui.platform.LocalContext
|
||||
import androidx.lifecycle.Lifecycle
|
||||
import androidx.lifecycle.LifecycleEventObserver
|
||||
import androidx.lifecycle.compose.LocalLifecycleOwner
|
||||
|
||||
/**
|
||||
* Whether the system has been asked to stop animating things.
|
||||
*
|
||||
* Android spells that request as scale factors rather than as a switch:
|
||||
* Accessibility -> Remove animations sets all of them to zero at once,
|
||||
* and developer options can set each on its own. Anything at zero is a
|
||||
* reader who has said they do not want motion, so either is enough.
|
||||
*
|
||||
* Written as "not above zero" rather than "equal to zero", because the
|
||||
* value comes out of a settings table that anything with the right
|
||||
* permission can write and is parsed with `Float.parseFloat`, which
|
||||
* answers for `NaN` and for a negative number as readily as for a
|
||||
* scale. Neither of those is a slower animation, and neither compares
|
||||
* usefully with `<=`.
|
||||
*/
|
||||
fun motionRemoved(animatorScale: Float, transitionScale: Float): Boolean =
|
||||
!(animatorScale > 0f) || !(transitionScale > 0f)
|
||||
|
||||
/** The same question, asked of the system this build is running on. */
|
||||
fun motionRemoved(resolver: ContentResolver): Boolean = motionRemoved(
|
||||
animatorScale = scaleOf(resolver, Settings.Global.ANIMATOR_DURATION_SCALE),
|
||||
transitionScale = scaleOf(resolver, Settings.Global.TRANSITION_ANIMATION_SCALE),
|
||||
)
|
||||
|
||||
private fun scaleOf(resolver: ContentResolver, key: String): Float =
|
||||
Settings.Global.getFloat(resolver, key, 1f)
|
||||
|
||||
/**
|
||||
* Follows [motionRemoved] while the screen is up.
|
||||
*
|
||||
* Worth observing rather than reading once: the setting is two taps away
|
||||
* in Settings, and a reader who turns it off mid-book should not have to
|
||||
* close the book to be believed. Both keys are watched, since the
|
||||
* accessibility switch writes both and developer options writes either.
|
||||
*
|
||||
* Resuming re-reads them as well, and that is not belt and braces. On
|
||||
* Android 8 and 9 the settings provider notifies the system user only,
|
||||
* so a Liseur running in a work profile or a second user hears nothing;
|
||||
* and going to Settings to change this only pauses the reader, which is
|
||||
* not enough to bring the composition back. Coming back from Settings
|
||||
* is exactly when the answer has usually just changed.
|
||||
*/
|
||||
@Composable
|
||||
fun rememberMotionRemoved(): Boolean {
|
||||
val resolver = LocalContext.current.contentResolver
|
||||
val lifecycle = LocalLifecycleOwner.current.lifecycle
|
||||
var removed by remember(resolver) { mutableStateOf(motionRemoved(resolver)) }
|
||||
DisposableEffect(resolver, lifecycle) {
|
||||
val observer = object : ContentObserver(Handler(Looper.getMainLooper())) {
|
||||
override fun onChange(selfChange: Boolean, uri: Uri?) {
|
||||
removed = motionRemoved(resolver)
|
||||
}
|
||||
}
|
||||
listOf(
|
||||
Settings.Global.ANIMATOR_DURATION_SCALE,
|
||||
Settings.Global.TRANSITION_ANIMATION_SCALE,
|
||||
).forEach { key ->
|
||||
resolver.registerContentObserver(Settings.Global.getUriFor(key), false, observer)
|
||||
}
|
||||
val onResume = LifecycleEventObserver { _, event ->
|
||||
if (event == Lifecycle.Event.ON_RESUME) removed = motionRemoved(resolver)
|
||||
}
|
||||
lifecycle.addObserver(onResume)
|
||||
// The setting may have moved while this screen was away, and the
|
||||
// observer only hears what happens from here on.
|
||||
removed = motionRemoved(resolver)
|
||||
onDispose {
|
||||
resolver.unregisterContentObserver(observer)
|
||||
lifecycle.removeObserver(onResume)
|
||||
}
|
||||
}
|
||||
return removed
|
||||
}
|
||||
|
|
@ -58,4 +58,51 @@ class PageTurnStyleTest {
|
|||
pageTurnStyleFrom(stored = PageTurnStyle.SLIDE.id, legacyAnimation = false),
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an ordinary screen turns the page the way the reader asked`() {
|
||||
PageTurnStyle.entries.forEach { style ->
|
||||
assertEquals(
|
||||
style,
|
||||
pageTurnStyleOnScreen(chosen = style, eInk = false, motionRemoved = false),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `electronic paper jumps whatever was chosen`() {
|
||||
PageTurnStyle.entries.forEach { style ->
|
||||
assertEquals(
|
||||
PageTurnStyle.NONE,
|
||||
pageTurnStyleOnScreen(chosen = style, eInk = true, motionRemoved = false),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A reader who turned animations off in Android has already said
|
||||
* what they want; saying it again in Liseur's own settings is the
|
||||
* accessibility bug in #201.
|
||||
*/
|
||||
@Test
|
||||
fun `a system with animations removed jumps whatever was chosen`() {
|
||||
PageTurnStyle.entries.forEach { style ->
|
||||
assertEquals(
|
||||
PageTurnStyle.NONE,
|
||||
pageTurnStyleOnScreen(chosen = style, eInk = false, motionRemoved = true),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `the slide is overruled as readily as the lift`() {
|
||||
assertEquals(
|
||||
PageTurnStyle.NONE,
|
||||
pageTurnStyleOnScreen(
|
||||
chosen = PageTurnStyle.SLIDE,
|
||||
eInk = false,
|
||||
motionRemoved = true,
|
||||
),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,54 @@
|
|||
package com.chmouel.liseur.ui
|
||||
|
||||
import org.junit.Assert.assertFalse
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Test
|
||||
|
||||
/**
|
||||
* Reading Android's animation scales as the request they stand for.
|
||||
*/
|
||||
class SystemMotionTest {
|
||||
|
||||
@Test
|
||||
fun `an ordinary phone animates`() {
|
||||
assertFalse(motionRemoved(animatorScale = 1f, transitionScale = 1f))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `slowed-down animations are still animations`() {
|
||||
assertFalse(motionRemoved(animatorScale = 10f, transitionScale = 0.5f))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `the accessibility switch takes both scales down`() {
|
||||
assertTrue(motionRemoved(animatorScale = 0f, transitionScale = 0f))
|
||||
}
|
||||
|
||||
/**
|
||||
* Developer options sets each scale on its own, so either at zero is
|
||||
* a reader who has asked for less motion than the other one offers.
|
||||
*/
|
||||
@Test
|
||||
fun `either scale alone is enough`() {
|
||||
assertTrue(motionRemoved(animatorScale = 0f, transitionScale = 1f))
|
||||
assertTrue(motionRemoved(animatorScale = 1f, transitionScale = 0f))
|
||||
}
|
||||
|
||||
/**
|
||||
* The value comes out of a settings table rather than out of a
|
||||
* checkbox, and `Float.parseFloat` answers for anything that looks
|
||||
* like a number. A negative scale is not a slower animation, and
|
||||
* `NaN` is not a scale at all.
|
||||
*/
|
||||
@Test
|
||||
fun `a scale below zero reads as no motion`() {
|
||||
assertTrue(motionRemoved(animatorScale = -1f, transitionScale = 1f))
|
||||
assertTrue(motionRemoved(animatorScale = 1f, transitionScale = -0.25f))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a scale that is not a number reads as no motion`() {
|
||||
assertTrue(motionRemoved(animatorScale = Float.NaN, transitionScale = 1f))
|
||||
assertTrue(motionRemoved(animatorScale = 1f, transitionScale = Float.NaN))
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue