feat(header): mobile-optimized search bar with OlFacetSelect + OlSearchBar LIT components#12757
Open
mekarpeles wants to merge 18 commits into
Open
feat(header): mobile-optimized search bar with OlFacetSelect + OlSearchBar LIT components#12757mekarpeles wants to merge 18 commits into
mekarpeles wants to merge 18 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR migrates the header search UI from the legacy server-rendered/native select markup to Lit-based ol-search-bar and ol-facet-select components, with supporting bridge logic, styles, and E2E coverage.
Changes:
- Adds
OlSearchBarandOlFacetSelectLit components and exports them through the Lit component bundle. - Updates the header template and
SearchBar.jsto use the new component path while retaining legacy fallback logic. - Adds Playwright E2E setup/tests and adjusts popover/select behavior for the new search UX.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
openlibrary/templates/lib/nav_head.html |
Replaces legacy header search markup with <ol-search-bar>. |
openlibrary/plugins/openlibrary/js/SearchBar.js |
Adds Lit-component detection and event-driven facet handling. |
openlibrary/components/lit/OlSearchBar.js |
Adds the new light-DOM header search component. |
openlibrary/components/lit/OlFacetSelect.js |
Adds the new single-select popover facet selector. |
openlibrary/components/lit/OlPopover.js |
Routes trigger-close behavior through the close request event path. |
openlibrary/components/lit/OlSelectPopover.js |
Adjusts trigger chevron order and mobile autofocus behavior. |
openlibrary/components/lit/index.js |
Exports the new Lit components. |
static/css/components/header-bar.css |
Adds header CSS integration for the new components. |
tests/e2e/header-search.spec.mjs |
Adds Playwright coverage for desktop/mobile header search behavior. |
playwright.config.mjs |
Adds Playwright configuration. |
package.json |
Adds Playwright dependency and test:e2e script. |
package-lock.json |
Locks Playwright dependencies. |
Comments suppressed due to low confidence (4)
tests/e2e/header-search.spec.mjs:62
ol-popoveris the host containing the trigger, so it can be visible even when the panel did not open. This assertion does not verify the click opened the popover; assert the open state or visible option panel/content instead.
// Popover panel with options should be visible
const panel = page.locator('ol-popover').first();
await expect(panel).toBeVisible();
tests/e2e/header-search.spec.mjs:108
- Catching the timeout and continuing means this test passes even when autocomplete never renders any results. Add an expectation for visible result items so the test actually covers the autocomplete behavior described by its name.
// Wait for autocomplete results to appear (debounced at 500ms)
await page.waitForSelector('header#header-bar .search-results li', { timeout: 5000 }).catch(() => null);
await page.screenshot({ path: screenshotPath('05-autocomplete-results'), clip: { x: 0, y: 0, width: 600, height: 400 } });
openlibrary/components/lit/OlSearchBar.js:120
- These placeholder/ARIA strings are hard-coded in English, whereas the previous header template translated the same user-facing labels. This will expose English text in localized UI and screen-reader labels; make them configurable and pass translated values from
nav_head.html.
placeholder="Search"
aria-label="Search"
tests/e2e/header-search.spec.mjs:130
- This test only clicks and waits, so it will pass even if the search bar never expands. Assert the expanded class and/or that the input/facet controls become visible after the click.
// Click the search area to expand it
const searchComponent = page.locator('header#header-bar .search-component').first();
await searchComponent.click();
await page.waitForTimeout(300);
cd72009 to
5e4ba51
Compare
5e4ba51 to
fb8f4bb
Compare
…chBar LIT components
Swaps the legacy jQuery-driven header search bar for two new Lit web
components while keeping full backward compatibility and an easy
one-line rollback path.
New components
- OlFacetSelect (ol-facet-select): single-select popover built on
OlPopover; fires ol-facet-select-change; keyboard ArrowUp/Down/Home/End
- OlSearchBar (ol-search-bar): light-DOM wrapper that renders the full
.search-bar-component block so existing jQuery selectors and global CSS
continue to work without modification
SearchBar.js bridge
- Detects <ol-search-bar> and switches to event-driven path
- Listens for ol-facet-change instead of native <select> change
- Skips initCollapsibleMode (handled by OlSearchBar internally)
- handleFacetValueChange uses setAttribute for pre-upgrade safety
CSS (header-bar.css)
- ol-search-bar { display: contents } — transparent to layout
- ol-facet-select mirrors legacy .search-facet hide/show behavior:
hidden by default on mobile (<568px), visible when expanded or ≥568px
nav_head.html
- Replaces 36-line .search-bar-component block with single
<ol-search-bar facet="all"></ol-search-bar>
Rollback: revert nav_head.html change; legacy <select> path in
SearchBar.js activates automatically.
Playwright E2E tests (playwright.config.mjs + tests/e2e/header-search.spec.mjs):
- Desktop: baseline, facet popover open/select, search submit, autocomplete
- Mobile: collapsed baseline, tap-to-expand
All 8 tests passing; 388 unit tests passing.
[pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
…bile overlay Replaces the header's native <select> search bar with a full port of the openlibrary-components prototype ol-search-bar (show-facets / droppable mode). New files: - OlSearchBar.js — Lit Shadow DOM component; panel overlay with instant-search book cards, filter chips, facet bar (availability, language, genre, subject, author, sort), and mobile full-screen overlay. OL adaptations: /search.json API, numFound field, subject param for fiction/genres/subjects, relative URLs. - OlFacetDrop.js — rich facet dropdown (port of prototype ol-facet-drop) - OlHowtoModal.js — search help modal with iframe (port of prototype) - search/filters.js, search/breakpoints.js, search/facets.js — shared constants Changed files: - index.js — exports OlFacetDrop, OlHowtoModal, OlSearchBar - nav_head.html — <ol-search-bar show-facets> replaces old search-bar-component HTML - SearchBar.js — early-exit guard when ol-search-bar[show-facets] is present; new component is self-contained so legacy init is skipped - header-bar.css — replaces display:contents with display:block; width:100% so shadow DOM component fills search-component container correctly Verified at 375px (mobile full-screen overlay), 785px (narrow panel), and 1280px (wide panel with full facet bar) via Playwright screenshots. All 388 JS unit tests pass.
…onsistent trigger style - OlFacetDrop rewritten to own its trigger button via OlPopover slot="trigger", matching OlFacetSelect CSS custom properties (--ol-trigger-padding, font-size, etc.) - OlSearchBar._renderFacetBar() replaces old pf-btn + conditional ol-facet-drop rendering with always-present <ol-facet-drop name="genre|subject|author"> elements - Language slot="trigger" button updated to use SVG chevron (matching all other facets) with rotation CSS on ol-select-popover[data-open] - Removed _openFacet, _lastFacetBtn, _toggleFacet, _facetLabel, _isFacetActive, _renderFacetBtn — state now owned by each OlPopover-based component - CSS: combined ol-facet-select and ol-facet-drop pf-wrap rules; removed pf-caret
…ounts+descriptions - Add triggerLabel prop to OlFacetSelect; when set, trigger button always shows that label regardless of selected value - Add count/subParts rendering to option list items (styled count badge + secondary description line with optional hyperlinks) - Widen panel min-width to 220px for the richer option layout - Pass trigger-label="Availability" and full AVAILABILITY_OPTIONS (with staticCount→count and subParts) from OlSearchBar
…Genre
OlSelectPopover has :host{display:inline-block} while OlFacetDrop/Select
use inline-flex. A flex-stretched inline-block item's cross-size is not
treated as definite by Chromium for child height:100% resolution, so the
slotted Language trigger was 27px (natural) instead of 32px (bar height).
Fix two-part:
- OlPopover: add --ol-popover-trigger-align custom property (default: center)
so callers can switch align-items from outside without editing internals
- OlSearchBar: override ol-select-popover display to inline-flex + align-items
stretch (makes cross-size definite for % resolution), and set the custom
property so the inner ol-popover also uses stretch
All three facet trigger bottoms now land at the same y position.
…xt-align) The OL page wraps ol-search-bar in .search-component which has text-align:right in global CSS. That inherits through the shadow DOM into ol-select-popover's panel, right-aligning the language labels. Adding text-align:left to :host in OlSearchBar resets the inherited value at the shadow root boundary.
…cet popovers - OlFacetDrop: change .footer justify-content from flex-end to center; add width:100% and text-align:center to .clear for full-width hit target - OlSelectPopover: add width:100% and text-align:center to .clear-button - OlSearchBar: change clear-label from "Clear" to "Clear selections" to match the genre/author/subject clear button label
… scrollbar - Move overflow-x:auto + scrollbar-width:none from @media(max-width:600px) to the base .pf-bar rule so the bar can always scroll when content is wider than the panel (e.g. narrow panels or future label additions) - Add min-width:max-content to .pf-wrap so buttons don't shrink below their natural text width — the bar scrolls instead of truncating to ellipsis - Remove the @media(max-width:600px) pf-bar scroll rules (dead code: they were immediately overridden by :host(.mobile-exp) .pf-bar{overflow:visible} at the same breakpoint)
…er in flex context
- OlFacetSelect: call _onClose() after popover.open=false so _isOpen/
data-open stay in sync (setting open=false directly bypasses the
ol-popover-close event that _onClose listens for; chevron would stay
stuck open after selecting an option)
- SearchBar.js: pierce shadow root via shadowRoot.querySelector('.trigger')
instead of jQuery (which cannot reach inside a shadow root); the
Shift+Tab focus path from autocomplete results now correctly reaches
the ol-facet-select trigger
- nav_head.html: add <noscript> fallback form so users with JS disabled
(or a failed bundle) still get a working search
- .gitignore: add tests/e2e/reports/, tests/e2e/screenshots/,
playwright-report/, test-results/ so playwright output never
accidentally gets committed
- header-search.spec.mjs: replace conditional test.skip guards with
hard expect(locator).toBeVisible() assertions; since this PR
introduces ol-facet-select, its absence should be a test failure
- playwright.config.mjs: document that E2E tests are manual-only and
require a running Docker instance
for more information, see https://pre-commit.ci
- Remove dead _olSearchBar jQuery bridge path (only nav_head.html uses ol-search-bar, always with show-facets which early-returns) - Data-drive 3× identical ol-facet-drop blocks via DROPS[] + repeat() - Rename opaque CSS prefixes: pf-* → facet-*, ac-* → suggestion-*, mob-back-* → mobile-header/mobile-back-btn - Extract _renderHeaderMode() and _renderSearchPageMode() from render() so render() is a one-line mode dispatch - Extract named methods _shuffleAuthors, _shuffleSubjects, _openHowto, _closeHowto to replace inline arrow fns in the template - Add Shadow DOM intent note to JSDoc - Fix shadow-root pierce for ol-facet-select trigger focus in SearchBar.js
- Scroll lock: only engage when _mobileExpanded (full-screen overlay), not on every desktop panel open — desktop page was un-scrollable - Mode persistence: add _readMode() reading localStorage 'mode' key; include in both _buildSearchUrl and _fetchAutocomplete so users with 'ebooks' mode aren't silently reset to 'everything' on header search - Suggestion links: remove target="_blank" — autocomplete should navigate in-tab, matching legacy search bar behavior - Gear button: add aria-label="Search help" and type="button" - OlHowtoModal: guard Escape handler on this.open — was firing for every Escape press on the page even when modal was closed - OlHowtoModal: change iframe src to /search/howto (was hard-coded to openlibrary.org, breaking localhost and staging) - Playwright: use single Chromium project; per-describe viewports in the spec already control desktop/mobile — dual projects caused every test to run twice under conflicting device settings - Autocomplete test: replace swallowed .catch(() => null) with a proper expect assertion; update selector from legacy .search-results to shadow-DOM-aware ol-search-bar .suggestion-row
1626bd4 to
01e7604
Compare
Member
Author
|
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.
Summary
Replaces the legacy jQuery/native-
<select>header search bar with two new Lit web components, giving us a popover-based facet selector and cleaner mobile collapse behavior while keeping all existing functionality intact.OlFacetSelect(ol-facet-select) — a new single-select popover primitive built onOlPopover. Sibling toOlSelectPopover(Lokesh's multi-select component from Add ol-select-popover component #12635/refactor: polish OlPopover and OlSelectPopover behavior #12680). Same contract withOlPopover; intentionally minimal — no filter input, no multi-select. Firesol-facet-select-change; keyboard-navigable (ArrowUp/Down/Home/End).OlSearchBar(ol-search-bar) — light-DOM wrapper (no Shadow DOM) that renders the full.search-bar-componentblock. Existing jQuery autocomplete selectors and global CSS continue to work without change. Handles mobile collapse/expand internally (replacesSearchBar.js'sinitCollapsibleMode).SearchBar.jsbridge — detects<ol-search-bar>and switches to an event-driven path; legacy<select>path remains for easy rollback.ol-facet-selectmirrors the legacy.search-facethide/show behavior: hidden on mobile by default, visible when expanded or ≥ 568px.Rollback
Revert
nav_head.htmlto the old<div class="search-bar-component">block — one line — and the legacy path inSearchBar.jsactivates automatically (it checks forol-search-barpresence).Screenshots
Desktop (1280px)
Search bar with new
OlFacetSelecttrigger ("All ▾"), text input, submit, barcode button:Mobile — collapsed (375px)
Only the search icon is visible (logo shows); matches legacy behavior:
Mobile — expanded (after tap)
Full bar: facet selector + input + buttons:
Testing
npm run test:js)npx playwright test)/search?q=..., autocomplete renders after 3+ charsmake lit-components && make js && make cssNotes for Lokesh
OlFacetSelectis designed as a natural complement toOlSelectPopover— sameOlPopoverbase, same event contract, different UX (single-select radio-style vs. multi-select). Happy to move it into the component library proper or adjust the API based on your feedback. The key design decision was NOT to modifyOlSelectPopoveritself, keeping your component's intent intact./cc @hornc @cdrini @internetarchive/openlibrary-maintainers