合并来自 auto/shelf-continue-shelve-glyph-e87d 的合并请求 #44

fix(web,server): 继续阅读带上完整书对象,书库能上架,放大镜回到中线

PR-URL: #44
This commit is contained in:
hectorqin authored and cnb committed 2026-09-19 12:20:31 +08:00
commit 2bed750152
23 files changed
+948 -73

No files matched your search

+35 -3
View File
@@ -290,7 +290,7 @@ pdf / 单图没有独立目录,回落到 items。
{ "name": "三体.epub", "path": "科幻/已读/三体.epub", "type": "file",
"size": 1234567, "mtime": 1789537864820, "mode": 420,
"hidden": false, "hiddenByRule": false, "scanned": true,
"ext": "epub", "indexed": true }
"ext": "epub", "indexed": true, "shelfState": "on" }
],
"total": 1, "dirs": 0, "files": 1, "size": 1234567,
"writable": true, "name": "已读" }
@@ -299,6 +299,14 @@ pdf / 单图没有独立目录,回落到 items。
`hidden` 是「以点开头」,`hiddenByRule` 是「落在扫描器跳过的目录里」,
`scanned` 是「扫描器会把它当一本书」。
`shelfState` 是「**调用者**的书架有没有这本书」:`"on"` / `"off"`,
目录和不是书的条目是 `null`。「不在书架」这个状态是这个接口能回答、
而其它任何接口都回答不了的问题——读者把一本书从书架上拿掉之后,
书库页和书架页说的是两件不同的事,而只有前者知道原因。
它是按调用者算的(服务端用当前登录用户解析),不是条目自身的属性。
在书库页面里给每一行都画「在书架上」等于用一整列噪音盖住唯一一行例外,
所以界面只标记 `"off"`。
**被跳过的条目照样列出来。** `.trash`、`@eaDir` 这些目录是设计上不被扫描的,
在这里过滤掉就正好藏住了读者要找的那批文件——在磁盘上、但不在书架上。标记出来,
让界面能解释,而不是替读者决定不用知道。
@@ -447,13 +455,24 @@ pdf / 单图没有独立目录,回落到 items。
{ "paths": ["旧扫描"], "action": "remove" }
```
→ 同 `metadata` 的结构。`action` 取 `add` / `remove` / `hide` / `unhide`。
→ 同 `metadata` 的结构。`action` 取 `add` / `remove` / `hide` / `unhide`,
其中 `add` 与 `unhide` 是同一个写、`remove` 与 `hide` 是同一个写
(两套名字来自早期想区分「加进书架」和「别再隐藏」的打算;只有一个标志位,
所以只有一对操作。名字保留是因为删掉一个已文档化的取值是破坏性变更)。
**磁盘上什么都不会变**,这正是重点:「把这四十份扫描从书架上拿掉」和
「把这四十份扫描移进某个目录」在一行行列表里只差一个词,后果却完全不同,
所以是两个端点。`remove` 写的是 `hidden = 1` 而不是删行——书还在书库里、还在索引里,
读者放得回去;删行在书架上长得一样,却不可逆。
**`add` 必须清掉 `hidden`,不能只是「不存在才插入」。** 这是 #40 报的
「书库的书不再自动加入书架,需要手动加入」的服务端一半:被拿掉过的书**已经有**一行
`user_books`(就是 `remove` 为了可逆写下的那一行,`hidden = 1`),所以
`INSERT OR IGNORE` 会跳过它,书继续不在书架上——读者选中它、点「加入书架」、
然后什么都没发生。可逆性只存在于表结构里而不存在于端点里,就不是可逆性。
`add` 用 upsert:不存在则插入,存在则清标志(并在确实重新加入时刷新 `added_at`,
所以「最近入库」对一本没离开过的书仍然指它第一次入库的时间)。
### `GET /books/:id/file?path=<rel_path>`
流式返回这本书的**某一个**文件(二进制流,不是 JSON)。
@@ -559,7 +578,20 @@ manifest 的每个条目还带 `ref`,即这个文件对应的**资源引用**
### `GET /library/continue?limit=20`
继续阅读列表,按最近阅读时间倒序。带 `percentage` 和 `chapterTitle`。
继续阅读列表,按最近阅读时间倒序。每一项是**一整本书**(与 `GET /books` 同样的字段)
加上 `percentage`、`chapterTitle` 和 `lastReadAt`。
**它是 `Book` 的扩展,不是另一套形状。** 这里曾经返回进度行自己的命名——
`bookId` / `updatedAt`——而客户端的 `ContinueReadingItem extends Book` 读的是
`id`,卡片上的 `title` 和 `coverUrl` 恰好两种拼法同名,所以卡片画得好好的,
点下去却把 `{ id: undefined }` 交给路由,也就是 `#/book/undefined`:404,
然后被报成「这本书不在书架上了」。两个手写类型的字段名漂移,中间没有任何一个
请求里出现过正确的那个名字。现在 `id` / `addedAt` / `manualFields` 都是必需字段,
服务端再退回窄形状会直接编译不过,而不是把失败推迟到一次点击。
选书这一列走的是 `ShelfService`,不是另写一条 `SELECT`:同一个 join、同一层手动覆盖、
同一个 `addedAt`。旧查询直接 join `books`,于是读者在书库管理页改过的书名,
书架上是新的、卡片上是旧的——同一屏上同一本书有两个名字。
**它只列出书架也会列出的书。** 可见性判据与 `GET /books` 一致:`hidden = 0`
**且存在一个未丢失的文件**。少了后一半,一份被删掉、或唯一副本在没插上的移动硬盘上的书
+23
View File
@@ -146,6 +146,29 @@ id = sha256( (dc:identifier ?? '').toLowerCase() + '\0' + sha256(fileBytes) )
`SyncService.push` 整个批在**一个事务**里。断网不会留下写了一半的进度。
### 「继续阅读」是书架的一个视图,不是第二套读法
`SyncService.recentlyRead` 注入 `ShelfService`,把书行的组装交给它,自己只负责
「按最近阅读时间排序」和那两个进度字段。
理由是一次已经发生过的漂移。#40 的「书架上点击继续阅读,提示书本不在书架上」
的根因就在这条缝里,而且是两层:
- **可见性判据**曾经是 `hidden = 0` 就完了,而书架还要求「存在一个未丢失的文件」。
于是一本文件被删掉、或唯一副本在没插上的移动盘上的书,仍然被画在「继续阅读」
这一行里,点开必然 404 —— 那句话是真的,错的是那张卡片,而它正画在同一屏
「共 N 本」的下面。修法是把判据做成同一句话,而不是教客户端先查一遍:
先查一遍只会把「读者点不开的书」修成「读者看不见的书」。
- **字段形状**:端点返回进度行自己的命名(`bookId` / `updatedAt`),而客户端的
`ContinueReadingItem extends Book` 读 `id`。`title` 和 `coverUrl` 恰好两种拼法
同名,所以卡片画得好好的,点下去却把 `{ id: undefined }` 交给路由——
`#/book/undefined`、404、然后被报成「这本书不在书架上了」。
两个手写类型之间的字段名漂移,中间没有任何一个请求里出现过正确的那个名字。
一条规则:**一个屏幕上的同一本书,只能是同一个对象。** 所以书行只在
`ShelfService` 里组装一次(`toDto` / `getMany`),其它接口要书行就调它;
「最近更新」被覆盖层影响,卡片也得跟着变。
### 增量拉取
`GET /api/v1/sync?since=<上次的 serverTime>`。客户端记住上次返回的 `serverTime`,
+2 -1
View File
@@ -15,7 +15,7 @@
| `08-reader-dark.png` | 阅读页 · 夜间 | 暗色下的正文与工具栏 |
| `09-panel-txt.png` | TXT · 正文排版 | 纯文本专属的缩进/段间距/编码三行,且面板仍是半屏 |
| `10-shelf-paged.png` | 书架 · 第二页 | 分页器在封面下方,当前页是填充状态;书库入口在标题右侧 |
| `11-library.png` | 书库 | 面包屑、行、扫描忽略的徽章、以及书架/书库之间的切换 |
| `11-library.png` | 书库 | 面包屑、行、「书籍」徽章、不在书架的书带「不在书架」标记,以及书架/书库之间的切换 |
| `12-library-paged.png` | 书库 · 第二页 | 同一个分页器组件;当前页与首尾页码 |
| `13-reader-paged-scrub.png` | 阅读页 · 滑杆拖到本章第 3 页 | 分栏模式下拖动底部滑杆:读数与画面都停在本章内,不换章 |
| `14-reader-epub-image.png` | EPUB · 插图 | 章节文档里的图片真的画出来了,而不是浏览器的破图占位符 |
@@ -56,5 +56,6 @@
| EPUB: 章节里的图片真的画出来了 | ✅ | naturalWidth=4 显示 4x4 src="/api/v1/books/review-illustrated/assets?__reader-book-resource__=1&ref=OEBPS%2FI" |
| EPUB: 插图下面的正文也在 | ✅ | 正文开头="台版 转自 天使动漫
插图下面还有正文,这样图片没显示出来的时候,空出来的位置也是看得见的。" |
| 搜索框: 放大镜与输入框中线对齐 | ✅ | 放大镜中线 97.7 / 输入框中线 98.1(偏差 -0.4px,字形 16px,框高 44.203125px) |
全部通过。
+6 -1
View File
@@ -57,7 +57,7 @@
{
"name": "11-library",
"label": "书库",
"what": "面包屑、行、扫描忽略的徽章、以及书架/书库之间的切换"
"what": "面包屑、行、「书籍」徽章、不在书架的书带「不在书架」标记,以及书架/书库之间的切换"
},
{
"name": "12-library-paged",
@@ -230,6 +230,11 @@
"name": "EPUB: 插图下面的正文也在",
"ok": true,
"detail": "正文开头=\"台版 转自 天使动漫\n插图下面还有正文,这样图片没显示出来的时候,空出来的位置也是看得见的。\""
},
{
"name": "搜索框: 放大镜与输入框中线对齐",
"ok": true,
"detail": "放大镜中线 97.7 / 输入框中线 98.1(偏差 -0.4px,字形 16px,框高 44.203125px)"
}
],
"failures": []
+56
View File
@@ -118,6 +118,36 @@ em,而墨迹画在基线周围,于是墨迹贴在盒子上沿、盒子下方
包围盒来断言上面这些,也会读 `reader.css` 里的 `.icon` 规则。两边都是之前没被
任何断言覆盖的地方。
**但度量对了 ≠ 一定会对齐。第三半:盒子对了,行盒不一定在对的地方。**
#40 报的「书架页面的搜索按钮没有垂直对齐」就是这么来的,而它逃过了所有度量断言,
因为度量这次是对的。
```css
/* 旧写法:盒子居中了,墨迹没有 */
.search-glyph { position: absolute; inset-block: 0; margin-block: auto; }
```
`position: absolute` 会把 `display` 变成 `block`。规则里的盒子确实被居中了
(实测 75.98..120.19 落在一个 75.98..120.19 的输入框里,正中),但**块级盒不会
把行盒在垂直方向居中,它把行盒放在顶部**。行盒只有 1em(`line-height: 1`),
于是 1.05rem 的墨迹中线落在 y=84,而输入框中线是 y=98:高了 14px。
修法是用布局系统说这件事,而不是用 transform:
```css
.search-glyph { position: absolute; inset-block: 0; margin-block: auto;
display: grid; place-content: center; }
```
`place-content: center` 居中**行盒**,配合 §1.4 的对称度量就是居中墨迹——
和 `.icon-button` 同一套写法,所以「盒子里的字形」在这个样式表里只有一个答案。
`top: 50%; transform: translateY(-50%)` 也能修好,但更差:要在每个调用点重复、
漏掉一个就再犯一次,而且它把偏移变成 transform 的事实,输入框高度一变就要跟着改。
**量的是墨迹,不是盒子。** `run.mjs` 用 `Range` 取字形的墨迹中线断言它落在
输入框中线 ±2px 内(实测偏差 0.4px)。断言盒子是没用的:盒子在缺陷存在时**就是对的**,
这正是它活过一轮布局评审的原因。
### 1.5 字符编码
- 码位从 `U+E900` 起,连续分配,位于**私有使用区**。
@@ -304,6 +334,29 @@ URL 再回落到 `showPage(page)`。这一趟往返就是页能活过 Back、For
翻页器也照此画:`aria-current="page"` 标当前页,两端禁用而不隐藏,页码窗口是
「首、尾、以及当前页的邻居」——50 页在手机上是一排需要**读**而不是**按**的按钮。
### 3.5.1 上架与下架:书库必须说得出「这本书为什么不在书架上」
书库页存在的理由就是回答「我的书去哪了」,所以它必须能说出「这本书不在你的书架上」,
并且能把它放回去。两件事曾经都没有,那就是 #40 报的「书库的书不再自动加入书架,
需要手动加入」。它是三层各缺一块:
- **`add` 没有清掉 `hidden`。** 服务端把 `add` 写成了 `INSERT OR IGNORE`,
而被拿掉过的书**已经有**一行 `user_books`(`remove` 为了可逆写下的 `hidden = 1`),
于是 `OR IGNORE` 跳过它、书继续不在书架上。读者选中它、点「加入书架」、
什么都没发生。可逆性只存在于表结构里而不存在于端点里,就不是可逆性。
现在 `add`/`unhide` 是 upsert:不存在则插入,存在则清标志。
- **界面没有「上架」这个动作。** 行菜单和批量条都只调用 `remove`,
一个已经不在书架上的书**没有任何控件**说得出不同的话。
现在行菜单按 `shelfState` 给对应方向,批量条两个方向都给。
- **书库不显示书架状态。** `BrowseEntry.shelfState` 是新增的按调用者解析的字段
(§3.4:书库说的是「**你**的书架有没有」)。只在 `"off"` 的行上画徽章:
给一整个目录的书都画「在书架上」是用一列噪音盖住唯一一行例外,
而例外正是读者来这里找的东西。
批量条两个方向**无条件**都给,不做状态判断:一次选中可能一半在架一半不在,
读者的意图是「让这些变成一样」,而这是有方向的。把其中一个方向藏起来,
一半在架的选中就只能往一个方向推——不能去的那个方向恰好是这次报告的方向。
### 3.6 悬浮提示,不占位
状态行(`同步中…`、`离线 · 进度待同步`)是**浮在正文上方的药丸**,不是
@@ -561,6 +614,9 @@ TXT 是唯一一种**没有作者排版**的书,所以它也是唯一一处「
只改一半会得到一个更小的偏移,而不是没有偏移。
9. **动布局 → 跑 `npm run ui:review`,并且看截图。** 断言只覆盖已经量过的那些
规则;新缺陷的第一道防线是眼睛。
- 给绝对定位的字形居中时,量**墨迹**(`Range.getBoundingClientRect()`),
不要量盒子——盒子居中而墨迹不居中是 §1.4 那个缺陷的第三次出现,每次
都能活过一轮「盒子看起来在中间」的检查。
10. **加一条能一直看着的断言**,如果这条规则是能量出来的。下一个人不会看到
这次截图。
11. **加一个列表页 → 页数由服务端给,页由服务端切,页是 URL 的一部分**(§3.5)。
+11 -2
View File
@@ -528,7 +528,7 @@ export function registerLibraryRoutes(app: FastifyInstance, ctx: AppContext): vo
* deployment's own.
*/
app.get('/api/v1/library/browse', { preHandler: auth }, async (request) => {
currentUser(request);
const user = currentUser(request);
const query = request.query as Record<string, string | undefined>;
/*
* `page` and `pageSize` are read here rather than by the service, because they
@@ -536,8 +536,17 @@ export function registerLibraryRoutes(app: FastifyInstance, ctx: AppContext): vo
* is parsed at the same boundary for the same reason. A directory can hold
* thousands of entries, and the response is one page of them — see
* `BrowseListing.entries`.
*
* The caller is passed through because a row's `shelfState` describes *their*
* shelf. It is the same rule the write endpoints already follow: the screen
* answers questions about the reader, not about the disk alone.
*/
return ctx.browse.list(query.path ?? '', numberParam(query.page, 1), numberParam(query.pageSize, 200));
return ctx.browse.list(
query.path ?? '',
numberParam(query.page, 1),
numberParam(query.pageSize, 200),
user.id,
);
});
app.post('/api/v1/library/browse/move', { preHandler: auth }, async (request) => {
+1 -1
View File
@@ -47,7 +47,7 @@ async function main(): Promise<void> {
});
ctx.users = new UserService(db, config);
ctx.shelf = new ShelfService(db);
ctx.sync = new SyncService(db);
ctx.sync = new SyncService(db, ctx.shelf);
ctx.tts = new TtsService(config);
ctx.browse = new BrowseService(db, config, ctx.shelf);
ctx.uploads = new UploadService(db, config, ctx.browse, ctx.scanner);
+111 -21
View File
@@ -60,6 +60,24 @@ export interface BrowseEntry {
ext: string;
/** True when this path is what the index actually holds for that book. */
indexed: boolean;
/**
* Whether the *caller's* shelf holds the book at this path.
*
* `null` for a path that is not an indexed book at all (a folder, a stray
* `.nfo`, an archive's page image), because "not on your shelf" would be a
* category error for a row that is not a book.
*
* This exists because the library screen could describe what is on the *disk*
* and not what is on the *shelf*, and the two are different sets the moment a
* reader takes a book off their shelf. Without it the file manager offered only
* "下架" — an action for a book that is on the shelf — with no way to see that a
* book was off it, and no way to put one back: the reader's only route to
* restoring a book was to guess, select it, and hope.
*
* It is per-caller rather than a property of the entry, so it is filled in by
* `list` when the caller supplies a user id.
*/
shelfState: 'on' | 'off' | null;
}
export interface BrowseListing {
@@ -150,11 +168,11 @@ export class BrowseService {
// ---- reads ----
list(relPathInput: string, page = 1, pageSize = BROWSE_PAGE_SIZE): Promise<BrowseListing> {
return this.listSync(relPathInput, page, pageSize);
list(relPathInput: string, page = 1, pageSize = BROWSE_PAGE_SIZE, userId?: string): Promise<BrowseListing> {
return this.listSync(relPathInput, page, pageSize, userId);
}
private async listSync(relPathInput: string, page: number, pageSize: number): Promise<BrowseListing> {
private async listSync(relPathInput: string, page: number, pageSize: number, userId?: string): Promise<BrowseListing> {
const relPath = relPathInput === '' ? '' : assertSafeRel(relPathInput);
const abs = resolveInside(this.config.booksDir, relPath);
const info = await stat(abs).catch(() => null);
@@ -186,9 +204,54 @@ export class BrowseService {
scanned: entry.isFile() && this.isBookFile(entry.name),
ext: extensionOf(entry.name),
indexed: Boolean(this.fileRow(childRel)),
// Filled in by the pass below, which is the only place that knows the
// caller: `listSync` does not take a user id when it is called from
// `upload`'s own destination probe.
shelfState: null,
});
}
/*
* The shelf flag, resolved in one query for the whole page.
*
* Asked per row it would be one query per file in a folder of four thousand
* scans — on the read path of the screen that exists because reading the disk
* is slow. The map is built from a single `IN`, and a path whose book has no
* `user_books` row (or has `hidden = 1`) is `'off'`: the shelf's own predicate
* is `hidden = 0`, and being off the shelf and being hidden are the same state
* seen from opposite ends.
*/
if (userId) {
const bookIdsByPath = new Map<string, string>();
const indexedPaths = entries.filter((entry) => entry.indexed).map((entry) => entry.path);
if (indexedPaths.length > 0) {
const fileRows = this.db.all<{ rel_path: string; book_id: string }>(
`SELECT rel_path, book_id FROM book_files WHERE rel_path IN (${indexedPaths.map(() => '?').join(',')})`,
...indexedPaths,
);
for (const row of fileRows) bookIdsByPath.set(row.rel_path, row.book_id);
}
const onShelf = new Set<string>();
const bookIds = [...new Set(bookIdsByPath.values())];
if (bookIds.length > 0) {
const rows = this.db.all<{ book_id: string }>(
`SELECT book_id FROM user_books
WHERE user_id = ? AND hidden = 0 AND book_id IN (${bookIds.map(() => '?').join(',')})`,
userId, ...bookIds,
);
for (const row of rows) onShelf.add(row.book_id);
}
for (const entry of entries) {
const bookId = bookIdsByPath.get(entry.path);
if (bookId === undefined) continue;
// A folder's own `shelfState` stays null: the flag describes the row the
// reader can tap, and a folder is not a book even when the books inside it
// are on the shelf. Selection acts on the path, and `batchShelf` already
// fans a folder out to the books under it.
if (entry.type !== 'dir') entry.shelfState = onShelf.has(bookId) ? 'on' : 'off';
}
}
/*
* The counts describe the whole directory; `entries` is one page of it.
*
@@ -465,32 +528,59 @@ export class BrowseService {
return rows.map((row) => row.book_id);
}
/**
* The two directions, with the aliases folded in.
*
* `add`/`unhide` and `remove`/`hide` are the same two writes under two names.
* The aliases date from when the API was going to distinguish "put it on my
* shelf" from "stop hiding it"; there is only one flag, so there is only one
* pair of operations, and a second name for each is kept because dropping a
* documented value is a breaking change. They are normalised here, at the one
* place that writes, so the rest of the method reasons about two cases.
*/
private setShelfState(bookId: string, userId: string, action: 'add' | 'remove' | 'hide' | 'unhide', now: number): void {
if (action === 'add') {
if (action === 'add' || action === 'unhide') {
/*
* `add` (and its synonym `hide`... see below) both *create* the row if it is
* absent and *clear the flag* if it is present.
*
* The upsert is the whole fix for the reported "书库的书不再自动加入书架,
* 需要手动加入". `add` used to be a bare `INSERT OR IGNORE`, which reads as
* "add it if it is not already there" and behaves as "do nothing" in the one
* case that matters: a book the reader had previously taken off the shelf
* already *has* a `user_books` row — the row `remove` wrote with `hidden = 1`
* precisely so the removal would be reversible — so `OR IGNORE` skipped it
* and the book stayed hidden. Selecting a book, choosing "加入书架", and
* watching it not appear is the exact shape of "需要手动加入,但加了也没用".
*
* `added_at` is refreshed only when the row is actually being re-added, so
* "最近入库" keeps meaning "when I first got this book" for a book that never
* left, and means "now" for one that did.
*/
this.db.run(
'INSERT OR IGNORE INTO user_books (user_id, book_id, added_at) VALUES (?,?,?)',
`INSERT INTO user_books (user_id, book_id, added_at, hidden) VALUES (?,?,?,0)
ON CONFLICT(user_id, book_id) DO UPDATE SET hidden = 0,
added_at = CASE WHEN user_books.hidden = 1 THEN excluded.added_at ELSE user_books.added_at END`,
userId, bookId, now,
);
return;
}
if (action === 'remove') {
// `hidden` rather than a delete: the book stays in the library and stays
// indexed, so the reader can put it back. Deleting the row would look
// identical on the shelf and be irreversible.
this.db.run('UPDATE user_books SET hidden = 1 WHERE user_id = ? AND book_id = ?', userId, bookId);
this.db.run(
'INSERT OR IGNORE INTO user_books (user_id, book_id, added_at, hidden) VALUES (?,?,?,1)',
userId, bookId, now,
);
return;
}
// unhide: an entry that was never added is left alone rather than created —
// "put it back" cannot resurrect a shelf entry that never existed.
// `remove` keeps the row: `hidden` rather than a delete, so the book stays in
// the library and stays indexed and the reader can put it back. Deleting the
// row would look identical on the shelf and be irreversible.
this.db.run('UPDATE user_books SET hidden = 1 WHERE user_id = ? AND book_id = ?', userId, bookId);
/*
* And it *creates* the row when there is none.
*
* A book the reader never added, then took off a shelf it was never on, has no
* row; without this insert the "off the shelf" state would have nowhere to live
* and the book would come back on the next page load — the shelf's predicate is
* `hidden = 0`, and a missing row is not hidden.
*/
this.db.run(
`UPDATE user_books SET hidden = 0 WHERE user_id = ? AND book_id = ?`,
userId, bookId,
'INSERT OR IGNORE INTO user_books (user_id, book_id, added_at, hidden) VALUES (?,?,?,1)',
userId, bookId, now,
);
void action;
}
/**
+30
View File
@@ -157,6 +157,36 @@ export class ShelfService {
return this.toDto(row);
}
/**
* Several books by id, in one round trip, as a map.
*
* The `user_books` join is applied for the same reason `get` applies it: an id
* the caller holds is not proof that *this* user may see the book, and a helper
* that skipped the check would be the one place in the service where an id alone
* was enough.
*
* Missing or invisible ids are simply absent from the map rather than an error:
* the callers are all "join this list of ids onto their books", and a book that
* vanished between the two queries is one fewer row to draw, not a failed
* request. `get` still throws, because a *single* book that cannot be found is
* exactly the case where the caller needs to know.
*
* `addedAt` is read from the join, so the DTOs here are identical to the ones
* `list` produces — which is what lets the continue-reading strip and the shelf
* grid be the same `Book` on the client.
*/
getMany(userId: string, bookIds: string[]): Map<string, BookDto> {
const unique = [...new Set(bookIds)].filter((id) => id !== '');
if (unique.length === 0) return new Map();
const rows = this.db.all<BookRow>(
`SELECT ${BOOK_COLUMNS.split(',').map((c) => `b.${c.trim()}`).join(', ')}, ub.added_at
FROM books b JOIN user_books ub ON ub.book_id = b.id
WHERE ub.user_id = ? AND ub.hidden = 0 AND b.id IN (${unique.map(() => '?').join(',')})`,
userId, ...unique,
);
return new Map(rows.map((row) => [row.id, this.toDto(row)]));
}
/**
* Applies the manual override layer, which sits above embedded metadata and
* above anything a provider filled in.
+74 -21
View File
@@ -1,6 +1,7 @@
import { randomUUID } from 'node:crypto';
import type { Db } from '../db/index.ts';
import { badRequest } from '../lib/errors.ts';
import type { BookDto, ShelfService } from './shelf.ts';
/**
* Sync payload is deliberately small and append-only where possible (§4): the
@@ -39,8 +40,41 @@ export interface SyncPushRequest {
notes?: NoteRecord[];
}
/**
* One row of the "继续阅读" strip: a whole book plus where the reader left off.
*
* It is a `BookDto` with the progress flattened onto it, and that is the *contract*
* rather than a convenience. The client's `ContinueReadingItem extends Book`, so
* the card is drawn from the same columns as the shelf's own grid and opens through
* the same `openBook(book)` path — which reads `book.id`.
*
* It used to answer the progress row's own shape instead: `bookId` and `updatedAt`
* where the card wanted `id` and `lastReadAt`. Nothing failed loudly — the card
* rendered, because `title` and `coverUrl` happened to be spelled the same — and
* tapping it called `openBook` with `{ id: undefined }`, i.e. `#/book/undefined`,
* which 404s and is reported as "这本书不在书架上了". A field-name drift between
* two hand-written types, with no request that ever contains the right name.
*/
export type ContinueReadingItem = BookDto & {
percentage: number;
chapterTitle: string;
/** When the reader last turned a page here; `null` if progress was never pushed. */
lastReadAt: number | null;
};
export class SyncService {
constructor(private readonly db: Db) {}
constructor(
private readonly db: Db,
/**
* The shelf's own DTO builder, injected rather than duplicated.
*
* This endpoint's whole job is "the shelf, ordered by recency of reading", so
* a book it returns has to be byte-for-byte what `GET /books` would return for
* the same row — overrides applied, `addedAt` resolved, cover URL built. A
* second `SELECT` with its own column list is how the original drift happened.
*/
private readonly shelf: ShelfService,
) {}
/**
* Merges a client batch. Conflicts resolve by last-writer-wins on updatedAt,
@@ -179,11 +213,17 @@ export class SyncService {
};
}
/** Continue-reading list: newest progress first, joined with book titles. */
recentlyRead(userId: string, limit = 20): Array<{
bookId: string; title: string; author: string; percentage: number;
chapterTitle: string; updatedAt: number; coverUrl: string | null;
}> {
/**
* Continue-reading list: newest progress first, as whole books.
*
* The ordering key is `reading_progress.updated_at` — "what did I read last",
* which is the question the strip asks. Everything else about the row comes from
* `ShelfService`, because the answer to "which book is this" has to be the same
* answer the shelf gives: identical overrides, identical `addedAt`, identical
* cover URL. See `ContinueReadingItem` for why that is the contract rather than
* duplication.
*/
recentlyRead(userId: string, limit = 20): ContinueReadingItem[] {
/*
* The visibility rule is `ShelfService.list`'s rule, character for character.
*
@@ -204,27 +244,40 @@ export class SyncService {
* keeps the predicate readable as "what the shelf would show".
*/
const rows = this.db.all<{
book_id: string; title: string; author: string; percentage: number;
chapter_title: string; updated_at: number; cover_path: string | null;
book_id: string; percentage: number; chapter_title: string; updated_at: number;
}>(
`SELECT p.book_id, b.title, b.author, p.percentage, p.chapter_title, p.updated_at, b.cover_path
`SELECT p.book_id, p.percentage, p.chapter_title, p.updated_at
FROM reading_progress p
JOIN books b ON b.id = p.book_id
JOIN user_books ub ON ub.book_id = b.id AND ub.user_id = p.user_id
JOIN user_books ub ON ub.book_id = p.book_id AND ub.user_id = p.user_id
WHERE p.user_id = ? AND ub.hidden = 0
AND EXISTS (SELECT 1 FROM book_files f WHERE f.book_id = b.id AND f.missing = 0)
AND EXISTS (SELECT 1 FROM book_files f WHERE f.book_id = p.book_id AND f.missing = 0)
ORDER BY p.updated_at DESC LIMIT ?`,
userId, limit,
);
return rows.map((row) => ({
bookId: row.book_id,
title: row.title,
author: row.author,
percentage: row.percentage,
chapterTitle: row.chapter_title,
updatedAt: row.updated_at,
coverUrl: row.cover_path ? `/api/v1/books/${row.book_id}/cover` : null,
}));
/*
* Read through `shelf.getMany` rather than re-selecting the book columns here.
*
* The old query joined `books` for `title`, `author` and `cover_path`, which
* looks equivalent and is not: it skipped the manual-override layer, so a book
* renamed in the library manager was still offered under its embedded title —
* and it could not produce `manualFields` or `addedAt` at all, which is why the
* client's type had to be widened to a `Book` while the response stayed narrow.
*
* A row that disappears between the two statements (a rescan mid-request) is
* dropped rather than rendered, for the same reason the `EXISTS` is above:
* offering a book the reader cannot open is the defect this endpoint had.
*/
const byId = this.shelf.getMany(userId, rows.map((row) => row.book_id));
return rows.flatMap((row) => {
const book = byId.get(row.book_id);
if (!book) return [];
return [{
...book,
percentage: row.percentage,
chapterTitle: row.chapter_title,
lastReadAt: row.updated_at,
}];
});
}
/** Client-generated note ids are accepted, but this keeps the API ergonomic. */
+83 -1
View File
@@ -77,7 +77,7 @@ before(async () => {
ctx.scanner = new Scanner(db, config, { info: () => {}, warn: () => {} });
ctx.users = new UserService(db, config);
ctx.shelf = new ShelfService(db);
ctx.sync = new SyncService(db);
ctx.sync = new SyncService(db, ctx.shelf);
ctx.tts = new TtsService(config);
ctx.browse = new BrowseService(db, config, ctx.shelf);
app = buildApp(ctx);
@@ -460,6 +460,88 @@ test('a book whose file is gone is not offered as "continue reading"', async ()
assert.equal((shelf.json() as { total: number }).total, 0);
});
test('a continue-reading item is the same book the shelf serves', async () => {
/*
* The card and the shelf have to agree about the *shape* of a book, not just
* about which books are visible.
*
* `/library/continue` used to answer the progress row's own naming —
* `{ bookId, title, author, percentage, chapterTitle, updatedAt, coverUrl }` —
* while the client's `ContinueReadingItem extends Book` and the tap path reads
* `book.id`. Nothing failed loudly: `title` and `coverUrl` are spelled the same
* in both shapes, so the card drew correctly, and tapping it routed to
* `#/book/undefined`, which 404s and is reported as "这本书不在书架上了".
*
* Asserting field-by-field against `GET /books` is the point: the two endpoints
* have to return the *same* object for the same book, or the strip is drawing a
* second, unofficial kind of book.
*/
const session = await createUser('continueuser2');
await writeFile(join(booksDir, '接着读.epub'), await makeEpub({
id: 'urn:isbn:9780000000043', title: '接着读', creator: '某作者',
}));
await ctx.scanner.scan();
const listing = await app.inject({ method: 'GET', url: '/api/v1/books?search=%E6%8E%A5%E7%9D%80%E8%AF%BB', headers: auth(session.token) });
const shelfBook = (listing.json() as { items: Array<Record<string, unknown>> }).items[0]!;
ctx.db.run(
`INSERT INTO reading_progress (user_id, book_id, locator, percentage, chapter_title, device, updated_at)
VALUES (?,?,?,?,?,?,?)`,
session.userId, shelfBook.id, 'r1:0:c0', 0.4, '第二章', 'test', Date.now(),
);
const res = await app.inject({ method: 'GET', url: '/api/v1/library/continue', headers: auth(session.token) });
const item = (res.json() as { items: Array<Record<string, unknown>> }).items[0]!;
// `id` is the field the tap path reads, and the one whose absence produced the
// "不在书架上了" report. Checked first because it is the actual defect.
assert.equal(item.id, shelfBook.id, 'the card must carry `id`, not `bookId`');
assert.equal(item.bookId, undefined, 'the progress-row spelling must not be what the client sees');
for (const field of ['title', 'author', 'format', 'coverUrl', 'addedAt', 'manualFields']) {
assert.deepEqual(item[field], shelfBook[field], `\`${field}\` must match GET /books`);
}
assert.equal(item.percentage, 0.4);
assert.equal(item.chapterTitle, '第二章');
assert.equal(typeof item.lastReadAt, 'number');
// And the id it carries is directly openable — the check the tap performs.
const opened = await app.inject({ method: 'GET', url: `/api/v1/books/${String(item.id)}`, headers: auth(session.token) });
assert.equal(opened.statusCode, 200, 'the id a continue card carries must resolve to a book');
});
test('a manual metadata override reaches the continue-reading card', async () => {
/*
* The second half of the same defect, and the reason the fix reads through
* `ShelfService` rather than re-selecting columns.
*
* The old query joined `books` directly, which meant it answered the *embedded*
* title. A reader who fixed a garbled title in the library manager therefore saw
* the corrected name on the shelf and the garbled one on the card above it — the
* same book, two names, one screen.
*/
const session = await createUser('overrideuser');
await writeFile(join(booksDir, '待改名.epub'), await makeEpub({
id: 'urn:isbn:9780000000044', title: '待改名', creator: '某人',
}));
await ctx.scanner.scan();
const listing = await app.inject({ method: 'GET', url: '/api/v1/books?search=%E5%BE%85%E6%94%B9%E5%90%8D', headers: auth(session.token) });
const book = (listing.json() as { items: Array<{ id: string }> }).items[0]!;
await app.inject({
method: 'PATCH', url: `/api/v1/books/${book.id}/metadata`,
headers: auth(session.token), payload: { title: '改好的名字' },
});
ctx.db.run(
`INSERT INTO reading_progress (user_id, book_id, locator, percentage, chapter_title, device, updated_at)
VALUES (?,?,?,?,?,?,?)`,
session.userId, book.id, 'r1:0:c0', 0.1, '第一章', 'test', Date.now(),
);
const res = await app.inject({ method: 'GET', url: '/api/v1/library/continue', headers: auth(session.token) });
const item = (res.json() as { items: Array<{ title: string; manualFields: string[] }> }).items[0]!;
assert.equal(item.title, '改好的名字');
assert.ok(item.manualFields.includes('title'));
});
test('an ambiguous "title - author" file name is kept whole rather than guessed at', async () => {
// `长篇 - 某作者` has no marker to say which side is which, so the conservative
// rule keeps the whole string as the title. Losing an author costs the reader one
+1 -1
View File
@@ -87,7 +87,7 @@ before(async () => {
ctx.scanner = new Scanner(db, config, { info: () => {}, warn: () => {} });
ctx.users = new UserService(db, config);
ctx.shelf = new ShelfService(db);
ctx.sync = new SyncService(db);
ctx.sync = new SyncService(db, ctx.shelf);
ctx.tts = new TtsService(config);
ctx.browse = new BrowseService(db, config, ctx.shelf);
app = buildApp(ctx);
+1 -1
View File
@@ -212,7 +212,7 @@ before(async () => {
ctx.scanner = new Scanner(db, config, { info: () => {}, warn: () => {} });
ctx.users = new UserService(db, config);
ctx.shelf = new ShelfService(db);
ctx.sync = new SyncService(db);
ctx.sync = new SyncService(db, ctx.shelf);
app = buildApp(ctx);
await app.ready();
+1 -1
View File
@@ -104,7 +104,7 @@ async function buildInstance(): Promise<void> {
ctx.scanner = new Scanner(db, config, { info: () => {}, warn: () => {} });
ctx.users = new UserService(db, config);
ctx.shelf = new ShelfService(db);
ctx.sync = new SyncService(db);
ctx.sync = new SyncService(db, ctx.shelf);
ctx.tts = new TtsService(config);
ctx.browse = new BrowseService(db, config, ctx.shelf);
app = buildApp(ctx);
+75 -1
View File
@@ -193,7 +193,7 @@ before(async () => {
ctx.scanner = new Scanner(db, config, { info: () => {}, warn: () => {} });
ctx.users = new UserService(db, config);
ctx.shelf = new ShelfService(db);
ctx.sync = new SyncService(db);
ctx.sync = new SyncService(db, ctx.shelf);
ctx.tts = new TtsService(config);
ctx.browse = new BrowseService(db, config, ctx.shelf);
ctx.uploads = new UploadService(db, config, ctx.browse, ctx.scanner);
@@ -677,6 +677,80 @@ describe('batch management', () => {
assert.equal(back!.n, before!.n);
});
test('a row reports whether its book is on the caller\'s shelf', async () => {
/*
* The file manager could describe the disk and not the shelf, and the two
* diverge the moment a reader takes a book off their shelf. Without this flag
* the screen offers "从书架拿掉" for every book — including the ones already off
* it — so a reader looking for the reason a book is missing from their shelf
* found the one screen that could explain it unable to say anything.
*/
const listing = async (): Promise<Array<{ name: string; shelfState: string | null }>> =>
(await app.inject({
method: 'GET', url: `/api/v1/library/browse?path=${encodeURIComponent(batchRoot)}`, headers: auth(),
})).json<{ entries: Array<{ name: string; shelfState: string | null }> }>().entries;
// A fresh scan puts every book on every user's shelf, so both start `on`.
assert.deepEqual((await listing()).map((entry) => entry.shelfState), ['on', 'on']);
const removed = await app.inject({
method: 'POST', url: '/api/v1/library/browse/shelf', headers: auth(),
payload: { paths: [`${batchRoot}/卷一.epub`], action: 'remove' },
});
assert.equal(removed.statusCode, 200, removed.body);
const after = new Map((await listing()).map((entry) => [entry.name, entry.shelfState]));
assert.equal(after.get('卷一.epub'), 'off', 'a book taken off the shelf must say so');
assert.equal(after.get('卷二.epub'), 'on');
});
test('"加入书架" puts back a book that was taken off the shelf', async () => {
/*
* The reported defect, at the layer that caused it.
*
* `add` was a bare `INSERT OR IGNORE`, and a book taken off the shelf already
* *had* a `user_books` row — the one `remove` writes with `hidden = 1`, which is
* exactly what makes a removal reversible. So `OR IGNORE` skipped the insert and
* the book stayed hidden: the reader selected it, chose "加入书架", and nothing
* happened. Reversibility that exists in the schema but not in the endpoint is
* not reversibility.
*/
const total = async (search: string): Promise<number> =>
(await app.inject({ method: 'GET', url: `/api/v1/books?search=${encodeURIComponent(search)}`, headers: auth() }))
.json<{ total: number }>().total;
assert.equal(await total('合集第一卷'), 0, 'precondition: the book is off the shelf');
const added = await app.inject({
method: 'POST', url: '/api/v1/library/browse/shelf', headers: auth(),
payload: { paths: [`${batchRoot}/卷一.epub`], action: 'add' },
});
assert.equal(added.statusCode, 200, added.body);
assert.equal((added.json() as { applied: number }).applied, 1);
assert.equal(await total('合集第一卷'), 1, '"add" must clear the hidden flag, not only create a missing row');
/*
* And a folder is one action, not forty.
*
* "Hide these forty scans from my shelf" is the batch the endpoint exists for,
* and its inverse has to be the same shape — otherwise putting a series back is
* a per-volume chore, which is the manual work the auto-add rule exists to
* avoid in the first place.
*/
const off = await app.inject({
method: 'POST', url: '/api/v1/library/browse/shelf', headers: auth(),
payload: { paths: [batchRoot], action: 'remove' },
});
assert.equal(off.statusCode, 200, off.body);
assert.equal((off.json() as { applied: number }).applied, 2);
assert.equal(await total('合集第二卷'), 0);
const restored = await app.inject({
method: 'POST', url: '/api/v1/library/browse/shelf', headers: auth(),
payload: { paths: [batchRoot], action: 'unhide' },
});
assert.equal(restored.statusCode, 200, restored.body);
assert.equal(await total('合集第二卷'), 1, 'the alias must be the same write as `add`');
});
test('an unknown shelf action is refused', async () => {
const res = await app.inject({
method: 'POST',
+33 -2
View File
@@ -282,9 +282,40 @@ export class ReaderApi {
}
}
/**
* The shelf's "继续阅读" strip: whole books, newest reading first.
*
* ## The two normalisations, and why they live here
*
* This endpoint is the one place where the server and the client disagreed about
* the *name* of a field, and the disagreement was invisible: the card is typed as
* `ContinueReadingItem extends Book` and reads `book.id`, while the server answered
* the progress row's `bookId`. `title` and `coverUrl` happen to be spelled the same
* in both shapes, so the card drew correctly, and the tap carried `id: undefined` —
* `#/book/undefined`, a 404, and "这本书不在书架上了" on a book that was plainly on
* the shelf behind the toast.
*
* The server is fixed, so this is not the repair; it is what stops the repair from
* being *deployed in halves*. The H5 bundle and the API version independently: an
* Android WebView can hold a cached client against a newer server, and a browser
* can hold a cached client against an older one. Normalising at the boundary means
* either combination works, rather than the six weeks in which one does not.
*
* A row with no id at all is **dropped**, not drawn: a card whose tap goes nowhere
* is precisely the reported symptom, and losing one shortcut out of twenty is a
* smaller failure than growing a control that lies about being one.
*/
async continueReading(limit = 20, options: RequestOptions = {}): Promise<ContinueReadingItem[]> {
const result = await this.get<{ items: ContinueReadingItem[] }>(`/api/v1/library/continue?limit=${limit}`, options);
return result.items;
const result = await this.get<{ items: Array<ContinueReadingItem & { bookId?: string }> }>(
`/api/v1/library/continue?limit=${limit}`, options,
);
return result.items.flatMap((item) => {
// `bookId` is the old spelling of `id`; `updatedAt` the old spelling of
// `lastReadAt`. See the type for the full history.
const id = item.id ?? item.bookId;
if (!id) return [];
return [{ ...item, id, lastReadAt: item.lastReadAt ?? item.updatedAt ?? null }];
});
}
// ---- library file manager ----
+27
View File
@@ -191,9 +191,28 @@ export interface Facets {
formats: string[];
}
/**
* One row of the shelf's "继续阅读" strip.
*
* A whole `Book` with the progress flattened onto it — not a book-with-progress
* *pair*, and not the progress record's own shape.
*
* That distinction is the bug this shape fixes. The server used to answer
* `{ bookId, title, author, percentage, chapterTitle, updatedAt, coverUrl }`, i.e.
* the *progress* row's naming, while this type asked for `Book`'s. Nothing failed
* loudly: `title` and `coverUrl` are spelled the same in both, so the card drew,
* and tapping it handed `openBook` an object with `id: undefined` — which routes to
* `#/book/undefined`, 404s, and gets reported as "这本书不在书架上了". Two
* hand-written types drifting on the two fields the tap path actually reads.
*
* Declaring it as an intersection is what makes the drift impossible to reintroduce
* quietly: `id`, `addedAt` and `manualFields` are now *required* here, so a server
* that answers the narrow shape fails this type instead of failing a tap.
*/
export interface ContinueReadingItem extends Book {
percentage: number;
chapterTitle: string;
/** When the reader last turned a page here; `null` if progress was never pushed. */
lastReadAt: number | null;
}
@@ -216,6 +235,14 @@ export interface BrowseEntry {
hiddenByRule: boolean;
/** The scanner indexes this path as a book of its own. */
scanned: boolean;
/**
* Whether the caller's shelf holds the book at this path.
*
* `null` when the path is not an indexed book at all — a folder, a stray `.nfo`,
* a page image inside an archive. The value is per-caller, because it describes
* *their* shelf: the server resolves it against the authenticated user.
*/
shelfState: 'on' | 'off' | null;
ext: string;
/** The index currently holds a row for this exact path. */
indexed: boolean;
+50 -7
View File
@@ -1000,20 +1000,48 @@ book-content {
/*
* The magnifier inside the search field.
*
* Centred on the field's *centre*: `inset-block: 0` with `margin-block: auto` resolves to
* the middle of the containing block, which is where the field's own text is centred too.
* The glyph is inline and the box is a line box, so "centre the box" and "centre the
* glyph" are the same instruction here.
* ## The bug this rule exists to remove
*
* The old rule anchored only the inline axis and left the block position to the field's
* top edge, which put the glyph's box where the field's centre was and the ink half an em
* above that — the same defect as the buttons, in a different container.
* The previous version said `inset-block: 0; margin-block: auto` and the comment above
* it claimed "the glyph is inline and the box is a line box, so centring the box and
* centring the glyph are the same instruction". That premise is false, and it is false
* for a reason that has nothing to do with this glyph: **`position: absolute` forces
* `display: block`**. The `<span>` was therefore a *block* box, not a line box, and a
* block box does not centre its line box vertically — it puts it at the top.
*
* So the rule did exactly what it said (the box was centred: measured 75.98..120.19 in
* a field of 75.98..120.19, dead centre) and produced exactly the wrong picture: the
* line box is 1em tall and sits at the box's top, so a 1.05rem glyph's ink landed with
* its centre at y=84 while the field's centre was y=98. Fourteen pixels high — the
* reported "搜索按钮没有垂直对齐".
*
* ## Why `grid` and not a transform
*
* `place-content: center` centres the *line box* inside the box, which — with the icon
* font's symmetric ascent/descent (see `tools/icons/build.mjs`) — centres the ink too.
* That is the same idiom `.icon-button` already uses, so "a glyph in a box" has one
* answer in this stylesheet rather than two.
*
* The alternative, `top: 50%; transform: translateY(-50%)`, also works and is worse: it
* has to be repeated at every call site and can be omitted at one, it makes the offset a
* fact about a *transform* rather than about the layout, and it interacts badly with any
* future padding (the percentage resolves against the padding box while the ink does
* not). Saying it with the layout system means the field can change height and the
* magnifier follows without this rule being touched.
*/
.search-glyph {
position: absolute;
inset-inline-start: 0.7rem;
inset-block: 0;
margin-block: auto;
/*
* The box is only as wide as the glyph and as tall as the field. `grid` +
* `place-content` is what turns "the box is centred" into "the ink is centred" — see
* `.icon-button` for the same construction.
*/
display: grid;
place-content: center;
width: 1.2rem;
font-size: 1.05rem;
color: var(--reader-muted);
pointer-events: none;
@@ -2976,6 +3004,21 @@ book-content {
color: var(--reader-accent);
}
/*
* "不在书架" — the exception mark.
*
* Filled rather than outlined, and in the accent colour, because it is the one
* badge in the row that means "you are looking at the reason this book is missing
* from your shelf". The `warn` style is an outline with accent text and is the
* other explainer badge; this one is solid so the two are distinguishable at a
* glance in a list where both can appear.
*/
.manager-badge.off {
border-color: transparent;
background: color-mix(in srgb, var(--reader-accent) 16%, transparent);
color: var(--reader-accent);
}
.manager-more {
width: 2rem;
height: 2rem;
+75 -3
View File
@@ -66,6 +66,25 @@ type Dialog =
*/
const PAGE_SIZE = 200;
/**
* What each shelf action is called, in the reader's words.
*
* `add` and `unhide` are two repairs for the same symptom and are *not* given the
* same label, because the server treats them as different operations: `unhide`
* clears the flag on a book that was deliberately taken off the shelf, while `add`
* creates the shelf row a book never had. Collapsing the labels would hide that
* the two are different writes, and the day one of them turns out to be wrong the
* report would say "放回书架 didn't work" for both.
*/
const SHELF_ACTION_LABELS: Record<ShelfAction, string> = {
add: '加入书架',
remove: '下架',
// Aliases of the two above — the server accepts both spellings and folds them
// into one write, so the labels are the same words rather than a second verb.
hide: '下架',
unhide: '加入书架',
};
interface ManagerState {
path: string;
/**
@@ -386,6 +405,7 @@ export class ManagerScreen {
onRename={(path, name) => void this.promptRename(path, name)}
onMetadata={() => void this.promptBatchMetadata()}
onShelve={() => void this.batchShelf('remove')}
onUnshelve={() => void this.batchShelf('unhide')}
onMove={() => void this.promptMove()}
onDelete={() => void this.confirmDelete()}
/>
@@ -453,10 +473,24 @@ export class ManagerScreen {
this.toast('书库是只读挂载,无法修改');
return;
}
/*
* The shelf entry names the action that applies to *this* book.
*
* `shelfState` is `'on'`, `'off'` or `null`, and only one of the three means
* anything here: `null` is a folder or a non-book, which has no shelf action at
* all. Offering "从书架拿掉" for a book that is already off the shelf was the
* visible half of the reported defect — the menu answered the same word whatever
* the book's state, so a reader looking at a book *missing* from their shelf had
* no way to put it back.
*/
const action = await this.pick(`「${entry.name}」`, [
{ value: 'select', label: '选择' },
{ value: 'metadata', label: '改资料' },
{ value: 'shelve', label: '从书架拿掉' },
...(entry.shelfState === 'on'
? [{ value: 'shelve:remove', label: '从书架拿掉' }]
: entry.shelfState === 'off'
? [{ value: 'shelve:add', label: '放回书架' }]
: []),
{ value: 'rename', label: '重命名' },
{ value: 'move', label: '移动到…' },
{ value: 'delete', label: '删除' },
@@ -470,7 +504,8 @@ export class ManagerScreen {
// include a batch ticked earlier and now off screen.
this.select([entry.path]);
if (action === 'metadata') return this.promptBatchMetadata();
if (action === 'shelve') return this.batchShelf('remove');
if (action === 'shelve:add') return this.batchShelf('add');
if (action === 'shelve:remove') return this.batchShelf('remove');
if (action === 'rename') return this.promptRename(entry.path, entry.name);
if (action === 'move') return this.promptMove();
if (action === 'delete') return this.confirmDelete();
@@ -537,10 +572,27 @@ export class ManagerScreen {
);
}
/**
* Puts books on the caller's shelf, or takes them off it.
*
* All four of the server's actions are reachable now, which is the fix for the
* half of the reported defect the menu could not express: the screen could only
* ever call `remove`, so a book that was off the shelf stayed off it.
*
* Only `remove` confirms, and only `remove` needs to: it is the one that takes
* something away. Adding is the reader saying "show me this", and a dialog in
* front of it would be a dialog in front of the repair.
*
* `add` and `unhide` are kept distinct rather than collapsed into one "put it
* back", because they are different repairs for the same symptom: `unhide`
* clears the flag on a book that was deliberately taken off, and `add` creates
* the shelf row a book never had. The server refuses to guess between them (see
* `setShelfState`), so the client must say which one it means.
*/
private async batchShelf(action: ShelfAction): Promise<void> {
const paths = this.selectedList();
if (paths.length === 0) return;
const label = action === 'remove' ? '下架' : '上架';
const label = SHELF_ACTION_LABELS[action];
if (action === 'remove') {
const ok = await this.confirm(
`${label} ${paths.length} 项?`,
@@ -903,6 +955,15 @@ function Row({ entry, selecting, selected, onActivate, onToggle, onMenu }: RowPr
if (entry.hiddenByRule) meta.push(<span className="manager-badge warn">扫描忽略</span>);
else if (entry.scanned) meta.push(<span className="manager-badge">书籍</span>);
if (entry.hidden && !entry.hiddenByRule) meta.push(<span className="manager-badge">隐藏</span>);
/*
* The shelf badge, and only when the answer is "off".
*
* "在书架上" on every row of a folder of books that are all on the shelf is a
* column of noise that hides the one row that is not. The reader is here to find
* the books that are *missing* from the shelf — that is the question this screen
* exists to answer — so the mark is on the exception.
*/
if (entry.shelfState === 'off') meta.push(<span className="manager-badge off">不在书架</span>);
return (
<div
@@ -981,6 +1042,7 @@ interface SelectionBarProps {
onRename(path: string, name: string): void;
onMetadata(): void;
onShelve(): void;
onUnshelve(): void;
onMove(): void;
onDelete(): void;
}
@@ -997,7 +1059,17 @@ function SelectionBar(props: SelectionBarProps): JSX.Element {
<Button onClick={() => props.onRename(props.only!, name)}>重命名</Button>
) : null}
<Button onClick={props.onMetadata}>改资料</Button>
{/*
Both shelf directions are shown, and shown *always*.
The row badge marks the individual book that is off the shelf, and the
menu offers the matching single action; this bar is the batch one, where
a selection can hold a mixture and where the reader's intent is "make
these match", which has a direction. Hiding one of the two behind a
state check would mean a selection that is half on and half off the shelf
could only be pushed one way.
*/}
<Button onClick={props.onShelve}>下架</Button>
<Button onClick={props.onUnshelve}>加入书架</Button>
<Button onClick={props.onMove}>移动…</Button>
<Button className="danger" onClick={props.onDelete}>
删除
+97
View File
@@ -27,6 +27,7 @@ function entry(partial: Partial<BrowseListing['entries'][number]> & { name: stri
scanned: false,
ext: '',
indexed: false,
shelfState: null,
...partial,
};
}
@@ -474,6 +475,102 @@ describe('batch management', () => {
expect(transport.requests.some((request) => request.url.endsWith('/browse/delete'))).toBe(false);
});
it('marks the book that is not on the shelf, and only that one', async () => {
/*
* The row badge is the *only* thing on this screen that answers "why is this book
* missing from my shelf" for a book the reader took off it. Before it, every row
* was identical whatever its shelf state — so the screen that exists to explain a
* missing book could not name one.
*
* The negative half of the assertion matters as much as the positive: a badge on
* every row is a column of noise that hides the one row that is the exception, and
* the exception is the reader's whole reason for being here.
*/
const transport = new FakeTransport();
transport.json(listing({
writable: true,
entries: [
entry({ name: '在架.epub', path: '在架.epub', scanned: true, indexed: true, shelfState: 'on' }),
entry({ name: '下架.epub', path: '下架.epub', scanned: true, indexed: true, shelfState: 'off' }),
entry({ name: '文件夹', type: 'dir', path: '文件夹', shelfState: null }),
],
}));
const { screen } = makeScreen(transport);
await screen.open();
const rows = [...screen.element.querySelectorAll<HTMLElement>('.manager-row')];
const badgeOf = (name: string): string | undefined =>
rows.find((row) => row.querySelector('.manager-label')?.textContent === name)
?.querySelector('.manager-badge.off')?.textContent ?? undefined;
expect(badgeOf('下架.epub')).toBe('不在书架');
expect(badgeOf('在架.epub')).toBeUndefined();
expect(badgeOf('文件夹')).toBeUndefined();
});
it('offers "放回书架" for a book that is off the shelf, and sends the add', async () => {
/*
* The reported "需要手动加入" was *half* a bug in the UI and half one in the API.
*
* The UI half: the row menu answered "从书架拿掉" whatever the book's state, so a
* reader looking at a book that was missing from their shelf had no control that
* said otherwise — and the batch bar only ever called `remove`. Selecting the book
* and looking for "put it back" found nothing.
*/
const transport = new FakeTransport();
transport.respondWith((request) =>
request.url.includes('/browse/shelf')
? { status: 200, headers: {}, json: { applied: 1, books: ['a'], failed: [] } }
: { status: 200, headers: {}, json: listing({
writable: true,
entries: [entry({ name: '下架.epub', path: '下架.epub', scanned: true, indexed: true, shelfState: 'off' })],
}) },
);
const { screen } = makeScreen(transport);
await screen.open();
// The row's own menu, which is the single-book path.
([...screen.element.querySelectorAll<HTMLElement>('.manager-row .manager-more')][0]!).click();
await vi.waitFor(() => expect(screen.element.querySelector('.dialog')).not.toBeNull());
const putBack = [...screen.element.querySelectorAll<HTMLElement>('.dialog-options .button, .dialog button')]
.find((button) => button.textContent === '放回书架');
expect(putBack, 'a book off the shelf must offer "放回书架"').toBeTruthy();
// And it must *not* offer to take it off, which is the action that was there before.
const takeOff = [...screen.element.querySelectorAll<HTMLElement>('.dialog button')]
.find((button) => button.textContent === '从书架拿掉');
expect(takeOff).toBeUndefined();
putBack!.click();
await vi.waitFor(() => {
expect(transport.requests.some((request) => request.url.endsWith('/browse/shelf'))).toBe(true);
});
expect(JSON.parse(String(transport.requests.find((r) => r.url.endsWith('/browse/shelf'))!.body))).toEqual({
paths: ['下架.epub'],
action: 'add',
});
});
it('shows both shelf directions in the batch bar', async () => {
/*
* The batch bar needs them *both*, unconditionally.
*
* A selection can hold a mixture — some on the shelf, some off it — and the
* reader's intent is "make these match", which has a direction. Hiding one of the
* two behind a state check would mean a mixed selection could only be pushed one
* way, and the way it could not go is the one the report was about.
*/
const transport = new FakeTransport();
transport.json(listing({
writable: true,
entries: [entry({ name: '下架.epub', path: '下架.epub', scanned: true, indexed: true, shelfState: 'off' })],
}));
const { screen } = makeScreen(transport);
await screen.open();
await selectAll(screen);
const labels = [...screen.element.querySelectorAll('.manager-actions .button')].map((button) => button.textContent);
expect(labels).toContain('下架');
expect(labels).toContain('加入书架');
});
it('keeps the form actions out of the fields-scan scroller', async () => {
const transport = new FakeTransport();
transport.json(twoBooks());
+77
View File
@@ -259,6 +259,83 @@ describe('the shelf pager', () => {
expect(requestedPages(transport)).toEqual([1]);
});
it('opens a continue card through the same book object the shelf uses', async () => {
/*
* The defect in #40, at the layer the reader touches.
*
* The server answered `/library/continue` with the *progress row's* naming —
* `{ bookId, title, author, percentage, chapterTitle, updatedAt, coverUrl }` —
* while the card is typed as `ContinueReadingItem extends Book` and the tap path
* reads `book.id`. Nothing failed loudly: `title` and `coverUrl` are spelled the
* same in both shapes, so the card drew correctly, and tapping it handed the
* shell `{ id: undefined }` — i.e. `#/book/undefined`, which 404s and surfaces as
* "这本书不在书架上了".
*
* The request here is the *real* one (no item field is spelled `bookId`), and the
* assertion is on what the tap passes on, because that is the value the shell
* routes with. A test that asserted the card *rendered* passed before the fix.
*/
const transport = new FakeTransport();
transport.respondWith((request) => {
if (request.url.startsWith('/api/v1/library/continue')) {
return { status: 200, headers: {}, json: { items: [{ ...book(1), percentage: 0.4, chapterTitle: '第二章', lastReadAt: Date.now() }] } };
}
return { status: 200, headers: {}, json: page(0, 0, 0) };
});
const { screen, calls } = await makeScreen(transport);
await screen.show();
const card = screen.element.querySelector<HTMLElement>('.continue-card');
expect(card, 'a continue card must be drawn').toBeTruthy();
card!.click();
expect(calls).toContain('book:b1');
expect(calls).not.toContain('book:undefined');
});
it('fills in a missing id from the field the old server sent', async () => {
/*
* The compatibility direction, and it is deliberate rather than defensive.
*
* The upstream bug was a *naming* drift between two hand-written types, and the
* one thing that made it expensive was that it was silent: a reader on an older
* server (or the Android WebView, which caches the client separately from the
* API) got a card that drew and a tap that went nowhere, with no error anywhere.
* Accepting `bookId` as a synonym means the tap works either way round, so the
* fix cannot be half-deployed into a broken state.
*/
const transport = new FakeTransport();
transport.respondWith((request) => {
if (request.url.startsWith('/api/v1/library/continue')) {
return { status: 200, headers: {}, json: { items: [{ bookId: 'legacy-1', title: '旧服务端的书', author: '', percentage: 0.2, chapterTitle: '', updatedAt: 1, coverUrl: null }] } };
}
return { status: 200, headers: {}, json: page(0, 0, 0) };
});
const { screen, calls } = await makeScreen(transport);
await screen.show();
screen.element.querySelector<HTMLElement>('.continue-card')?.click();
expect(calls).toContain('book:legacy-1');
});
it('does not draw a card it cannot open', async () => {
/*
* And the last line of defence.
*
* A card whose book has no id at all is a card with a dead tap — the exact
* symptom reported as "提示书本不在书架上". Dropping it is better than drawing it:
* the reader loses one shortcut from a row of many, instead of gaining a control
* that lies about being one.
*/
const transport = new FakeTransport();
transport.respondWith((request) => {
if (request.url.startsWith('/api/v1/library/continue')) {
return { status: 200, headers: {}, json: { items: [{ title: '没有 id 的书', author: '', percentage: 0.2, chapterTitle: '', coverUrl: null }] } };
}
return { status: 200, headers: {}, json: page(0, 0, 0) };
});
const { screen } = await makeScreen(transport);
await screen.show();
expect(screen.element.querySelector('.continue-card')).toBeNull();
});
it('goes back to page one when the sort changes', async () => {
const transport = new FakeTransport();
transport.respondWith((request) =>
+53 -1
View File
@@ -69,7 +69,7 @@ const SCENES = [
{
name: '11-library',
label: '书库',
what: '面包屑、行、扫描忽略的徽章、以及书架/书库之间的切换',
what: '面包屑、行、「书籍」徽章、不在书架的书带「不在书架」标记,以及书架/书库之间的切换',
openLibraryAt: '#/library',
},
{
@@ -789,6 +789,58 @@ async function audit(cdp, origin, scenes, results) {
`正文开头="${chapterText}"`,
);
/*
* The search field's magnifier, measured rather than eyeballed.
*
* The report was "书架页面的搜索按钮没有垂直对齐", and it is the class of defect
* this harness exists for: nothing about it is a functional failure. The field
* works, the input focuses, the search runs — the glyph is simply drawn fourteen
* pixels above where it looks like it belongs, and every green test in the suite
* says so.
*
* Measured on the **ink**, not on the box, because the box was *already* correct
* when the defect was reported (75.98..120.19 inside a field of 75.98..120.19 —
* dead centre) and that is exactly what made the bug survive a layout review. A
* range over the glyph's contents is the browser's own answer to "where is the
* character", which is the thing a reader sees and the thing a box check cannot.
*
* The tolerance is 2px: the icon font's ink is not perfectly symmetric inside a
* symmetric em (measured 0.4px of asymmetry at this size), so an exact match would
* be an assertion about the generator's curve fitting rather than about alignment.
* The defect it guards against was 14px.
*/
// The shelf, explicitly: `audit` runs after the scenes, and the last one leaves
// the reader open. The check is about the shelf's own search field, so it has to
// put the shelf up rather than assert on whatever screen happened to be there.
await cdp.navigate(`${origin}/#/shelf`);
await cdp.waitFor('document.querySelector(".shelf-search input") !== null', 20_000);
await cdp.sleep(400);
const searchGlyph = await cdp.evaluate(`(() => {
const glyph = document.querySelector('.search-glyph');
const input = document.querySelector('.shelf-search input');
if (!glyph || !input) return { missing: true };
const range = document.createRange();
range.selectNodeContents(glyph);
const ink = range.getBoundingClientRect();
const field = input.getBoundingClientRect();
return {
ink: (ink.top + ink.bottom) / 2,
field: (field.top + field.bottom) / 2,
inkHeight: ink.height,
fieldHeight: field.height,
display: getComputedStyle(glyph).display,
};
})()`);
check(
'搜索框: 放大镜与输入框中线对齐',
!searchGlyph.missing && searchGlyph.inkHeight > 0
&& Math.abs(searchGlyph.ink - searchGlyph.field) <= 2,
searchGlyph.missing
? '没有找到搜索框或放大镜'
: `放大镜中线 ${searchGlyph.ink.toFixed(1)} / 输入框中线 ${searchGlyph.field.toFixed(1)}`
+ `(偏差 ${(searchGlyph.ink - searchGlyph.field).toFixed(1)}px,字形 ${searchGlyph.inkHeight}px,框高 ${searchGlyph.fieldHeight}px)`,
);
return { failures, results };
}
+26 -5
View File
@@ -406,6 +406,18 @@ export function createReviewServer({ port = 5199 } = {}) {
scanned: true,
ext: 'epub',
indexed: true,
/*
* One book is deliberately *off* the shelf.
*
* The library screen's whole reason to exist is answering "why is this book
* not on my shelf", and the answer for a book the reader took off it is a
* badge and an action. A fixture where every row is `'on'` renders a screen
* with neither, so the review would certify the state where the feature is
* invisible — the same way the continue-reading fixture certified a card that
* could not be opened. `三体.epub` is the one row that carries the mark, and
* the scene's label names it.
*/
shelfState: entry.type === 'dir' ? null : entry.name === '三体.epub' ? 'off' : 'on',
}));
const from = (page - 1) * pageSize;
return json(reply, {
@@ -422,11 +434,20 @@ export function createReviewServer({ port = 5199 } = {}) {
});
}
if (path === '/api/v1/library/continue') {
// A `ContinueReadingItem` *is* a `Book` with the progress flattened onto it,
// not a book-with-progress pair — see `ContinueReadingItem` in the client's
// types. Reading the server contract wrong here is what made this fixture
// crash the shelf's continue card, which is a useful thing for a review
// harness to be able to do.
/*
* A `ContinueReadingItem` is a whole `Book` with the progress flattened onto
* it — `id`, `addedAt`, `manualFields` and the rest, exactly as
* `GET /api/v1/books` returns them. See `ContinueReadingItem` in the client's
* types for why that is the contract.
*
* This fixture used to answer a *shorter*, differently-spelled object — the
* progress row's `bookId`/`updatedAt` — which is precisely the shape the real
* server was returning while the shelf's card drew from `book.id`. The
* harness agreed with the server and both were wrong, so the review passed on
* a screen whose every card deep-linked to `#/book/undefined`. Spreading the
* same `book` the shelf fixture uses is what keeps the two in step: a card
* that cannot be opened now fails the review instead of hiding in it.
*/
return json(reply, {
items: [{ ...book, percentage: 0.18, chapterTitle: CHAPTERS[1].title, lastReadAt: Date.now() }],
});