feat: Search modal UX improvements - #12797
Conversation
Replace the legacy inline autocomplete dropdown on the header search bar with an OlDialog-based modal. On mobile it goes fullscreen; on desktop it anchors near the top of the viewport. - OlDialog: native <dialog>-based modal with focus trap, animations, placement="top", fullscreen-on-mobile, and a custom header slot - OlOptionsPopover: filter trigger + popover for rich single-select options (used by the modal for Availability) - search-modal/SearchModal: takes over autocomplete duties from SearchBar via a new disableAutocomplete option on the legacy bar - focus-utils, slot-utils: shared helpers for shadow-DOM components - Registration guards on OlPopover/OlSelectPopover so they don't double-define when imported through both lit-components and the search-modal webpack consumer
β¦ Builds on lokesh's draft internetarchive#12690.
for more information, see https://pre-commit.ci
|
Thanks for the PR, @Armansiddiqui9! π€ Copilot has been assigned for an initial review. @lokesh is assigned to this PR and currently has:
PR triage checklist (maintainers / Pam)
Note This comment was automatically generated by Pam, Open Library's Project AI Manager, on behalf of @mekarpeles. Pam is designed to provide status visibility, perform basic project management functions and relevant codebase research, and provide actionable feedback so contributors aren't left waiting. |
There was a problem hiding this comment.
Pull request overview
Enhances the header search experience by moving filtering and autocomplete into a Lit-based search modal, alongside new reusable dialog/popover components and associated styling/JS wiring.
Changes:
- Hide the legacy header βAllβ facet selector via CSS and disable the legacy inline autocomplete in
SearchBarwhen the modal is active. - Introduce a new Lit
ol-search-modalcomponent with Availability + Language filters, session persistence, and modal-driven autocomplete/results. - Add new Lit infrastructure components/utilities (
OlDialog,OlOptionsPopover, focus/slot utils) and guard custom-element registration to avoid double-definitions.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| static/css/components/header-bar.css | Hides legacy facet UI and adjusts header search input styling/width. |
| openlibrary/plugins/openlibrary/js/SearchBar.js | Adds disableAutocomplete option to allow SearchModal to take over autocomplete. |
| openlibrary/plugins/openlibrary/js/search-modal/SearchModal.js | New Lit search modal implementation with filters, results rendering, and language loading. |
| openlibrary/plugins/openlibrary/js/search-modal/constants.js | Defines Availability + Language option metadata and sessionStorage keys. |
| openlibrary/plugins/openlibrary/js/ol.js | Initializes SearchBar with autocomplete disabled and mounts SearchModal on the header input. |
| openlibrary/components/lit/utils/slot-utils.js | Adds helper for detecting meaningful slotted content. |
| openlibrary/components/lit/utils/focus-utils.js | Adds focus helpers for shadow DOM and slotted focusables. |
| openlibrary/components/lit/OlSelectPopover.js | Prevents double custom-element registration. |
| openlibrary/components/lit/OlPopover.js | Prevents double custom-element registration. |
| openlibrary/components/lit/OlOptionsPopover.js | New single-select βrich optionsβ popover component (WCAG semantics fix included). |
| openlibrary/components/lit/OlDialog.js | New native-<dialog> modal component with focus trap, placement options, and slots. |
| openlibrary/components/lit/index.js | Exports newly added Lit components from the bundle entrypoint. |
Comments suppressed due to low confidence (2)
openlibrary/plugins/openlibrary/js/search-modal/SearchModal.js:427
_loadAllLanguages()fetches/search.json?...&facets=true&facet=languageand expectsfacet_counts.facet_fields.language, but the FastAPI/search.jsonendpoint currently always runs withfacet=False(no facet_counts). As a resultrawbecomes[]and the code replaces_languageItemswith an empty array, breaking the Language popover. This should use an endpoint that actually returns language facet data (or add one) and avoid overwriting the defaults when facet data is missing/empty.
async _loadAllLanguages() {
this._langsLoading = true;
try {
const res = await fetch(
'/search.json?q=*&facets=true&limit=0&facet=language',
{ signal: AbortSignal.timeout?.(8000) }
);
if (!res.ok) throw new Error(`HTTP ${res.status}`);
const data = await res.json();
const raw = data?.facet_counts?.facet_fields?.language || [];
openlibrary/plugins/openlibrary/js/search-modal/SearchModal.js:493
- Several new user-facing strings are hardcoded in the component (e.g. dialog aria-label, input placeholder, results headings, button text). Open Libraryβs frontend typically sources JS UI strings from server-rendered i18n strings (e.g. hidden
*-i18n-stringsinputs) so they can be translated. Please wire these strings through the existing i18n mechanism instead of embedding English literals in the component.
return html`
<ol-dialog
?open=${this.open}
without-header
fullscreen-on-mobile
width="large"
placement="top"
aria-label="Search Open Library"
style="
--ol-dialog-padding: 0;
--ol-dialog-top-offset: 54px;
--ol-dialog-animation-duration: 160ms;
--ol-dialog-width-large: min(680px, 92vw);
--ol-dialog-backdrop-color: hsla(0,0%,0%,0.18);
"
@ol-after-open=${this._onDialogOpened}
@ol-after-close=${this._onDialogClosed}
>
<div slot="header" class="bar">
${SearchModal._searchIcon}
<input
type="search"
class="search-input"
placeholder="Search books, authorsβ¦"
aria-label="Search"
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Add Web Component Library entries for the two new Lit components introduced in this branch, following the existing select-popover partial conventions. Includes real-world examples: availability/sort filters and custom triggers for ol-options-popover; confirmation, form, width-preset, and search-modal dialogs for ol-dialog.
Wire the header search modal and the /search filter row through the data-i18n pattern so their UI strings are translatable instead of hardcoded English: - availability option labels/descriptions via a shared search/availability_i18n.html, read by availabilityOptionsFromElement - modal chrome strings (placeholders, aria-labels, status messages) via search/search_modal_i18n.html, read by searchModalStringsFromElement - English fallbacks (AVAILABILITY_OPTIONS, DEFAULT_SEARCH_MODAL_STRINGS) kept in search-modal/constants.js; interpolated chip label uses sprintf - unit tests for the new helpers (searchModalConstants.test.js) Bundled with the surrounding search-filter feature work on this branch (OLChip/popover components, header-bar + token CSS, worksearch wiring).
Selecting multiple languages emitted one fq per value, which Solr ANDs β
requiring a work to be in every selected language at once. Combine them
into a single language:("a" OR "b") clause so the filter is additive.
Field name is kept first so editions.fq rewriting (splits on first ':')
still resolves the field.
The legacy header SearchBar.js (and its SearchPage.js page-init shim and SearchBar.test.js tests) is fully superseded by the new ol-search-bar LIT component used by nav_head.html. Remove the .search-component .expanded/.collapsed CSS rules that only the old bar's collapse-on-small- screen behavior consumed, plus the related width clamps in the desktop / tablet stylesheets and their legacy.css duplicates. Drop addModeInputsToForm and SearchModeSelector from SearchUtils.js for the same reason β both were only called from the deleted SearchBar.js. PersistentValue / mode are kept; SearchFilterBar.js still imports them.
β¦/search URL is the source of truth on /search; sessionStorage mirrors it so the header search modal opens with the same filters next time and so filters typed into one search box carry into the next one in the same session. Server-rendered chips for availability + language (work_search_selected_ facets.html) collapse the has_fulltext / public_scan / print_disabled trio into a single Availability chip via get_active_availability() and get_availability_label() β kept in sync with AVAILABILITY_TO_PARAMS in search-modal/constants.js. The chips render on the first paint, before the async facet_counts request returns, so they're visible immediately on page load. Other facet chips (author, subject, β¦) still wait for facet_counts since they need it to resolve a display name. SearchFilterBar.js: on init, if the URL has any filter param, mirror it to sessionStorage; if the URL has no filter param and sessionStorage has a non-default value, replace-navigate with the sticky filters applied. Listen for ol-chip-select so removing the last filter via a chip clears sessionStorage before navigation (otherwise the sticky branch would bounce it right back). Schema fix (works.py): facet_rewrites can emit negated fq values like '-ebook_access:public' (from public_scan=false / 'borrowable'). Strip the leading '-' before convert_work_field_to_edition_field() lookup and re-apply it to the rewritten field; without this the editions.fq was silently dropped for the Borrowable filter. page-user.css: use visibility:hidden instead of display:none on the header search component on /search so the also-flex:1 navigation block doesn't absorb the freed slot and shift My Books / Browse.
β¦ability-language-filters
a11y/focus-trap (the primary fix): - Filter hidden elements out of the ol-dialog focus trap. display:none close buttons in SearchModal kept getting included, and .focus() on a hidden element is a silent no-op β Tab/Shift+Tab appeared stuck on the ESC pill and the "Clear all" button on mobile. - Make ol-options-popover and ol-select-popover keyboard-reachable inside the dialog: tabindex="0" on the host + focus() override that delegates to the internal trigger button (otherwise the trigger lives in shadow DOM and the outer trap can't see it). - Walk shadow boundaries when computing the trap's current index, so the delegated host is recognized even though deep activeElement is inside. - Unit tests for focus-utils.js (isFocusable, getFocusableFromSlot, findFocusableIndex, getDeepActiveElement) including a regression test for the hidden-element bug. Search modal / filter WIP bundled in: - SearchModal: drop barcode button from the modal header, replace with a close-button shown only on touch devices (paired with the ESC pill on hover-capable pointers); add publish-year to result rows; track numFound as state. - nav_head: surface the barcode link directly on touch devices (it was previously hidden and only read by the modal). - SearchFilterBar / constants: extract shared readStoredLanguages helper. - work_search_selected_facets, header-bar CSS, options-popover template, i18n strings, worksearch code: associated cleanup.
for more information, see https://pre-commit.ci
Restructure the header search area so the barcode scanner reads as its own button on touch devices and stays reachable from /search. - Move the barcode link out of .search-bar-component to be a sibling of it under .search-component, so the search-page hide rule (which now targets only the pill) leaves the scanner reachable on touch widths. - Make .search-component a flex container that lays out the pill and the barcode side-by-side with a gap. - Drop the visible 'Search' label on mobile/touch β the trigger renders as a compact icon-only pill. Desktop (>=960px) restores the wide 'Search Q' bar with the label, via overrides in header-bar--desktop.css and mirrored in legacy.css. - Add .search-page bodyclass on work_search.html and update the hide rule to scope to .search-bar-component only. - Drop the vestigial margin-right: -5px on .search-component (a 2021 nudge that no longer matches the current hamburger sizing).
Adds a mixin that lets shadow-DOM custom elements participate in outer focus traps as single tabbable leaves: - Sets tabindex="0" on the host so light-DOM focus-trap queries pick it up, and turns on delegatesFocus so .focus() and :focus-visible reach the inner control. - Apply the mixin to OLChip, OlOptionsPopover, and OlSelectPopover so they no longer need ad-hoc focus shims. - OlDialog skips its Tab trap when focus is inside an open nested <ol-popover>, letting the popover's own trap drive focus. - Adds focusableHostMixin.test.js covering the mixin contract.
β¦ch-modal work Pulls lessons from the header search-modal PR into the AI-agent docs so the next agent (or human) starting in this area doesn't re-discover them. web-components.md - Registration: the customElements.get() guard idiom β needed because some components are imported through both the lit-components bundle and a downstream webpack consumer, and a second define() call throws. - Focus and Shadow DOM: when to reach for FocusableHostMixin, why hidden elements have to be filtered out of trap lists (.focus() is a silent no-op), the deep-active-element + shadow-boundary walk for finding the current trap index, and the stash-and-restore pattern for keeping focus alive across Lit repeat() re-renders. - ARIA on lists: role="radiogroup" on a <ul> strips list semantics; wrap in a <div> instead. Plus the whitespace-in-<ul> accesslint gotcha. - Autofocus on mobile: skip text-input autofocus at the mobile breakpoint so the soft keyboard doesn't eat the panel. design.md - New Mobile section: 16px input font-size to dodge iOS Safari auto-zoom, and the @media (hover: hover) and (pointer: fine) pattern for both hover styles and touch-vs-keyboard affordance swaps (e.g. close button on touch vs. ESC pill on hover-capable pointers). i18n.md (new) - Documents the data-i18n bridge pattern: _i18n.html partial renders a JSON dict via $_(), drops it onto a data-* attribute, JS reads and merges over an English-default fallback. JS-side ugettext is a pass-through, so this is the only working path for translating client-rendered UI strings.
The site-wide lit-components bundle (ol-components.js, loaded from footer.html) already registers every <ol-*> custom element via the index.js re-exports. Side-effect imports from page bundles (e.g. SearchModal) were re-running customElements.define(), so each component file carried a defensive customElements.get() guard, and OlDialog also had an isServer guard for SSR safety. Drop the guards and stop re-importing component modules from SearchModal β the elements are registered before any page-JS handler runs. Update docs/ai/web-components.md to reflect the single registration site.
The header search modal autocomplete and the /search results page were
producing different counts for the same query + availability filter
("red" + Readable Books Only: 2 unfiltered vs 0 filtered) because the
FastAPI /search.json handler silently dropped the availability params.
`PublicQueryOptions` declared only the Solr field `public_scan_b` (the
boolean storage field, `_b` is Solr's dynamicField convention). The
URL-API names `public_scan` and `print_disabled` are different β they
aren't Solr fields; they're keys in `WorkSearchScheme.facet_rewrites`
that get translated to `ebook_access:public` / `ebook_access:printdisabled`
fq clauses. The web.py /search handler explicitly whitelists both names,
but FastAPI's Pydantic model only knew about `public_scan_b`, so
`?public_scan=true` from the modal was discarded as an unknown param
before reaching the facet_rewrites step.
Declare both fields on `PublicQueryOptions` so the FastAPI handler mirrors
the web.py whitelist. After the fix, all four availability values
(all/readable/borrowable/open) produce identical numFound on both
endpoints.
Request the editions subquery from /search.json so the modal opts into the same edition-level block-join the /search page uses. Without it, the availability filters (public_scan/print_disabled) only matched the work-level ebook_access aggregate, so the modal surfaced works the page hid β e.g. a work whose only query-matching edition is non-readable.
|
Latest version of the search: search.mp4 |
Screen.Recording.2026-06-11.162507.mp4Everything is looking great on testing the filters, recent searches, author suggestions, and mobile layout, all working cleanly. Attaching a screen recording above. |
β¦ability-language-filters # Conflicts: # openlibrary/components/lit/index.js # openlibrary/i18n/messages.pot # openlibrary/templates/design.html
for more information, see https://pre-commit.ci
Add a "Readable" access pill to result rows (from the work-level ebook_access aggregate) and a quiet "In <language>" hint when Readable Only surfaces a copy whose promoted edition isn't in the patron's site language. The trigger now carries data-search-lang (the 2-letter UI code); the modal maps it to a MARC bibliographic code via siteLanguageToMarc to compare against Solr's language field. Adds constants helpers (UI_LANG_TO_MARC, siteLanguageToMarc, languageNameFromOptions, readableLanguageMismatch), the clearAria / accessReadable / inLanguage i18n strings, and unit tests for the new helpers.
Treat JSDoc as the source of truth for the web-components API tables: only render properties that carry a description (@prop). This drops undocumented class fields, including Lit reactive state (state: true) that the analyzer can't reliably distinguish from public attributes. Also normalize manifest defaults β the analyzer emits the literal strings "null"/"undefined" for unset constructor fields, so show those as blank rather than a misleading default β and read the manifest as UTF-8 explicitly. Regenerates custom-elements.json with the JSDoc descriptions now picked up for the chip, modal, and other components.
readableLanguageMismatch now lists up to two of a readable edition's languages (joined with a comma) and appends a trailing ", β¦" when it carries more, instead of naming only the first. Codes with no display name are dropped before counting, so they neither fill a slot nor force the ellipsis.
The Readable Only count already widens to include printdisabled scans for patrons verified for print-disabled access (get_fulltext_min, keyed off the pd cookie), but the per-row Readable badge was hardcoded to public/borrowable only β so a qualifying patron was counted a book the row then showed no badge for. Thread the patron's print-disabled status onto the search trigger (data-print-disabled, read straight from the pd cookie to match the server's signal) and fold it into a new _isReadableAccess helper that treats printdisabled as readable only for those patrons. Badge and count now agree in both directions.
Key the per-result Readable badge off the promoted edition's own ebook_access rather than the work-level aggregate, falling back to the work aggregate only when no edition is promoted. A work whose only readable copies are in another language no longer flashes Readable off a copy the patron can't read in their site language; the badge and the In-<language> hint now derive from the same edition.
- Move ssGet/ssSet into constants.js; share them across SearchModal and SearchFilterBar instead of redefining the try/catch pair in each. - Extract _baseSearchParams() and parameterize _appendFilterParams() so the readable-count URL reuses the main search builder instead of mirroring it. - Add _resetResults() to replace the result-state reset open-coded across _clearInput, the short-query branch, and the fetch error handler.
Switches the searchbox input to type=search (enterkeyhint=search for mobile keyboards) and hides the WebKit search-cancel/decoration pseudo-elements so the native clear button doesn't disturb the existing styling.
β¦ered When the patron filters by two or more languages, a "Readable" row could be in any of them, so name the promoted edition's language(s) below the badge so they can tell which of their filters a row satisfies. The single-/no-filter cases are unchanged: a lone language filter constrains every result to it (no hint needed), and with no filter the existing site-language mismatch hint still applies. Extracts the shared listEditionLanguages() formatter (renamed the limit constant to HINT_LANG_LIMIT) and adds readableEditionLanguages(), which fires only for >=2 selected languages and skips the site-language gating since the explicit filter overrides it. Reuses the existing inLanguage string and result__lang element. Adds unit tests for the new helper.
Addresses review feedback on the search-modal work: - constants.js: readRecentSearches/readStoredLanguages now drop non-string entries from a corrupt or hand-edited storage value, so a bad value can't render `[object Object]`/blank recent rows or leak a bogus `language=<n>` filter. - searchModalConstants.test.js: add coverage for the storage helpers β cap, dedup-to-front, trim/blank handling, non-array values, JSON-parse failure, and getItem/setItem throwing (private browsing / quota). - OlOptionsPopover: adopt full WAI-ARIA roving-selection semantics β Arrow/ Home/End now move focus AND select the focused option (staying open), Enter/Space/click commit and close. Arrows wrap. Native same-name radios already provide the single roving tab stop. - legacy.css: remove orphaned `.mode-options` rules (the markup was deleted with the old search-mode radios).
|
@Armansiddiqui9 we're close! I made some tweaks to to the search results to make sure the items being shown in the result was the one being linked to. There was a disconnect in the code. Now waiting on reviews from @mekarpeles and @cdrini |
|
Non-technical explanation of how the search modal works in the latest state: How the search modal worksClick the header search box and a panel drops down. Type and results appear as you go (no Enter needed). Empty input shows your recent searches (up to 8, stored per-device). What appears: mostly books, plus an occasional author shortcut at the top β shown only when your query looks like an author's name (it reuses the existing results, no extra lookup, so title searches don't trigger it). Which edition shows: a work can have hundreds of editions, so we promote the single best one β best text match, nudged toward your site language. The row's title, cover, and link all come from that promoted edition, so what you click matches what you see (falling back to the plain work if none can be promoted). Links: a book row links to the promoted edition's page (or the work); an author row links to the author page. The whole row is one link. "Readable" badge: shown when you can read the promoted edition in-browser β free (public domain) or borrowable. Non-readable copies get no badge, so a book whose only readable copy is in another language won't falsely show "Readable." (Print-disabled patrons get the badge on additional scans.) Language hints: we show "In French" under the badge when (a) you haven't filtered by language but the readable copy isn't in your site language, or (b) you've picked several languages and need to know which this row satisfies. Filters:
Filters persist between the modal and the What's filtered out & why: "Readable Only" hides anything you can't read online; a language filter keeps only matching languages. Crucially, filters now apply at the edition level, matching the main |
|
@lokesh That sounds great, this is a really solid summary. Thanks for writing it all up so clearly. The edition-promotion fix landed too, so good to hear it's actually resolved at the right layer now. |
The dialog's _setInitialFocus() runs on rAF (one frame after open) and focuses the first focusable light-DOM element. On mobile that's the visible .back-btn, which stole focus from the input that _openModal had just focused in-gesture β dismissing the soft keyboard iOS had begun to raise. ol-after-open then re-focused the input, but outside the gesture, so the keyboard never came back. _setInitialFocus checks [autofocus] first, so marking the input autofocus makes all three focus paths converge on it: the in-gesture focus raises the keyboard, the rAF re-focuses the already-focused input (no blur), and ol-after-open is genuinely a no-op. Native showModal() honors autofocus too, reinforcing the in-gesture focus.
Closes #12752
Related #11216
To-dos for @Armansiddiqui9
To-do's for @lokesh
Builds on the search modal foundation from #12690 (Lokesh's draft), extending it with filter UI (Availability + Language), author suggestions, loading feedback, mobile improvements, and a full header search bar cleanup.
Technical
Search bar cleanup
The legacy
SearchBar.jsand its "All" facet<select>dropdown have been removed from the header entirely (along with the now-dead header CSS and theSearchBar.test.jssuite). Filter scoping moved into the modal, so the header search box is now a single trigger button that opens<ol-search-modal>viainitSearchModal().Search modal UX polish
--ol-dialog-top-offset: 54pxβ anchors the dialog just below the header so it feels like a dropdown, not a modal takeover--ol-dialog-width-large: min(680px, 92vw)β narrower and more focused (was 800px), like mek's prototype--ol-dialog-backdrop-color: hsla(0,0%,0%,0.18)β lighter backdrop, less visual weightmax-heightreduced from50vhto320pxAvailability filter β dedicated
<ol-availability-filter>componentThe Availability dropdown is now a bespoke component rather than the generic
<ol-options-popover>, because availability has presentation the generic single-select doesn't model:It composes
<ol-popover>for animation, focus trap, mobile tray, and Escape/outside-click dismissal β reusing that behaviour rather than reimplementing it. ("In scope" is purely visual; only the radio's checked state is exposed to assistive tech.) This is the revisited visual treatment for the "readable online" hierarchy.Author suggestions
When the query names the author of one of the top works
/search.jsonalready returned, the modal surfaces an author row linking straight to that author's page β covering the common "type a name β I want that author" case (the old Author facet) with no extra Solr round-trip. Matching is self-protecting: a title search returns an author's works, but the title isn't part of their name, so nothing is surfaced. Capped at 3 rows; the matching logic lives inauthorSuggestion.jsas pure, unit-tested functions.Loading feedback on result press
Pressing a result navigates the whole window, and the next page can take a beat to start painting. The pressed row now holds full opacity while the rest dim back, its cover darkens under a spinner (mirroring the
<ol-button>loading spinner), and the row scales to0.985β matching the header search field's press feedback. Cleared on close, on a new query, and onpageshowso it never lingers after a bfcache restore.Language filter β 20 defaults + full OL catalogue
DEFAULT_LANGUAGE_OPTIONS(20 curated languages) shown instantly on first open_loadAllLanguages()fetches every language in the OL Solr index, merges with static labels, deduplicates and sorts alphabetically/searchvia session storageBackend β modal results aligned with
/searchThe modal's availability params now map to the same Solr query the
/searchpage uses, so the two agree. Includes anchoring the negatededitions.fqso "Borrowable" returns results, and acceptingpublic_scan/print_disabledon/search.json.WCAG 1.3.1 fix β
<ol-options-popover>role="radiogroup"was on the<ul>directly, stripping list semantics and making<li>children invalid in the accessibility tree (flagged by accesslint on #12690). Fixed by moving the role to a wrapping<div role="radiogroup">and keeping the<ul>a pure list.Shared component infra
FocusableHostMixin/focus-utils.jscentralize shadow-DOM focus-trap discovery so the dialog, popover, and filter components share one robust implementation (with unit tests).Design page
/developers/designdocuments the components:ol-dialog,ol-options-popover, and the popovers. The options-popover demos were simplified to neutral sort/genre examples so they no longer mimic the (separate) availability filter.Screenshot
Moving target, see it on testing instead.
Stakeholders
@lokesh