refactor(series): unify series-gaps toolbar with shared books_select_toolbar (bookshelf-v1ap) #1197
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "bd-bookshelf-v1ap"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
templates/pages/series_show.html(the series-gaps mixed-grid branch) with{{template "books_select_toolbar" ...}}— the same canonical SVG-icon partial introduced by bookshelf-rzlf and already used by the grid and table views.books_select_filter_controller.jsto thepage_scriptsblock inseries_show.html(required by the canonical toolbar'sbooks-select-filterStimulus controller).Test plan
make build— template renders without parse errorsmake test— all unit tests passgo build -tags e2e ./e2e/browser/...— e2e compilesseries-gaps page: checking a book reveals the canonical SVG-icon toolbar): seeds a series with missing entries, checks the owned book, asserts.books-select-barbecomes visible and.bst-trigger[aria-label="Metadata actions"]is present, posts PR screenshotCloses bead bookshelf-v1ap on merge.
Replace the bespoke inline emoji-button bulk toolbar in the series_show.html series-gaps branch with {{template "books_select_toolbar" ...}}, the same canonical SVG-icon partial used by the grid and table views since bookshelf-rzlf. Also adds books_select_filter_controller.js to the page_scripts block (required by the canonical toolbar's books-select-filter controller). Adds It 8 to Journey 5 (Bulk Operations) e2e: navigates to a series page with missing entries, checks a book, and asserts the canonical SVG-icon toolbar appears (proving the unified partial wires up correctly in the series-gaps DOM). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>Security Review — PR #1197 (series-gaps canonical bulk toolbar)
Scope: presentation-only refactor swapping the bespoke inline emoji toolbar in
templates/pages/series_show.html(MissingEntries branch) for the canonicalbooks_select_toolbarpartial, plus a browser e2e step.Findings:
No security findings.
Verification notes:
books_select_toolbarpartial emits only static SVG markup, fixed/books/bulk/*action URLs, and numeric/{{with .Filter}}data-attributes. Notemplate.HTML/safeHTML/| safeintroduced. User-controlled fields (book titles, series names) continue to render through standard auto-escaped{{.Title}}pipelines already on the page — unchanged. html/template auto-escaping intact./books/bulk/{delete,locks,attach,move,enrich,custom-fetch,metadata,generate-covers,llm-vision}URLs the removed bespoke bar already wired. No new privileged action is exposed: the partial's all-matching "Select all books" button renders only{{if .HasMoreBooks}}, andHasMoreBooksis not passed in the series_show dict (nil.Filter-> filter data-values resolve to 0/empty), so thebooks-select-filterall-matching mode is inert here. Bulk endpoints enforce user scoping server-side (unchanged by this PR). Presentation only.style=and no inlineonclick=added — all handlers use Stimulusdata-action="click->...". Compliant with productionstyle-src 'self'.data-controller="books-select shelf-assign"wrapper.REVIEW VERDICT: 0 blocker, 0 major, 0 minor
[MINOR] static/css/main.css — dead CSS rule
.books-select-bar-metaThe old emoji toolbar in series_show.html used this rule to set
font-size: 1.1remon the Metadata button. With the switch to the canonical SVG-icon toolbar (which uses different button classes like.bst-trigger), this rule is no longer referenced anywhere in the codebase (confirmed via git grep on the PR branch). No other templates use.books-select-bar-meta.Suggested fix: remove the
.books-select-bar-meta { font-size: 1.1rem; }rule from main.css as dead code.Behavior & wiring verification:
data-controller="books-select shelf-assign"for toolbar to read selection statestyle=attributes; toolbar visibility controlled viahiddenattribute and classesREVIEW VERDICT: 0 blocker, 0 major, 1 minor
Series-gaps page — canonical bulk-select toolbar (bookshelf-v1ap)
The series-gaps mixed grid (owned books + ghost missing-entry cards) now renders the shared
books_select_toolbarpartial with SVG-icon submenus, replacing the bespoke inline emoji toolbar. Screenshot captured locally with go-rod.UI Review — PR #1197 (bookshelf-v1ap)
Screenshot reviewed: attachment
641a4966-ebb9-46ef-b98e-f541ac474b4b(1280×800 PNG confirmed)What the screenshot shows
Series detail page for "v1ap-gap-series-screenshot" (1 BOOK / 1/3 OWNED / 0/1 READ). Books-in-series section displays a mixed grid: one owned card (#1 "Gap Series Book One", checkbox checked/blue) plus two ghost gap cards (#2, #3, dashed-border placeholder cells). The canonical
books_select_toolbarfloats at the bottom showing "1 selected" + the full SVG-icon action row + red trash on the far right.1. Canonical-component reuse
The toolbar rendered is the exact canonical
books_select_toolbar.htmlpartial (invoked attemplates/pages/series_show.html:268). Icon order matches: database-cylinder submenu → bookmark → lock → arrows → paperclip → ellipsis submenu → check-square → x-square → red trash (btn-danger). TheHasMoreBooksselect-all button is correctly absent (no pagination on a single series). No bespoke class system, no hand-rolled toolbar. Pass.2. Layout / overlap
The floating bar overlaps the lower portion of the gap cards (#2/#3). This is identical behaviour to the main book-list grid — the bar is
position: fixedat the bottom and appears when ≥1 item is selected. The gap card content (#2/#3 numbers and "Not in library" text) is visible above the bar. Acceptable — consistent with canonical usage.3. Spacing / alignment / typography
All CSS classes use design-system tokens:
var(--border)for the dashed ghost border,var(--fg-muted)for ghost text/numbers. No inlinestyle=attributes found (grep confirmed). Ghost card modifier classes (.book-card-tile--ghost,.cover-card--ghost,.cover-card-placeholder--ghost,.series-gap-label) are all defined instatic/css/main.css:7941–7982using token variables. Icon buttons are vertically centred, consistent sizing with canonical bar on other pages. Pass.4. Gap cards
Ghost cards render as dashed-border placeholders showing only the series number (#2, #3) centred. The
pointer-events: nonerule (main.css:7943) prevents accidental checkbox activation. The owned book tile shows the expected cover placeholder ("G" initial), blue checkbox, and three-dot kebab — matching the canonicalbooks_select_gridtile structure. Pass.Source cross-check
No inline
style=found. No bespoke modal/button/dialog classes. Ghost-card CSS uses token variables throughout.REVIEW VERDICT: 0 blocker, 0 major, 0 minor
e3f76a27421c217a78d7