From f2701da70389498682126835f0b0aca3624b68c3 Mon Sep 17 00:00:00 2001 From: hectorqin <8m0v8c12sZDrY2infddElF+hectorqin@noreply.cnb.cool> Date: Sat, 19 Sep 2026 11:46:42 +0800 Subject: [PATCH] =?UTF-8?q?fix(web,server):=20=E7=BB=A7=E7=BB=AD=E9=98=85?= =?UTF-8?q?=E8=AF=BB=E5=B8=A6=E4=B8=8A=E5=AE=8C=E6=95=B4=E4=B9=A6=E5=AF=B9?= =?UTF-8?q?=E8=B1=A1=EF=BC=8C=E4=B9=A6=E5=BA=93=E8=83=BD=E4=B8=8A=E6=9E=B6?= =?UTF-8?q?=EF=BC=8C=E6=94=BE=E5=A4=A7=E9=95=9C=E5=9B=9E=E5=88=B0=E4=B8=AD?= =?UTF-8?q?=E7=BA=BF?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 三处 #40 后续：继续阅读卡片点了报「不在书架上」、书库的书加不回书架、 书架搜索框的放大镜高了一行。 --- docs/api.md | 38 ++++++++- docs/architecture.md | 23 ++++++ docs/ui-review/README.md | 3 +- docs/ui-review/report.json | 7 +- docs/ui.md | 56 +++++++++++++ server/src/http/routes/library.ts | 13 ++- server/src/main.ts | 2 +- server/src/services/browse.ts | 132 +++++++++++++++++++++++++----- server/src/services/shelf.ts | 30 +++++++ server/src/services/sync.ts | 95 ++++++++++++++++----- server/test/api.test.ts | 84 ++++++++++++++++++- server/test/browse.test.ts | 2 +- server/test/formats.test.ts | 2 +- server/test/tts.test.ts | 2 +- server/test/uploads.test.ts | 76 ++++++++++++++++- web/src/api/client.ts | 35 +++++++- web/src/api/types.ts | 27 ++++++ web/src/styles/reader.css | 57 +++++++++++-- web/src/ui/manager-screen.tsx | 78 +++++++++++++++++- web/test/manager.test.ts | 97 ++++++++++++++++++++++ web/test/shelf-screen.test.ts | 77 +++++++++++++++++ web/tools/ui-review/run.mjs | 54 +++++++++++- web/tools/ui-review/server.mjs | 31 +++++-- 23 files changed, 948 insertions(+), 73 deletions(-) diff --git a/docs/api.md b/docs/api.md index e9f9747..d0df2ff 100644 --- a/docs/api.md +++ b/docs/api.md @@ -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=` 流式返回这本书的**某一个**文件（二进制流，不是 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` **且存在一个未丢失的文件**。少了后一半，一份被删掉、或唯一副本在没插上的移动硬盘上的书 diff --git a/docs/architecture.md b/docs/architecture.md index f7a7ed3..a617757 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -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`， diff --git a/docs/ui-review/README.md b/docs/ui-review/README.md index 7202200..9685ec1 100644 --- a/docs/ui-review/README.md +++ b/docs/ui-review/README.md @@ -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） | 全部通过。 diff --git a/docs/ui-review/report.json b/docs/ui-review/report.json index 570e0f6..e412376 100644 --- a/docs/ui-review/report.json +++ b/docs/ui-review/report.json @@ -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": [] diff --git a/docs/ui.md b/docs/ui.md index 4a02846..2958f16 100644 --- a/docs/ui.md +++ b/docs/ui.md @@ -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）。 diff --git a/server/src/http/routes/library.ts b/server/src/http/routes/library.ts index 08c1a06..c4b9e97 100644 --- a/server/src/http/routes/library.ts +++ b/server/src/http/routes/library.ts @@ -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; /* * `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) => { diff --git a/server/src/main.ts b/server/src/main.ts index f3467a0..4ca2ebd 100644 --- a/server/src/main.ts +++ b/server/src/main.ts @@ -47,7 +47,7 @@ async function main(): Promise { }); 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); diff --git a/server/src/services/browse.ts b/server/src/services/browse.ts index e35a448..78d7378 100644 --- a/server/src/services/browse.ts +++ b/server/src/services/browse.ts @@ -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 { - return this.listSync(relPathInput, page, pageSize); + list(relPathInput: string, page = 1, pageSize = BROWSE_PAGE_SIZE, userId?: string): Promise { + return this.listSync(relPathInput, page, pageSize, userId); } - private async listSync(relPathInput: string, page: number, pageSize: number): Promise { + private async listSync(relPathInput: string, page: number, pageSize: number, userId?: string): Promise { 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(); + 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(); + 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; } /** diff --git a/server/src/services/shelf.ts b/server/src/services/shelf.ts index 6291d79..a35fd67 100644 --- a/server/src/services/shelf.ts +++ b/server/src/services/shelf.ts @@ -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 { + const unique = [...new Set(bookIds)].filter((id) => id !== ''); + if (unique.length === 0) return new Map(); + const rows = this.db.all( + `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. diff --git a/server/src/services/sync.ts b/server/src/services/sync.ts index d000aac..d3fd0b7 100644 --- a/server/src/services/sync.ts +++ b/server/src/services/sync.ts @@ -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. */ diff --git a/server/test/api.test.ts b/server/test/api.test.ts index e8e51ad..915a6db 100644 --- a/server/test/api.test.ts +++ b/server/test/api.test.ts @@ -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> }).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> }).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 diff --git a/server/test/browse.test.ts b/server/test/browse.test.ts index 2ec89d8..1345bbb 100644 --- a/server/test/browse.test.ts +++ b/server/test/browse.test.ts @@ -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); diff --git a/server/test/formats.test.ts b/server/test/formats.test.ts index 29d5d69..1b51a56 100644 --- a/server/test/formats.test.ts +++ b/server/test/formats.test.ts @@ -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(); diff --git a/server/test/tts.test.ts b/server/test/tts.test.ts index 557bce6..cef2213 100644 --- a/server/test/tts.test.ts +++ b/server/test/tts.test.ts @@ -104,7 +104,7 @@ async function buildInstance(): Promise { 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); diff --git a/server/test/uploads.test.ts b/server/test/uploads.test.ts index 741458a..f08a364 100644 --- a/server/test/uploads.test.ts +++ b/server/test/uploads.test.ts @@ -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> => + (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 => + (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', diff --git a/web/src/api/client.ts b/web/src/api/client.ts index 1558e97..168169c 100644 --- a/web/src/api/client.ts +++ b/web/src/api/client.ts @@ -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 { - const result = await this.get<{ items: ContinueReadingItem[] }>(`/api/v1/library/continue?limit=${limit}`, options); - return result.items; + const result = await this.get<{ items: Array }>( + `/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 ---- diff --git a/web/src/api/types.ts b/web/src/api/types.ts index 2518818..de18ab1 100644 --- a/web/src/api/types.ts +++ b/web/src/api/types.ts @@ -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; diff --git a/web/src/styles/reader.css b/web/src/styles/reader.css index 7561295..8b9af12 100644 --- a/web/src/styles/reader.css +++ b/web/src/styles/reader.css @@ -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 `` 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; diff --git a/web/src/ui/manager-screen.tsx b/web/src/ui/manager-screen.tsx index af6d38f..54e9d14 100644 --- a/web/src/ui/manager-screen.tsx +++ b/web/src/ui/manager-screen.tsx @@ -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 = { + 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 { 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(扫描忽略); else if (entry.scanned) meta.push(书籍); if (entry.hidden && !entry.hiddenByRule) meta.push(隐藏); + /* + * 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(不在书架); return (
props.onRename(props.only!, name)}>重命名 ) : null} + {/* + 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. + */} +