From 2bfd4d1ff45f4b66b67b1c7c7b6cba0c25b82ed8 Mon Sep 17 00:00:00 2001 From: neon <18559658+neonsolstice@users.noreply.github.com> Date: Mon, 2 Nov 2026 03:40:49 +0000 Subject: [PATCH] fix(collections): page the KOReader Shelves list instead of squeezing rows Fit every library, collection and SmartScope on one screen shrank the rows with no lower bound, so a Kobo Libra with 45 shelves drew titles on top of each other. Keep rows at a readable minimum and run the shelves over further pages, repeating a section heading where it continues. Wide screens keep their two columns and turn the page once both are full, and only the shelves on the current page fetch thumbnails. Closes #2042 --- .../bookorbit_catalog_lists.lua | 22 +- .../bookorbit_list_data.lua | 57 +++++ .../bookorbit_list_pages.lua | 51 +++-- .../spec/bookorbit_shelves_paging_test.lua | 211 ++++++++++++++++++ 4 files changed, 314 insertions(+), 27 deletions(-) create mode 100644 koreader-plugin/spec/bookorbit_shelves_paging_test.lua diff --git a/koreader-plugin/bookorbit.koplugin/bookorbit_catalog_lists.lua b/koreader-plugin/bookorbit.koplugin/bookorbit_catalog_lists.lua index 779143ccf..96449128f 100644 --- a/koreader-plugin/bookorbit.koplugin/bookorbit_catalog_lists.lua +++ b/koreader-plugin/bookorbit.koplugin/bookorbit_catalog_lists.lua @@ -401,10 +401,10 @@ function CatalogLists:listPaging(context) end return windowPaging(self:listStart(context), context.page_size, context.total, context.has_next) end - if kind == "seriespage" or kind == "authorpage" then + if kind == "seriespage" or kind == "authorpage" or kind == "shelves" then return { page = context.local_page or 1, pages = context.local_pages or 1 } end - if kind == "search" or kind == "shelves" then return { page = 1, pages = 1 } end + if kind == "search" then return { page = 1, pages = 1 } end if (kind == "books" or kind == "reading") and context.page_size and context.total then return windowPaging(self:listStart(context), context.page_size, context.total) end @@ -424,7 +424,7 @@ function CatalogLists:listGoPage(page) local p = self:listPaging(context) if page < 1 or (p.pages and page > p.pages) then return true end if page == p.page then return true end - if kind == "seriespage" or kind == "authorpage" then + if kind == "seriespage" or kind == "authorpage" or kind == "shelves" then context.local_page = page self:updateItems(nil, true) return true @@ -1270,7 +1270,18 @@ function CatalogLists:listBody(kind, context) return list or {} end local m = { libraries = tag(shelves.libraries, "libraries"), collections = tag(shelves.collections, "collections"), scopes = tag(shelves["smart-scopes"], "smart-scopes") } - return function(f) return ListPages.shelvesBody(m, f) end, nil + local f = ListView.frame(sw, sh, { toolbar = false, offline = context.list_offline }) + local layout = ListPages.shelvesLayout(m, f) + context.local_pages = #layout.pages + context.local_page = math.min(context.local_page or 1, #layout.pages) + m.local_page = context.local_page + context.visible_shelves = {} + for _ci, chunks in ipairs(layout.pages[context.local_page]) do + for _i, chunk in ipairs(chunks) do + for i = chunk.from, chunk.to do table.insert(context.visible_shelves, layout.groups[chunk.section][3][i]) end + end + end + return function(body_f) return ListPages.shelvesBody(m, body_f) end, m end if kind == "search" then local m = { results = context.results, query = context.query, scope = context.scope or "all", screen_w = sw, screen_h = sh, @@ -1427,8 +1438,7 @@ function CatalogLists:listThumbnailBooks(kind, context) for _i, b in ipairs(context.books or {}) do addBook(b) end local entries = context.entries or {} if kind == "shelves" then - entries = {} - for _s, list in pairs(context.shelves or {}) do for _i, e in ipairs(list) do table.insert(entries, e) end end + entries = context.visible_shelves or {} elseif kind == "search" then entries = {} for _i, e in ipairs(context.results.authors or {}) do table.insert(entries, e) end diff --git a/koreader-plugin/bookorbit.koplugin/bookorbit_list_data.lua b/koreader-plugin/bookorbit.koplugin/bookorbit_list_data.lua index 99e5b2e78..38520eec8 100644 --- a/koreader-plugin/bookorbit.koplugin/bookorbit_list_data.lua +++ b/koreader-plugin/bookorbit.koplugin/bookorbit_list_data.lua @@ -320,6 +320,63 @@ function ListData.fillRows(h, min_row) return { n = n, rh = math.floor(h / n) } end +-- Libraries, collections and scopes over as many pages as their rows need. +-- sizes: the rows each section holds. o: content_h, head_h, gap, row_h (the +-- natural row), min_row_h, columns (the section numbers each column of a +-- single page holds) and cols (how many columns a paged page runs). +-- Everything stays on one page while its rows can keep min_row_h; past that +-- the sections flow through the columns at min_row_h, a section that runs on +-- repeating its heading. { pages = { [page] = { [col] = { { section, from, +-- to } } } }, row_h }. An empty section keeps its heading and has to = 0. +function ListData.shelfPages(sizes, o) + local rh = o.row_h + for _i, col in ipairs(o.columns) do + local rows = 0 + for _j, s in ipairs(col) do rows = rows + sizes[s] end + local avail = o.content_h - #col * o.head_h - (#col - 1) * o.gap + rh = math.min(rh, math.floor(avail / math.max(1, rows))) + end + if rh >= o.min_row_h then + local page = {} + for ci, col in ipairs(o.columns) do + page[ci] = {} + for _j, s in ipairs(col) do table.insert(page[ci], { section = s, from = 1, to = sizes[s] }) end + end + return { pages = { page }, row_h = rh } + end + + rh = o.min_row_h + local function blank() + local page = {} + for ci = 1, o.cols do page[ci] = {} end + return page + end + local pages = { blank() } + local p, c, used = 1, 1, 0 + for s, n in ipairs(sizes) do + local from = 1 + repeat + local gap = used > 0 and o.gap or 0 + local room = math.floor((o.content_h - used - gap - o.head_h) / rh) + if room < 1 and used > 0 then + if c < o.cols then + c = c + 1 + else + c, p = 1, p + 1 + pages[p] = blank() + end + used, gap = 0, 0 + room = math.floor((o.content_h - o.head_h) / rh) + end + local take = math.min(n - from + 1, math.max(1, room)) + table.insert(pages[p][c], { section = s, from = from, to = from + take - 1 }) + used = used + gap + o.head_h + take * rh + from = from + take + until from > n + end + return { pages = pages, row_h = rh } +end + -- The page holding a 0-based index, 1-based. function ListData.pageOf(index, per) if not index or not per or per < 1 then return 1 end diff --git a/koreader-plugin/bookorbit.koplugin/bookorbit_list_pages.lua b/koreader-plugin/bookorbit.koplugin/bookorbit_list_pages.lua index a1f58e35a..d42168f89 100644 --- a/koreader-plugin/bookorbit.koplugin/bookorbit_list_pages.lua +++ b/koreader-plugin/bookorbit.koplugin/bookorbit_list_pages.lua @@ -82,36 +82,45 @@ function Pages.listBody(m, f) end -- --------------------------------------------------------------- shelves --- Libraries, collections and scopes on one page; wide screens set the --- sections in two columns. -function Pages.shelvesBody(m, f) +-- Libraries, collections and scopes. Wide screens set the sections in two +-- columns; more shelves than a screen holds at a readable row run on to +-- further pages, a section that continues repeating its heading. +function Pages.shelvesLayout(m, f) local groups = { { _("Libraries"), "library", m.libraries or {} }, { _("Collections"), "folder-open", m.collections or {} }, { _("SmartScopes"), "aperture", m.scopes or {} }, } local wide = ListView.screenW() >= 760 - local gap_s = px(14) - local columns = wide and { { groups[1] }, { groups[2], groups[3] } } or { groups } - local col_w = wide and math.floor((f.w - px(28)) / 2) or f.w - local rh = px(72) - for _i, cg in ipairs(columns) do - local heads, rows = #cg, 0 - for _j, g in ipairs(cg) do rows = rows + #g[3] end - rh = math.min(rh, math.floor((f.content_h - heads * px(36) - (heads - 1) * gap_s) / math.max(1, rows))) - end + local layout = ListData.shelfPages({ #groups[1][3], #groups[2][3], #groups[3][3] }, { + content_h = f.content_h, head_h = px(36), gap = px(14), row_h = px(72), min_row_h = px(56), + columns = wide and { { 1 }, { 2, 3 } } or { { 1, 2, 3 } }, cols = wide and 2 or 1, + }) + layout.groups, layout.wide = groups, wide + return layout +end + +function Pages.shelvesBody(m, f) + local layout = Pages.shelvesLayout(m, f) + local page = math.max(1, math.min(#layout.pages, m.local_page or 1)) + local col_w = layout.wide and math.floor((f.w - px(28)) / 2) or f.w + local rh = layout.row_h local col_els = {} - for _i, cg in ipairs(columns) do - local kids = {} - for gi, g in ipairs(cg) do - if gi > 1 then table.insert(kids, space(nil, gap_s)) end - table.insert(kids, ListView.sectionHead(col_w, { icon = g[2], label = g[1], count = #g[3] })) - for i, s in ipairs(g[3]) do - local inner = vstack({ w = col_w, h = rh }, { i > 1 and ListView.rule(col_w) or false, flex(0), ListView.shelfRow(col_w, rh, s), flex(0) }) - table.insert(kids, region(inner, { kind = "open-shelf", shelf = s }, "shelf-" .. g[1] .. "-" .. i, "shelf-" .. s.section .. "-" .. tostring(s.id))) + for _ci, chunks in ipairs(layout.pages[page]) do + if #chunks > 0 then + local kids = {} + for ki, chunk in ipairs(chunks) do + local g = layout.groups[chunk.section] + if ki > 1 then table.insert(kids, space(nil, px(14))) end + table.insert(kids, ListView.sectionHead(col_w, { icon = g[2], label = g[1], count = #g[3] })) + for i = chunk.from, chunk.to do + local s = g[3][i] + local inner = vstack({ w = col_w, h = rh }, { i > chunk.from and ListView.rule(col_w) or false, flex(0), ListView.shelfRow(col_w, rh, s), flex(0) }) + table.insert(kids, region(inner, { kind = "open-shelf", shelf = s }, "shelf-" .. g[1] .. "-" .. i, "shelf-" .. s.section .. "-" .. tostring(s.id))) + end end + table.insert(col_els, vstack({ w = col_w }, kids)) end - table.insert(col_els, vstack({ w = col_w }, kids)) end return hstack({ w = f.w, valign = "top", gap = px(28) }, col_els) end diff --git a/koreader-plugin/spec/bookorbit_shelves_paging_test.lua b/koreader-plugin/spec/bookorbit_shelves_paging_test.lua new file mode 100644 index 000000000..e1f08b5af --- /dev/null +++ b/koreader-plugin/spec/bookorbit_shelves_paging_test.lua @@ -0,0 +1,211 @@ +-- Proves the Shelves page never squeezes a row below its own text: shelves a +-- screen cannot hold at a readable row run on to further pages, a section that +-- continues repeats its heading, and every shelf is shown exactly once. + +package.loaded["gettext"] = function(text) + return text +end +package.loaded["ffi/util"] = { + template = function(text, ...) + local values = { ... } + return (text:gsub("%%(%d+)", function(index) + return tostring(values[tonumber(index)]) + end)) + end, +} +package.loaded["document/documentregistry"] = { hasProvider = function() return true end } +package.loaded["util"] = { + fixUtf8 = function(value) return value end, + diskUsage = function() return { total = 1, available = 1 } end, +} +package.loaded["datetime"] = { + shortDayOfWeekToLongTranslation = {}, + shortDayOfWeekTranslation = {}, + longMonthTranslation = {}, + shortMonthTranslation = {}, + secondsToHour = function() return "12:00" end, + stringISO8601ToSeconds = function() return nil end, +} +package.loaded["ui/network/manager"] = { isConnected = function() return true end } +package.loaded["bookorbit_stats_reader"] = { + getDailySeconds = function() return {} end, + getBookPace = function() return nil end, + getHourMinutes = function() return nil end, + getSessionRecords = function() return nil end, +} +package.loaded["lua-ljsqlite3/init"] = false +package.loaded["datastorage"] = { getSettingsDir = function() return "/nonexistent" end } +package.loaded["libs/libkoreader-lfs"] = { attributes = function() return nil end } +package.loaded["ffi/blitbuffer"] = setmetatable({}, { __index = function() return 0 end }) +package.loaded["ui/font"] = { getFace = function() return {} end } + +-- A Kobo Libra: 1264 x 1680 at 300 dpi. +local screen = { w = 1264, h = 1680, dpi = 300 } +package.loaded["device"] = { + hasFrontlight = function() return false end, + screen = { + getWidth = function() return screen.w end, + getHeight = function() return screen.h end, + getDPI = function() return screen.dpi end, + scaleBySize = function(_, value) + return math.ceil(value * math.min(screen.w, screen.h) / 600) + end, + }, +} + +G_reader_settings = { isTrue = function() return false end } + +package.path = "koreader-plugin/bookorbit.koplugin/?.lua;" .. package.path + +local ListData = require("bookorbit_list_data") +local BoardMetrics = require("bookorbit_board_metrics") + +local function assertEqual(actual, expected, label) + if actual ~= expected then + error(string.format("%s: expected %s, got %s", label, tostring(expected), tostring(actual))) + end +end + +local function assertTrue(value, label) + if not value then error(label) end +end + +-- Every shelf, in the order the pages show them, as "section:row". +local function shown(layout) + local out = {} + for _p, page in ipairs(layout.pages) do + for _c, chunks in ipairs(page) do + for _k, chunk in ipairs(chunks) do + for i = chunk.from, chunk.to do table.insert(out, chunk.section .. ":" .. i) end + end + end + end + return out +end + +local function expected(sizes) + local out = {} + for s, n in ipairs(sizes) do + for i = 1, n do table.insert(out, s .. ":" .. i) end + end + return out +end + +local function assertAllShownOnce(layout, sizes, label) + assertEqual(table.concat(shown(layout), ","), table.concat(expected(sizes), ","), label .. ": every shelf shows once, in order") +end + +local function assertColumnsFit(layout, o, label) + for p, page in ipairs(layout.pages) do + for c, chunks in ipairs(page) do + local used = 0 + for k, chunk in ipairs(chunks) do + used = used + (k > 1 and o.gap or 0) + o.head_h + (chunk.to - chunk.from + 1) * layout.row_h + end + assertTrue(used <= o.content_h, string.format("%s: page %d column %d is %d tall in %d", label, p, c, used, o.content_h)) + end + end +end + +local function opts(extra) + local o = { content_h = 1000, head_h = 36, gap = 14, row_h = 72, min_row_h = 56, columns = { { 1, 2, 3 } }, cols = 1 } + for k, v in pairs(extra or {}) do o[k] = v end + return o +end + +-- A few shelves keep their natural row on one page. +local sizes = { 1, 2, 1 } +local layout = ListData.shelfPages(sizes, opts()) +assertEqual(#layout.pages, 1, "a few shelves stay on one page") +assertEqual(layout.row_h, 72, "at the natural row height") +assertAllShownOnce(layout, sizes, "few shelves") + +-- Rows shrink toward the minimum before anything pages. +sizes = { 3, 8, 3 } +layout = ListData.shelfPages(sizes, opts()) +assertEqual(#layout.pages, 1, "fourteen shelves still fit one page") +assertEqual(layout.row_h, math.floor((1000 - 3 * 36 - 2 * 14) / 14), "their rows share the room") +assertTrue(layout.row_h >= 56 and layout.row_h < 72, "between the minimum and the natural row") + +-- One row more than the minimum allows pages instead of squeezing. +local fits = math.floor((1000 - 3 * 36 - 2 * 14) / 56) +sizes = { 3, fits - 3, 0 } +layout = ListData.shelfPages(sizes, opts()) +assertEqual(#layout.pages, 1, "the most rows a page holds at the minimum stay on one page") +assertTrue(layout.row_h >= 56 and layout.row_h < 58, "at about the minimum") +sizes = { 3, fits - 2, 0 } +layout = ListData.shelfPages(sizes, opts()) +assertTrue(#layout.pages > 1, "one more row pages") +assertEqual(layout.row_h, 56, "and never goes below the minimum") +assertAllShownOnce(layout, sizes, "boundary") +assertColumnsFit(layout, opts(), "boundary") + +-- The reported library: 3 libraries, 27 collections, 15 scopes. +sizes = { 3, 27, 15 } +layout = ListData.shelfPages(sizes, opts()) +assertTrue(#layout.pages > 1, "45 shelves run over several pages") +assertEqual(layout.row_h, 56, "at the minimum row") +assertAllShownOnce(layout, sizes, "reported library") +assertColumnsFit(layout, opts(), "reported library") +local first = layout.pages[1][1][1] +assertEqual(first.section .. ":" .. first.from, "1:1", "Libraries open the first page") +local second = layout.pages[2][1][1] +assertTrue(second.from > 1 or second.section > 1, "a later page continues where the last stopped") +for p = 2, #layout.pages do + local chunk = layout.pages[p][1][1] + assertTrue(chunk.to >= chunk.from, "a continuing section always has a row under its heading (page " .. p .. ")") +end + +-- Two columns flow left to right, then on to the next page. +local wide = opts({ columns = { { 1 }, { 2, 3 } }, cols = 2 }) +layout = ListData.shelfPages(sizes, wide) +assertTrue(#layout.pages > 1, "45 shelves page on a wide screen too") +assertTrue(#layout.pages < #ListData.shelfPages(sizes, opts()).pages, "two columns need fewer pages than one") +assertAllShownOnce(layout, sizes, "wide") +assertColumnsFit(layout, wide, "wide") +assertEqual(#layout.pages[1][2] > 0, true, "the second column fills before a page turns") + +-- A wide page whose columns fit keeps the sections where they were. +sizes = { 3, 6, 4 } +layout = ListData.shelfPages(sizes, wide) +assertEqual(#layout.pages, 1, "a few shelves fit one wide page") +assertEqual(#layout.pages[1][1], 1, "Libraries have the left column") +assertEqual(#layout.pages[1][2], 2, "Collections and SmartScopes share the right") +assertAllShownOnce(layout, sizes, "wide fit") + +-- Empty sections keep their heading and never break the flow. +layout = ListData.shelfPages({ 0, 0, 0 }, opts()) +assertEqual(#layout.pages, 1, "no shelves at all is one page") +assertEqual(layout.pages[1][1][2].to, 0, "an empty section has no rows") +layout = ListData.shelfPages({ 0, 40, 0 }, opts()) +assertAllShownOnce(layout, { 0, 40, 0 }, "only collections") +assertColumnsFit(layout, opts(), "only collections") + +-- No section is left with a heading and no rows, however the page breaks. +for total = 1, 80 do + local s = { 3, total, 15 } + local l = ListData.shelfPages(s, opts()) + assertAllShownOnce(l, s, "sweep " .. total) + assertColumnsFit(l, opts(), "sweep " .. total) + for p, page in ipairs(l.pages) do + for _c, chunks in ipairs(page) do + for _k, chunk in ipairs(chunks) do + assertTrue(chunk.to >= chunk.from or s[chunk.section] == 0, "sweep " .. total .. ": page " .. p .. " has a heading without rows") + end + end + end +end + +-- On the Libra itself the minimum row holds the two lines of a shelf's text. +local px, lh1 = BoardMetrics.px, BoardMetrics.lh1 +local text_h = lh1(15) + px(2) + lh1(13) +assertTrue(px(56) >= text_h, string.format("a minimum row (%d) holds a title and its count (%d)", px(56), text_h)) +local content_h = screen.h - px(6) - px(8) - px(64) - px(8) - px(40) +local libra = ListData.shelfPages({ 3, 27, 15 }, { + content_h = content_h, head_h = px(36), gap = px(14), row_h = px(72), min_row_h = px(56), + columns = { { 1, 2, 3 } }, cols = 1, +}) +assertTrue(libra.row_h >= text_h, string.format("the reported library's rows (%d) are never shorter than their text (%d)", libra.row_h, text_h)) +assertTrue(#libra.pages >= 2, "the reported library pages on a Libra") + +print("bookorbit_shelves_paging_test.lua: ok")