Fix six on-device issues found in the first real phone test
The cover one is the interesting bug. BookCover branched on
`painter.state`, but in coil3 that is a StateFlow<State>, not a State —
so every `is AsyncImagePainter.State.X` arm was always false. A `when`
used as a statement needs no else, so it compiled clean and drew
NOTHING: no cover, no placeholder, no error icon. That is why a
successfully looked-up book showed as a bare title floating in space.
Collect the flow before matching on it.
Two more cover defects sat behind that one:
- OpenLibraryDtos synthesized covers.openlibrary.org/b/isbn/{isbn}-L.jpg
unconditionally. For an edition with no art that URL answers 200 with a
43-byte 1x1 transparent GIF (verified against the live service), which
an image loader calls a successful load. So even with the state bug
fixed it would have painted an invisible cover and never fallen back.
Now the URL comes from OL's own `cover` object, which is present only
when art actually exists.
- Because that URL was never blank, MetadataMerger's "fill blanks from
the other source" rule could never reach Google Books' thumbnail. With
OL reporting null, the fallback works, and MetadataRepository adds the
by-ISBN URL as a genuine last resort — with `default=false`, so a miss
is a 404 the loader can report instead of a blank image.
The placeholder itself is now drawn in every non-success state (loading
included, where it previously drew an empty box) and reads as a book:
mahogany spine strip, gold hairline, letterpress panel.
Also:
- SyncStatusBar owns its navigation-bar inset and takes wider horizontal
padding, so it clears a phone's rounded display corners; it moves into
Scaffold's bottomBar slot so its height reaches the content padding.
- SetupScreen takes safeDrawingPadding outside verticalScroll, so the IME
shrinks the viewport instead of covering Password, plus Next/Next/Done
IME actions.
- Library card titles drop to a 20sp line height with the author given
its own 4dp gap: at titleSmall's 24sp leading a wrapped title left the
author closer to the last title line than the title lines were to each
other, so the byline read as part of the title.
- The scan sheet now names the ISBN it just read and says "Searching…",
so a decoded barcode is legible as decoded and the user can lower the
book instead of holding it to the camera at a bare spinner.
assembleDebug + assembleRelease exit 0; 106 unit tests, 0 failures;
verifyPaparazziDebug green against re-recorded snapshots.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CDcPottghJXEvfYKqFM7zf
@@ -28,6 +28,7 @@
|
|||||||
<activity
|
<activity
|
||||||
android:name=".MainActivity"
|
android:name=".MainActivity"
|
||||||
android:exported="true"
|
android:exported="true"
|
||||||
|
android:windowSoftInputMode="adjustResize"
|
||||||
android:theme="@style/Theme.Bookshelf">
|
android:theme="@style/Theme.Bookshelf">
|
||||||
<intent-filter>
|
<intent-filter>
|
||||||
<action android:name="android.intent.action.MAIN" />
|
<action android:name="android.intent.action.MAIN" />
|
||||||
|
|||||||
@@ -22,10 +22,28 @@ class MetadataRepository(
|
|||||||
|
|
||||||
suspend fun lookup(isbn: String): BookMetadata? {
|
suspend fun lookup(isbn: String): BookMetadata? {
|
||||||
val isbn13 = IsbnUtils.toIsbn13(isbn) ?: return null
|
val isbn13 = IsbnUtils.toIsbn13(isbn) ?: return null
|
||||||
return coroutineScope {
|
val merged = coroutineScope {
|
||||||
val openLibrary = async { openLibraryClient.lookup(isbn13) }
|
val openLibrary = async { openLibraryClient.lookup(isbn13) }
|
||||||
val googleBooks = async { googleBooksClient.lookup(isbn13) }
|
val googleBooks = async { googleBooksClient.lookup(isbn13) }
|
||||||
MetadataMerger.merge(openLibrary.await(), googleBooks.await())
|
MetadataMerger.merge(openLibrary.await(), googleBooks.await())
|
||||||
|
} ?: return null
|
||||||
|
return if (merged.coverUrl.isNullOrBlank()) {
|
||||||
|
merged.copy(coverUrl = byIsbnCoverUrl(isbn13))
|
||||||
|
} else {
|
||||||
|
merged
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
companion object {
|
||||||
|
/**
|
||||||
|
* Last resort when neither source reported cover art, per SPEC's cover chain.
|
||||||
|
* `default=false` is load-bearing: without it this endpoint answers 200 with a
|
||||||
|
* 1x1 transparent GIF for editions it has no art for, which an image loader
|
||||||
|
* treats as a successful load and paints as an invisible cover. With it, a
|
||||||
|
* miss is a 404, so [org.modg.bookshelf.ui.components.BookCover] can fall back
|
||||||
|
* to its placeholder.
|
||||||
|
*/
|
||||||
|
fun byIsbnCoverUrl(isbn13: String): String =
|
||||||
|
"https://covers.openlibrary.org/b/isbn/$isbn13-L.jpg?default=false"
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -18,6 +18,7 @@ data class OpenLibraryBookDto(
|
|||||||
@SerialName("publish_date") val publishDate: String? = null,
|
@SerialName("publish_date") val publishDate: String? = null,
|
||||||
@SerialName("number_of_pages") val numberOfPages: Int? = null,
|
@SerialName("number_of_pages") val numberOfPages: Int? = null,
|
||||||
val identifiers: OpenLibraryIdentifiersDto? = null,
|
val identifiers: OpenLibraryIdentifiersDto? = null,
|
||||||
|
val cover: OpenLibraryCoverDto? = null,
|
||||||
)
|
)
|
||||||
|
|
||||||
@Serializable
|
@Serializable
|
||||||
@@ -26,13 +27,33 @@ data class OpenLibraryAuthorDto(val name: String? = null)
|
|||||||
@Serializable
|
@Serializable
|
||||||
data class OpenLibraryPublisherDto(val name: String? = null)
|
data class OpenLibraryPublisherDto(val name: String? = null)
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Present only when Open Library actually holds cover art for the edition, and so
|
||||||
|
* the only trustworthy "has a cover" signal from this API. A synthesized by-ISBN
|
||||||
|
* covers.openlibrary.org URL is NOT evidence of one: for an edition with no art it
|
||||||
|
* answers 200 with a 43-byte 1x1 transparent GIF (verified 2026-09-09), which any
|
||||||
|
* image loader reports as a successful load — the cover then renders as nothing at
|
||||||
|
* all and no error placeholder ever fires. Only `?default=false` turns that into a
|
||||||
|
* 404; see [MetadataRepository] for the last-resort URL that uses it.
|
||||||
|
*/
|
||||||
|
@Serializable
|
||||||
|
data class OpenLibraryCoverDto(
|
||||||
|
val small: String? = null,
|
||||||
|
val medium: String? = null,
|
||||||
|
val large: String? = null,
|
||||||
|
)
|
||||||
|
|
||||||
@Serializable
|
@Serializable
|
||||||
data class OpenLibraryIdentifiersDto(
|
data class OpenLibraryIdentifiersDto(
|
||||||
@SerialName("isbn_10") val isbn10: List<String> = emptyList(),
|
@SerialName("isbn_10") val isbn10: List<String> = emptyList(),
|
||||||
@SerialName("isbn_13") val isbn13: List<String> = emptyList(),
|
@SerialName("isbn_13") val isbn13: List<String> = emptyList(),
|
||||||
)
|
)
|
||||||
|
|
||||||
/** Maps the OL DTO to the source-agnostic [BookMetadata], deriving the cover URL per SPEC. */
|
/**
|
||||||
|
* Maps the OL DTO to the source-agnostic [BookMetadata]. [coverUrl] stays null unless
|
||||||
|
* OL reports real cover art, so that the SPEC merge rule can fall through to Google
|
||||||
|
* Books' thumbnail instead of pinning a URL that resolves to a blank image.
|
||||||
|
*/
|
||||||
fun OpenLibraryBookDto.toBookMetadata(lookupIsbn13: String): BookMetadata = BookMetadata(
|
fun OpenLibraryBookDto.toBookMetadata(lookupIsbn13: String): BookMetadata = BookMetadata(
|
||||||
isbn13 = identifiers?.isbn13?.firstOrNull() ?: lookupIsbn13,
|
isbn13 = identifiers?.isbn13?.firstOrNull() ?: lookupIsbn13,
|
||||||
isbn10 = identifiers?.isbn10?.firstOrNull(),
|
isbn10 = identifiers?.isbn10?.firstOrNull(),
|
||||||
@@ -43,5 +64,5 @@ fun OpenLibraryBookDto.toBookMetadata(lookupIsbn13: String): BookMetadata = Book
|
|||||||
publishedDate = publishDate,
|
publishedDate = publishDate,
|
||||||
pageCount = numberOfPages,
|
pageCount = numberOfPages,
|
||||||
description = null,
|
description = null,
|
||||||
coverUrl = "https://covers.openlibrary.org/b/isbn/$lookupIsbn13-L.jpg",
|
coverUrl = cover?.large?.takeIf { it.isNotBlank() } ?: cover?.medium?.takeIf { it.isNotBlank() },
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -3,15 +3,20 @@ package org.modg.bookshelf.ui.components
|
|||||||
import androidx.compose.foundation.background
|
import androidx.compose.foundation.background
|
||||||
import androidx.compose.foundation.border
|
import androidx.compose.foundation.border
|
||||||
import androidx.compose.foundation.layout.Box
|
import androidx.compose.foundation.layout.Box
|
||||||
|
import androidx.compose.foundation.layout.Row
|
||||||
import androidx.compose.foundation.layout.aspectRatio
|
import androidx.compose.foundation.layout.aspectRatio
|
||||||
|
import androidx.compose.foundation.layout.fillMaxHeight
|
||||||
import androidx.compose.foundation.layout.fillMaxSize
|
import androidx.compose.foundation.layout.fillMaxSize
|
||||||
import androidx.compose.foundation.layout.padding
|
import androidx.compose.foundation.layout.padding
|
||||||
|
import androidx.compose.foundation.layout.width
|
||||||
import androidx.compose.material.icons.Icons
|
import androidx.compose.material.icons.Icons
|
||||||
import androidx.compose.material.icons.outlined.AutoStories
|
import androidx.compose.material.icons.outlined.AutoStories
|
||||||
import androidx.compose.material.icons.outlined.BrokenImage
|
import androidx.compose.material.icons.outlined.BrokenImage
|
||||||
import androidx.compose.material3.Icon
|
import androidx.compose.material3.Icon
|
||||||
import androidx.compose.material3.MaterialTheme
|
import androidx.compose.material3.MaterialTheme
|
||||||
import androidx.compose.runtime.Composable
|
import androidx.compose.runtime.Composable
|
||||||
|
import androidx.compose.runtime.collectAsState
|
||||||
|
import androidx.compose.runtime.getValue
|
||||||
import androidx.compose.ui.Alignment
|
import androidx.compose.ui.Alignment
|
||||||
import androidx.compose.ui.Modifier
|
import androidx.compose.ui.Modifier
|
||||||
import androidx.compose.ui.draw.clip
|
import androidx.compose.ui.draw.clip
|
||||||
@@ -27,9 +32,12 @@ const val BookCoverAspectRatio = 2f / 3f
|
|||||||
/**
|
/**
|
||||||
* A book cover image, always drawn at [BookCoverAspectRatio]. Covers are the
|
* A book cover image, always drawn at [BookCoverAspectRatio]. Covers are the
|
||||||
* hero of this app's design — real art fills the whole shape edge to edge.
|
* hero of this app's design — real art fills the whole shape edge to edge.
|
||||||
* When there's no [coverUrl], or the load fails, we fall back to the same
|
* When there's no [coverUrl], or the load fails, or one is still in flight, we
|
||||||
* restrained "letterpress" placeholder: a paper-toned panel with a debossed
|
* fall back to the same restrained "letterpress" placeholder: a paper-toned panel
|
||||||
* spine motif rather than a broken-image icon or empty grey box.
|
* with a debossed spine motif rather than a broken-image icon or empty grey box.
|
||||||
|
* The placeholder is drawn in EVERY non-success state on purpose — a cover slot
|
||||||
|
* that renders nothing at all leaves the title floating in blank space, which is
|
||||||
|
* exactly what a transparent 1x1 stand-in cover used to produce.
|
||||||
*/
|
*/
|
||||||
@Composable
|
@Composable
|
||||||
fun BookCover(
|
fun BookCover(
|
||||||
@@ -52,11 +60,16 @@ fun BookCover(
|
|||||||
modifier = Modifier.fillMaxSize(),
|
modifier = Modifier.fillMaxSize(),
|
||||||
contentScale = ContentScale.Crop,
|
contentScale = ContentScale.Crop,
|
||||||
) {
|
) {
|
||||||
when (painter.state) {
|
// painter.state is a StateFlow<State>, NOT a State. Branching on it
|
||||||
|
// directly compiles (a `when` used as a statement needs no else) but
|
||||||
|
// every branch is always false, so the slot draws nothing at all —
|
||||||
|
// no cover and no placeholder. Collect it before matching.
|
||||||
|
val state by painter.state.collectAsState()
|
||||||
|
when (state) {
|
||||||
is AsyncImagePainter.State.Error -> CoverPlaceholder(errored = true)
|
is AsyncImagePainter.State.Error -> CoverPlaceholder(errored = true)
|
||||||
is AsyncImagePainter.State.Loading,
|
is AsyncImagePainter.State.Loading,
|
||||||
is AsyncImagePainter.State.Empty,
|
is AsyncImagePainter.State.Empty,
|
||||||
-> CoverPlaceholder(errored = false, loading = true)
|
-> CoverPlaceholder(errored = false)
|
||||||
is AsyncImagePainter.State.Success -> SubcomposeAsyncImageContent()
|
is AsyncImagePainter.State.Success -> SubcomposeAsyncImageContent()
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -65,25 +78,39 @@ fun BookCover(
|
|||||||
}
|
}
|
||||||
|
|
||||||
@Composable
|
@Composable
|
||||||
private fun CoverPlaceholder(errored: Boolean, loading: Boolean = false) {
|
private fun CoverPlaceholder(errored: Boolean) {
|
||||||
val paperAlt = MaterialTheme.colorScheme.surfaceVariant
|
val paperAlt = MaterialTheme.colorScheme.surfaceVariant
|
||||||
val ink = MaterialTheme.colorScheme.onSurfaceVariant
|
val ink = MaterialTheme.colorScheme.onSurfaceVariant
|
||||||
|
val spine = MaterialTheme.colorScheme.primary
|
||||||
|
val gold = MaterialTheme.colorScheme.secondary
|
||||||
Box(
|
Box(
|
||||||
modifier = Modifier
|
modifier = Modifier
|
||||||
.fillMaxSize()
|
.fillMaxSize()
|
||||||
.background(paperAlt)
|
.background(paperAlt)
|
||||||
.border(width = 1.dp, color = ink.copy(alpha = 0.15f))
|
.border(width = 1.dp, color = ink.copy(alpha = 0.22f)),
|
||||||
.padding(2.dp)
|
|
||||||
.border(width = 1.dp, color = ink.copy(alpha = 0.1f)),
|
|
||||||
contentAlignment = Alignment.Center,
|
contentAlignment = Alignment.Center,
|
||||||
) {
|
) {
|
||||||
if (!loading) {
|
// A bound: a mahogany spine strip down the left edge with a gold hairline
|
||||||
Icon(
|
// beside it. This is what makes an art-less cover still read as a book.
|
||||||
imageVector = if (errored) Icons.Outlined.BrokenImage else Icons.Outlined.AutoStories,
|
Row(modifier = Modifier.fillMaxSize()) {
|
||||||
contentDescription = null,
|
Box(
|
||||||
tint = ink.copy(alpha = if (errored) 0.35f else 0.28f),
|
modifier = Modifier
|
||||||
modifier = Modifier.fillMaxSize(0.32f),
|
.fillMaxHeight()
|
||||||
|
.width(10.dp)
|
||||||
|
.background(spine.copy(alpha = 0.35f)),
|
||||||
|
)
|
||||||
|
Box(
|
||||||
|
modifier = Modifier
|
||||||
|
.fillMaxHeight()
|
||||||
|
.width(1.dp)
|
||||||
|
.background(gold.copy(alpha = 0.55f)),
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
Icon(
|
||||||
|
imageVector = if (errored) Icons.Outlined.BrokenImage else Icons.Outlined.AutoStories,
|
||||||
|
contentDescription = null,
|
||||||
|
tint = ink.copy(alpha = 0.4f),
|
||||||
|
modifier = Modifier.fillMaxSize(0.3f),
|
||||||
|
)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -2,7 +2,6 @@ package org.modg.bookshelf.ui.components
|
|||||||
|
|
||||||
import androidx.compose.foundation.layout.Column
|
import androidx.compose.foundation.layout.Column
|
||||||
import androidx.compose.foundation.layout.PaddingValues
|
import androidx.compose.foundation.layout.PaddingValues
|
||||||
import androidx.compose.foundation.layout.fillMaxSize
|
|
||||||
import androidx.compose.material3.CenterAlignedTopAppBar
|
import androidx.compose.material3.CenterAlignedTopAppBar
|
||||||
import androidx.compose.material3.ExperimentalMaterial3Api
|
import androidx.compose.material3.ExperimentalMaterial3Api
|
||||||
import androidx.compose.material3.FloatingActionButton
|
import androidx.compose.material3.FloatingActionButton
|
||||||
@@ -18,6 +17,12 @@ import androidx.compose.ui.Modifier
|
|||||||
* screen title and a gold hairline rule underneath, plus optional nav/action
|
* screen title and a gold hairline rule underneath, plus optional nav/action
|
||||||
* slots, FAB, and a bottom [SyncStatusBar] slot. Screens should reach for
|
* slots, FAB, and a bottom [SyncStatusBar] slot. Screens should reach for
|
||||||
* this instead of a bare [Scaffold] so the chrome stays consistent.
|
* this instead of a bare [Scaffold] so the chrome stays consistent.
|
||||||
|
*
|
||||||
|
* [syncStatusBar] goes in the Scaffold's own bottom-bar slot rather than in a
|
||||||
|
* hand-rolled Column under the content: that is what makes the height it
|
||||||
|
* occupies show up in the [PaddingValues] handed to [content], so a screen that
|
||||||
|
* applies them can't scroll its last row underneath the status line. The bar
|
||||||
|
* takes the navigation-bar inset itself (see [SyncStatusBar]).
|
||||||
*/
|
*/
|
||||||
@OptIn(ExperimentalMaterial3Api::class)
|
@OptIn(ExperimentalMaterial3Api::class)
|
||||||
@Composable
|
@Composable
|
||||||
@@ -49,14 +54,8 @@ fun BookshelfScaffold(
|
|||||||
}
|
}
|
||||||
},
|
},
|
||||||
floatingActionButton = floatingActionButton,
|
floatingActionButton = floatingActionButton,
|
||||||
|
bottomBar = syncStatusBar,
|
||||||
containerColor = MaterialTheme.colorScheme.surface,
|
containerColor = MaterialTheme.colorScheme.surface,
|
||||||
content = { innerPadding ->
|
content = content,
|
||||||
Column(modifier = Modifier.fillMaxSize()) {
|
|
||||||
Column(modifier = Modifier.weight(1f)) {
|
|
||||||
content(innerPadding)
|
|
||||||
}
|
|
||||||
syncStatusBar()
|
|
||||||
}
|
|
||||||
},
|
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -11,6 +11,7 @@ import androidx.compose.foundation.layout.Arrangement
|
|||||||
import androidx.compose.foundation.layout.Box
|
import androidx.compose.foundation.layout.Box
|
||||||
import androidx.compose.foundation.layout.Row
|
import androidx.compose.foundation.layout.Row
|
||||||
import androidx.compose.foundation.layout.fillMaxWidth
|
import androidx.compose.foundation.layout.fillMaxWidth
|
||||||
|
import androidx.compose.foundation.layout.navigationBarsPadding
|
||||||
import androidx.compose.foundation.layout.padding
|
import androidx.compose.foundation.layout.padding
|
||||||
import androidx.compose.foundation.layout.size
|
import androidx.compose.foundation.layout.size
|
||||||
import androidx.compose.foundation.shape.CircleShape
|
import androidx.compose.foundation.shape.CircleShape
|
||||||
@@ -38,8 +39,13 @@ enum class SyncStatus {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* A slim, quiet status line — never a blocking banner or dialog. Sits at the
|
* A slim, quiet status line — never a blocking banner or dialog. Sits in
|
||||||
* bottom of [BookshelfScaffold] screens.
|
* [BookshelfScaffold]'s bottom-bar slot.
|
||||||
|
*
|
||||||
|
* The app draws edge to edge, so this is the one composable that sits against
|
||||||
|
* the very bottom of the display: it owns the navigation-bar inset itself, and
|
||||||
|
* its horizontal padding is deliberately wider than the app's usual 16dp so the
|
||||||
|
* dot and label clear a phone's rounded display corners.
|
||||||
*/
|
*/
|
||||||
@Composable
|
@Composable
|
||||||
fun SyncStatusBar(
|
fun SyncStatusBar(
|
||||||
@@ -51,7 +57,8 @@ fun SyncStatusBar(
|
|||||||
modifier = modifier
|
modifier = modifier
|
||||||
.fillMaxWidth()
|
.fillMaxWidth()
|
||||||
.background(MaterialTheme.colorScheme.surfaceContainerLow)
|
.background(MaterialTheme.colorScheme.surfaceContainerLow)
|
||||||
.padding(horizontal = 16.dp, vertical = 8.dp),
|
.navigationBarsPadding()
|
||||||
|
.padding(horizontal = 28.dp, vertical = 8.dp),
|
||||||
verticalAlignment = Alignment.CenterVertically,
|
verticalAlignment = Alignment.CenterVertically,
|
||||||
horizontalArrangement = Arrangement.spacedBy(8.dp),
|
horizontalArrangement = Arrangement.spacedBy(8.dp),
|
||||||
) {
|
) {
|
||||||
|
|||||||
@@ -39,6 +39,7 @@ import androidx.compose.ui.Alignment
|
|||||||
import androidx.compose.ui.Modifier
|
import androidx.compose.ui.Modifier
|
||||||
import androidx.compose.ui.text.style.TextOverflow
|
import androidx.compose.ui.text.style.TextOverflow
|
||||||
import androidx.compose.ui.unit.dp
|
import androidx.compose.ui.unit.dp
|
||||||
|
import androidx.compose.ui.unit.sp
|
||||||
import androidx.lifecycle.viewmodel.compose.viewModel
|
import androidx.lifecycle.viewmodel.compose.viewModel
|
||||||
import androidx.lifecycle.viewmodel.initializer
|
import androidx.lifecycle.viewmodel.initializer
|
||||||
import androidx.lifecycle.viewmodel.viewModelFactory
|
import androidx.lifecycle.viewmodel.viewModelFactory
|
||||||
@@ -264,9 +265,14 @@ internal fun LibraryBookCard(book: BookEntity, onClick: () -> Unit) {
|
|||||||
contentDescription = book.title,
|
contentDescription = book.title,
|
||||||
modifier = Modifier.fillMaxWidth(),
|
modifier = Modifier.fillMaxWidth(),
|
||||||
)
|
)
|
||||||
|
// titleSmall's 24sp line height is right for a paragraph and wrong here: on a
|
||||||
|
// wrapped two-line title it opened a bigger gap between the title's own lines
|
||||||
|
// than between the title and the author beneath it, so the author read as part
|
||||||
|
// of the title block. Tighten the leading and give the author its own gap, so
|
||||||
|
// the card groups as one title + one byline.
|
||||||
Text(
|
Text(
|
||||||
text = book.title,
|
text = book.title,
|
||||||
style = MaterialTheme.typography.titleSmall,
|
style = MaterialTheme.typography.titleSmall.copy(lineHeight = 20.sp),
|
||||||
color = MaterialTheme.colorScheme.onSurface,
|
color = MaterialTheme.colorScheme.onSurface,
|
||||||
maxLines = 2,
|
maxLines = 2,
|
||||||
overflow = TextOverflow.Ellipsis,
|
overflow = TextOverflow.Ellipsis,
|
||||||
@@ -280,6 +286,7 @@ internal fun LibraryBookCard(book: BookEntity, onClick: () -> Unit) {
|
|||||||
color = MaterialTheme.colorScheme.onSurfaceVariant,
|
color = MaterialTheme.colorScheme.onSurfaceVariant,
|
||||||
maxLines = 1,
|
maxLines = 1,
|
||||||
overflow = TextOverflow.Ellipsis,
|
overflow = TextOverflow.Ellipsis,
|
||||||
|
modifier = Modifier.padding(top = 4.dp),
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -18,7 +18,14 @@ object DuplicateCheck {
|
|||||||
/** What the scan bottom sheet is currently showing. */
|
/** What the scan bottom sheet is currently showing. */
|
||||||
sealed interface ScanSheetState {
|
sealed interface ScanSheetState {
|
||||||
data object Hidden : ScanSheetState
|
data object Hidden : ScanSheetState
|
||||||
data object Loading : ScanSheetState
|
|
||||||
|
/**
|
||||||
|
* Carries [isbn13] so the sheet can name the code it just read. A bare spinner
|
||||||
|
* doesn't tell the user the barcode was recognised, and they keep holding the
|
||||||
|
* book up to the camera; echoing the number back is the signal that they can
|
||||||
|
* lower it.
|
||||||
|
*/
|
||||||
|
data class Loading(val isbn13: String) : ScanSheetState
|
||||||
data class Found(val isbn13: String, val metadata: BookMetadata, val duplicate: DuplicateStatus) : ScanSheetState
|
data class Found(val isbn13: String, val metadata: BookMetadata, val duplicate: DuplicateStatus) : ScanSheetState
|
||||||
data class NotFound(val isbn13: String) : ScanSheetState
|
data class NotFound(val isbn13: String) : ScanSheetState
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -137,9 +137,7 @@ fun ScanScreen(
|
|||||||
when (val state = sheetState) {
|
when (val state = sheetState) {
|
||||||
is ScanSheetState.Hidden -> Unit
|
is ScanSheetState.Hidden -> Unit
|
||||||
is ScanSheetState.Loading -> ModalBottomSheet(onDismissRequest = { }, sheetState = rememberModalBottomSheetState()) {
|
is ScanSheetState.Loading -> ModalBottomSheet(onDismissRequest = { }, sheetState = rememberModalBottomSheetState()) {
|
||||||
Box(modifier = Modifier.fillMaxWidth().padding(32.dp), contentAlignment = Alignment.Center) {
|
SearchingSheet(isbn13 = state.isbn13)
|
||||||
CircularProgressIndicator()
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
is ScanSheetState.Found -> ModalBottomSheet(
|
is ScanSheetState.Found -> ModalBottomSheet(
|
||||||
onDismissRequest = { viewModel.dismissSheet() },
|
onDismissRequest = { viewModel.dismissSheet() },
|
||||||
@@ -238,6 +236,40 @@ internal fun ScanReticle(modifier: Modifier = Modifier) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Shown the moment a barcode is decoded, while the metadata lookup runs. It names
|
||||||
|
* the ISBN it read and says so in words, because a lone spinner reads as "still
|
||||||
|
* working on it" — the user goes on holding the book up to the camera when the
|
||||||
|
* camera is already done with it.
|
||||||
|
*/
|
||||||
|
@Composable
|
||||||
|
internal fun SearchingSheet(isbn13: String) {
|
||||||
|
Column(
|
||||||
|
modifier = Modifier.fillMaxWidth().padding(horizontal = 24.dp, vertical = 32.dp),
|
||||||
|
horizontalAlignment = Alignment.CenterHorizontally,
|
||||||
|
) {
|
||||||
|
CircularProgressIndicator()
|
||||||
|
Text(
|
||||||
|
text = "Searching…",
|
||||||
|
style = MaterialTheme.typography.titleMedium,
|
||||||
|
color = MaterialTheme.colorScheme.onSurface,
|
||||||
|
modifier = Modifier.padding(top = 20.dp),
|
||||||
|
)
|
||||||
|
Text(
|
||||||
|
text = "ISBN $isbn13",
|
||||||
|
style = MaterialTheme.typography.bodyMedium,
|
||||||
|
color = MaterialTheme.colorScheme.onSurfaceVariant,
|
||||||
|
modifier = Modifier.padding(top = 4.dp),
|
||||||
|
)
|
||||||
|
Text(
|
||||||
|
text = "Barcode read — you can lower the book.",
|
||||||
|
style = MaterialTheme.typography.bodySmall,
|
||||||
|
color = MaterialTheme.colorScheme.onSurfaceVariant,
|
||||||
|
modifier = Modifier.padding(top = 12.dp),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
@Composable
|
@Composable
|
||||||
internal fun SessionBadge(count: Int, modifier: Modifier = Modifier) {
|
internal fun SessionBadge(count: Int, modifier: Modifier = Modifier) {
|
||||||
if (count == 0) return
|
if (count == 0) return
|
||||||
|
|||||||
@@ -52,7 +52,7 @@ class ScanViewModel(
|
|||||||
|
|
||||||
private suspend fun onScanned(isbn13: String) {
|
private suspend fun onScanned(isbn13: String) {
|
||||||
if (_sheetState.value !is ScanSheetState.Hidden) return // a sheet is already up for a previous hit
|
if (_sheetState.value !is ScanSheetState.Hidden) return // a sheet is already up for a previous hit
|
||||||
_sheetState.value = ScanSheetState.Loading
|
_sheetState.value = ScanSheetState.Loading(isbn13)
|
||||||
val duplicate = DuplicateCheck.check(bookRepository.findByIsbn13(isbn13))
|
val duplicate = DuplicateCheck.check(bookRepository.findByIsbn13(isbn13))
|
||||||
val metadata = metadataRepository.lookup(isbn13)
|
val metadata = metadataRepository.lookup(isbn13)
|
||||||
_sheetState.value = ScanMetadataOutcome.from(isbn13, metadata, duplicate)
|
_sheetState.value = ScanMetadataOutcome.from(isbn13, metadata, duplicate)
|
||||||
|
|||||||
@@ -4,7 +4,9 @@ import androidx.compose.foundation.layout.Arrangement
|
|||||||
import androidx.compose.foundation.layout.Column
|
import androidx.compose.foundation.layout.Column
|
||||||
import androidx.compose.foundation.layout.fillMaxSize
|
import androidx.compose.foundation.layout.fillMaxSize
|
||||||
import androidx.compose.foundation.layout.fillMaxWidth
|
import androidx.compose.foundation.layout.fillMaxWidth
|
||||||
|
import androidx.compose.foundation.layout.safeDrawingPadding
|
||||||
import androidx.compose.foundation.layout.padding
|
import androidx.compose.foundation.layout.padding
|
||||||
|
import androidx.compose.foundation.text.KeyboardActions
|
||||||
import androidx.compose.foundation.text.KeyboardOptions
|
import androidx.compose.foundation.text.KeyboardOptions
|
||||||
import androidx.compose.foundation.verticalScroll
|
import androidx.compose.foundation.verticalScroll
|
||||||
import androidx.compose.foundation.rememberScrollState
|
import androidx.compose.foundation.rememberScrollState
|
||||||
@@ -15,6 +17,9 @@ import androidx.compose.runtime.Composable
|
|||||||
import androidx.compose.runtime.collectAsState
|
import androidx.compose.runtime.collectAsState
|
||||||
import androidx.compose.runtime.getValue
|
import androidx.compose.runtime.getValue
|
||||||
import androidx.compose.ui.Modifier
|
import androidx.compose.ui.Modifier
|
||||||
|
import androidx.compose.ui.focus.FocusDirection
|
||||||
|
import androidx.compose.ui.platform.LocalFocusManager
|
||||||
|
import androidx.compose.ui.text.input.ImeAction
|
||||||
import androidx.compose.ui.text.input.KeyboardType
|
import androidx.compose.ui.text.input.KeyboardType
|
||||||
import androidx.compose.ui.text.input.PasswordVisualTransformation
|
import androidx.compose.ui.text.input.PasswordVisualTransformation
|
||||||
import androidx.compose.ui.unit.dp
|
import androidx.compose.ui.unit.dp
|
||||||
@@ -38,11 +43,23 @@ fun SetupScreen(onSetupComplete: () -> Unit, container: AppContainer) {
|
|||||||
},
|
},
|
||||||
)
|
)
|
||||||
val state by viewModel.uiState.collectAsState()
|
val state by viewModel.uiState.collectAsState()
|
||||||
|
val focusManager = LocalFocusManager.current
|
||||||
|
|
||||||
|
val canSubmit = !state.isSubmitting &&
|
||||||
|
state.serverUrl.isNotBlank() &&
|
||||||
|
state.email.isNotBlank() &&
|
||||||
|
state.password.isNotBlank()
|
||||||
|
|
||||||
PaperSurface(modifier = Modifier.fillMaxSize()) {
|
PaperSurface(modifier = Modifier.fillMaxSize()) {
|
||||||
|
// This screen has no Scaffold of its own, so it owns its window insets.
|
||||||
|
// safeDrawingPadding covers the IME as well as the system bars, and it sits
|
||||||
|
// OUTSIDE verticalScroll on purpose: the keyboard then shrinks the scrollable
|
||||||
|
// viewport rather than covering it, so Compose brings the newly focused field
|
||||||
|
// into view instead of leaving Password stranded behind the IME.
|
||||||
Column(
|
Column(
|
||||||
modifier = Modifier
|
modifier = Modifier
|
||||||
.fillMaxSize()
|
.fillMaxSize()
|
||||||
|
.safeDrawingPadding()
|
||||||
.verticalScroll(rememberScrollState())
|
.verticalScroll(rememberScrollState())
|
||||||
.padding(24.dp),
|
.padding(24.dp),
|
||||||
verticalArrangement = Arrangement.Center,
|
verticalArrangement = Arrangement.Center,
|
||||||
@@ -61,7 +78,11 @@ fun SetupScreen(onSetupComplete: () -> Unit, container: AppContainer) {
|
|||||||
label = { Text("Server URL") },
|
label = { Text("Server URL") },
|
||||||
placeholder = { Text("https://library.example.com") },
|
placeholder = { Text("https://library.example.com") },
|
||||||
singleLine = true,
|
singleLine = true,
|
||||||
keyboardOptions = KeyboardOptions(keyboardType = KeyboardType.Uri),
|
keyboardOptions = KeyboardOptions(
|
||||||
|
keyboardType = KeyboardType.Uri,
|
||||||
|
imeAction = ImeAction.Next,
|
||||||
|
),
|
||||||
|
keyboardActions = KeyboardActions(onNext = { focusManager.moveFocus(FocusDirection.Down) }),
|
||||||
isError = state.urlError != null,
|
isError = state.urlError != null,
|
||||||
modifier = Modifier.fillMaxWidth(),
|
modifier = Modifier.fillMaxWidth(),
|
||||||
)
|
)
|
||||||
@@ -81,7 +102,11 @@ fun SetupScreen(onSetupComplete: () -> Unit, container: AppContainer) {
|
|||||||
onValueChange = viewModel::onEmailChanged,
|
onValueChange = viewModel::onEmailChanged,
|
||||||
label = { Text("Email") },
|
label = { Text("Email") },
|
||||||
singleLine = true,
|
singleLine = true,
|
||||||
keyboardOptions = KeyboardOptions(keyboardType = KeyboardType.Email),
|
keyboardOptions = KeyboardOptions(
|
||||||
|
keyboardType = KeyboardType.Email,
|
||||||
|
imeAction = ImeAction.Next,
|
||||||
|
),
|
||||||
|
keyboardActions = KeyboardActions(onNext = { focusManager.moveFocus(FocusDirection.Down) }),
|
||||||
isError = state.credentialsError != null,
|
isError = state.credentialsError != null,
|
||||||
modifier = Modifier.fillMaxWidth().padding(top = 8.dp),
|
modifier = Modifier.fillMaxWidth().padding(top = 8.dp),
|
||||||
)
|
)
|
||||||
@@ -91,7 +116,16 @@ fun SetupScreen(onSetupComplete: () -> Unit, container: AppContainer) {
|
|||||||
label = { Text("Password") },
|
label = { Text("Password") },
|
||||||
singleLine = true,
|
singleLine = true,
|
||||||
visualTransformation = PasswordVisualTransformation(),
|
visualTransformation = PasswordVisualTransformation(),
|
||||||
keyboardOptions = KeyboardOptions(keyboardType = KeyboardType.Password),
|
keyboardOptions = KeyboardOptions(
|
||||||
|
keyboardType = KeyboardType.Password,
|
||||||
|
imeAction = ImeAction.Done,
|
||||||
|
),
|
||||||
|
keyboardActions = KeyboardActions(
|
||||||
|
onDone = {
|
||||||
|
focusManager.clearFocus()
|
||||||
|
if (canSubmit) viewModel.submit(onSetupComplete)
|
||||||
|
},
|
||||||
|
),
|
||||||
isError = state.credentialsError != null,
|
isError = state.credentialsError != null,
|
||||||
modifier = Modifier.fillMaxWidth().padding(top = 8.dp),
|
modifier = Modifier.fillMaxWidth().padding(top = 8.dp),
|
||||||
)
|
)
|
||||||
@@ -107,10 +141,7 @@ fun SetupScreen(onSetupComplete: () -> Unit, container: AppContainer) {
|
|||||||
PrimaryButton(
|
PrimaryButton(
|
||||||
text = if (state.isSubmitting) "Signing in…" else "Sign in",
|
text = if (state.isSubmitting) "Signing in…" else "Sign in",
|
||||||
onClick = { viewModel.submit(onSetupComplete) },
|
onClick = { viewModel.submit(onSetupComplete) },
|
||||||
enabled = !state.isSubmitting &&
|
enabled = canSubmit,
|
||||||
state.serverUrl.isNotBlank() &&
|
|
||||||
state.email.isNotBlank() &&
|
|
||||||
state.password.isNotBlank(),
|
|
||||||
modifier = Modifier.padding(top = 24.dp),
|
modifier = Modifier.padding(top = 24.dp),
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,25 @@
|
|||||||
|
package org.modg.bookshelf.data.metadata
|
||||||
|
|
||||||
|
import org.junit.Assert.assertEquals
|
||||||
|
import org.junit.Assert.assertTrue
|
||||||
|
import org.junit.Test
|
||||||
|
|
||||||
|
class MetadataRepositoryTest {
|
||||||
|
|
||||||
|
/**
|
||||||
|
* `default=false` is the whole point of this URL. Without it covers.openlibrary.org
|
||||||
|
* answers 200 with a 43-byte 1x1 transparent GIF for an edition it holds no art for
|
||||||
|
* — an image loader calls that a successful load, so the cover slot renders empty
|
||||||
|
* and the placeholder never appears. With it, a miss is a 404 the loader can report.
|
||||||
|
*/
|
||||||
|
@Test
|
||||||
|
fun `by-isbn cover url disables the blank stand-in image`() {
|
||||||
|
val url = MetadataRepository.byIsbnCoverUrl("9780201558029")
|
||||||
|
|
||||||
|
assertEquals(
|
||||||
|
"https://covers.openlibrary.org/b/isbn/9780201558029-L.jpg?default=false",
|
||||||
|
url,
|
||||||
|
)
|
||||||
|
assertTrue("must opt out of the 1x1 stand-in", url.contains("default=false"))
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -29,7 +29,36 @@ class OpenLibraryClientTest {
|
|||||||
assertEquals(672, result.pageCount)
|
assertEquals(672, result.pageCount)
|
||||||
assertEquals("9780201558029", result.isbn13)
|
assertEquals("9780201558029", result.isbn13)
|
||||||
assertEquals("0201558025", result.isbn10)
|
assertEquals("0201558025", result.isbn10)
|
||||||
assertEquals("https://covers.openlibrary.org/b/isbn/9780201558029-L.jpg", result.coverUrl)
|
assertEquals("https://covers.openlibrary.org/b/id/675832-L.jpg", result.coverUrl)
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The by-ISBN cover endpoint answers 200 with a 1x1 transparent GIF for editions
|
||||||
|
* with no art, so synthesizing that URL here would hand the UI a cover that loads
|
||||||
|
* "successfully" and paints nothing. No `cover` object means no cover URL, which
|
||||||
|
* is what lets [MetadataMerger] fall through to Google Books' thumbnail.
|
||||||
|
*/
|
||||||
|
@Test
|
||||||
|
fun `leaves coverUrl null when the response reports no cover art`() {
|
||||||
|
val body = """
|
||||||
|
{"ISBN:9780201558029": {"title": "Concrete Mathematics", "publishers": [{"name": "AW"}]}}
|
||||||
|
""".trimIndent()
|
||||||
|
val result = checkNotNull(client.parseResponse(body, "9780201558029"))
|
||||||
|
|
||||||
|
assertEquals("Concrete Mathematics", result.title)
|
||||||
|
assertNull(result.coverUrl)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `falls back to the medium cover when no large one is offered`() {
|
||||||
|
val body = """
|
||||||
|
{"ISBN:9780201558029": {"title": "Concrete Mathematics",
|
||||||
|
"cover": {"small": "https://covers.openlibrary.org/b/id/675832-S.jpg",
|
||||||
|
"medium": "https://covers.openlibrary.org/b/id/675832-M.jpg"}}}
|
||||||
|
""".trimIndent()
|
||||||
|
val result = checkNotNull(client.parseResponse(body, "9780201558029"))
|
||||||
|
|
||||||
|
assertEquals("https://covers.openlibrary.org/b/id/675832-M.jpg", result.coverUrl)
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
|||||||
@@ -28,6 +28,7 @@ import org.modg.bookshelf.ui.components.BookshelfScaffold
|
|||||||
import org.modg.bookshelf.ui.scan.DuplicateStatus
|
import org.modg.bookshelf.ui.scan.DuplicateStatus
|
||||||
import org.modg.bookshelf.ui.scan.FoundBookSheet
|
import org.modg.bookshelf.ui.scan.FoundBookSheet
|
||||||
import org.modg.bookshelf.ui.scan.ScanReticle
|
import org.modg.bookshelf.ui.scan.ScanReticle
|
||||||
|
import org.modg.bookshelf.ui.scan.SearchingSheet
|
||||||
import org.modg.bookshelf.ui.scan.SessionBadge
|
import org.modg.bookshelf.ui.scan.SessionBadge
|
||||||
import org.modg.bookshelf.ui.theme.BookshelfTheme
|
import org.modg.bookshelf.ui.theme.BookshelfTheme
|
||||||
|
|
||||||
@@ -52,6 +53,10 @@ class ScanScreenPaparazziTest {
|
|||||||
@Test
|
@Test
|
||||||
fun scanFoundSheetLight() = snapshotBoth("scan-found-sheet") { FoundSheetOverlay() }
|
fun scanFoundSheetLight() = snapshotBoth("scan-found-sheet") { FoundSheetOverlay() }
|
||||||
|
|
||||||
|
/** The state between "barcode decoded" and "metadata back" — see [SearchingSheet]. */
|
||||||
|
@Test
|
||||||
|
fun scanSearchingSheetLight() = snapshotBoth("scan-searching-sheet") { SearchingSheetOverlay() }
|
||||||
|
|
||||||
@Composable
|
@Composable
|
||||||
private fun ReticleOverlay() = Shell {
|
private fun ReticleOverlay() = Shell {
|
||||||
Box(modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center) {
|
Box(modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center) {
|
||||||
@@ -89,6 +94,19 @@ class ScanScreenPaparazziTest {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Composable
|
||||||
|
private fun SearchingSheetOverlay() = Shell {
|
||||||
|
Box(modifier = Modifier.fillMaxSize()) {
|
||||||
|
Surface(
|
||||||
|
modifier = Modifier.align(Alignment.BottomCenter).fillMaxWidth(),
|
||||||
|
shape = RoundedCornerShape(topStart = 16.dp, topEnd = 16.dp),
|
||||||
|
color = MaterialTheme.colorScheme.surface,
|
||||||
|
) {
|
||||||
|
SearchingSheet(isbn13 = "9780765326355")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
@Composable
|
@Composable
|
||||||
private fun Shell(overlay: @Composable () -> Unit) {
|
private fun Shell(overlay: @Composable () -> Unit) {
|
||||||
BookshelfScaffold(
|
BookshelfScaffold(
|
||||||
|
|||||||
|
Before Width: | Height: | Size: 6.8 KiB After Width: | Height: | Size: 6.7 KiB |
|
Before Width: | Height: | Size: 6.8 KiB After Width: | Height: | Size: 7.1 KiB |
|
Before Width: | Height: | Size: 21 KiB After Width: | Height: | Size: 21 KiB |
|
Before Width: | Height: | Size: 20 KiB After Width: | Height: | Size: 20 KiB |
|
Before Width: | Height: | Size: 11 KiB After Width: | Height: | Size: 11 KiB |
|
Before Width: | Height: | Size: 11 KiB After Width: | Height: | Size: 11 KiB |
|
Before Width: | Height: | Size: 65 KiB After Width: | Height: | Size: 65 KiB |
|
Before Width: | Height: | Size: 64 KiB After Width: | Height: | Size: 64 KiB |
|
Before Width: | Height: | Size: 25 KiB After Width: | Height: | Size: 25 KiB |
|
Before Width: | Height: | Size: 26 KiB After Width: | Height: | Size: 26 KiB |
|
Before Width: | Height: | Size: 50 KiB After Width: | Height: | Size: 51 KiB |
|
Before Width: | Height: | Size: 52 KiB After Width: | Height: | Size: 54 KiB |
|
Before Width: | Height: | Size: 21 KiB After Width: | Height: | Size: 21 KiB |
|
Before Width: | Height: | Size: 22 KiB After Width: | Height: | Size: 22 KiB |
|
After Width: | Height: | Size: 17 KiB |
|
After Width: | Height: | Size: 17 KiB |
|
Before Width: | Height: | Size: 36 KiB After Width: | Height: | Size: 36 KiB |
|
Before Width: | Height: | Size: 35 KiB After Width: | Height: | Size: 36 KiB |