Shared reading preference + author-page filter bar (results-toolbar Phase 0–1)#12949
Draft
lokesh wants to merge 6 commits into
Draft
Shared reading preference + author-page filter bar (results-toolbar Phase 0–1)#12949lokesh wants to merge 6 commits into
lokesh wants to merge 6 commits into
Conversation
Foundations for unifying filter/sort across listing surfaces. Storage: availability and language move from per-session sessionStorage to cross-session localStorage, re-scoped from surface-specific keys (ol-header-search-*) to a global reading-preference namespace (ol-reading-pref-*). The read/write helpers are centralized in constants.js (readStoredAvailability/writeStoredAvailability/writeStoredLanguages) and shared by the header modal and the /search filter row, so every surface persists the preference identically. This makes availability + language durable across visits and ready to be inherited by author and other listing pages (Phase 1+). De-dup guard: AVAILABILITY_TO_PARAMS is mirrored in code.py and constants.js; test_availability_sync.py now parses both from source and fails CI on any drift, replacing the bare 'keep in sync' comment.
works_by_author now accepts an availability value ('all'/'readable'/'borrowable'/
'open') and a list of language codes, merging them into the Solr param dict
exactly as /search does — availability via the shared AVAILABILITY_TO_PARAMS
(rewritten into ebook_access filters by run_solr_query), language as an OR'd fq.
'all' with no languages leaves the author scope unchanged, so existing behavior
is preserved.
Author.get_books reads these off the request: the legacy ?mode=ebooks link still
maps to the readable filter, and an explicit availability filter in the URL
(has_fulltext/public_scan) is detected via get_active_availability and wins.
This is the server half of promoting the author page's read-only facets to a
live filter bar; the template + client wiring follow. Tests cover the param
merge and the unchanged default scope.
Promotes the author works list from a single 'Show only ebooks?' link to the shared filter bar (availability toggle + language popover), reusing SearchFilterBar.js — generalized to navigate the current page's path instead of a hardcoded /search, so the same controller drives /search and author pages. The global reading preference is inherited here: a stored 'Readable only' lands pre-applied and visibly reflected in the toggle. Guardrails per the agreed model: - Visibility: the bar shows when there's more than one work to act on, OR whenever a filter is active (so an inherited filter is always clearable). A single-work author with no filter has nothing to filter, so it's hidden. - Escape hatch: when an active/inherited filter matches zero works, an inline 'Show all N works?' clears the filters back to the full bibliography rather than implying the author has none. The search-within-author box now preserves the active availability/language/sort instead of unconditionally forcing has_fulltext. Verified against the running app: filters reach Solr (3->0 works), escape hatch fires with the real total, and the visibility rule holds across 1-work and 3-work authors.
The 'Show all N works' link cleared the filter params from the URL, but the durable global reading preference was untouched — so maybeApplyStickyFilters re-inherited it on reload and zeroed the page again (e.g. a stored French language filter). The link now carries js-clear-reading-prefs; SearchFilterBar.js wipes the stored availability + language preference on click so 'show all' sticks. Degrades cleanly without JS (no inheritance runs without JS).
The toggle rendered in its default off state, then SearchFilterBar.js set checked=true after the JS bundle loaded — so a readable-scoped page visibly animated the toggle on after first paint. Both /search and author pages now emit `checked` when availability is active (get_active_availability(param) != 'all'), so the toggle paints in its final position with no transition; the client-side seed becomes a no-op when the state already matches.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this does
Lays the foundation for a shared, capability-driven results toolbar across Open Library's book-listing surfaces, and ships the first real consumer: a live filter bar on author pages.
/searchfilter-bar controller.This is intentionally scoped as a proof-of-concept for the broader effort; subject pages, lists, reading log, and the extraction of a single
<results-toolbar>component are follow-ups (see Roadmap).How we think about filter/sort state
The central design question this PR answers: when a user sets a filter on one listing surface, where else should it apply, and for how long? The guiding principle:
Intent vs. scope
What's shared vs. not, and across what span
localStorage(ol-reading-pref-availability)localStorage(ol-reading-pref-languages)Availability + language are a single durable reading preference: set it anywhere it's offered, and it's read everywhere it's offered, including on a return visit. We moved this state from
sessionStorage(per-tab) tolocalStorage(per-device) and renamed the keys from surface-specific (ol-header-search-*) to a neutral global namespace (ol-reading-pref-*), because the value is owned by no single surface.Two guardrails that make global, cross-session stickiness safe
A durable filter that silently prunes results with no visible cause is a footgun. Two rules prevent that:
The capability model (where this is heading)
Each surface's toolbar is the intersection of two layers:
Rendering the intersection is what makes the single-work / small-N / uniform-facet cases fall out of one rule instead of per-page special-casing. This PR implements the visibility half of that on author pages; the full descriptor lands when the component is extracted.
De-duplication
AVAILABILITY_TO_PARAMS(the availability → Solr-param mapping) genuinely runs in two runtimes — Python (worksearch/code.py, server-rendered URLs + active-filter detection) and JS (constants.js, client-side URL building with no round-trip). A single shared literal isn't practical, so instead of the old "keep in sync" comment,test_availability_sync.pyparses both copies from source and fails CI on any drift (and checks the availability value set is consistent on both sides).Commits
sessionStorage→localStorage, global key namespace, centralized read/write helpers shared by the modal + filter row, and the Python/JS taxonomy sync test.works_by_authoraccepts availability + language, merged into the Solr query exactly as/searchdoes;Author.get_booksreads them off the URL.SearchFilterBar.jsgeneralized from a hardcoded/searchto the current page's path so one controller drives both surfaces; visibility rule + escape hatch.Verification
works_by_authorparam merge.test_availability_sync.py+test_worksearch.pypass in Docker.Roadmap (not in this PR)
<results-toolbar>web component (composing the segmented view control + sort dropper + availability toggle + language popover) driven by the capability descriptor, now that/searchand author pages share the pattern.Notes for reviewers
/subjects/— promoting those to in-place facet filters is deliberately out of scope here./search). This is intentional — "show all" means "stop filtering" — and consistent with the single-preference model.