Skip to content

Wire facet droppers to context-aware counts (frontend for #12978)#13064

Open
Armansiddiqui9 wants to merge 3 commits into
internetarchive:masterfrom
Armansiddiqui9:Search-improvements
Open

Wire facet droppers to context-aware counts (frontend for #12978)#13064
Armansiddiqui9 wants to merge 3 commits into
internetarchive:masterfrom
Armansiddiqui9:Search-improvements

Conversation

@Armansiddiqui9

Copy link
Copy Markdown
Contributor

Closes #13060
Depends on #12980 (backend API, currently WIP — see note below)

Feature
Makes the Language filter on /search reflect the current search instead
of always showing the full catalogue. When a patron has an active query,
opening the Language dropper now shows only languages with matching
results, with a count next to each, sorted by relevance, no more dead-end
selections that silently return zero books.

Technical

  • OlSelectPopover.js — extended item shape to {value, label, count?}.
    Renders count as a muted, comma-formatted trailing number. Added a
    loading attribute for a host-driven spinner state. No fetch or domain
    logic added — stays a pure UI component per the
    building-components guide.
  • search-modal/searchFacets.js (new) — service module owning the
    /search/facets.json call. Exposes fetchFacetCounts(field, searchParams).
  • SearchFilterBar.js — orchestration. On ol-popover-open, fetches
    the language catalogue and facet counts in parallel, merges by MARC code,
    sorts descending, hides zero-count values (keeps the current selection
    visible even at 0 so it can still be unticked). Falls back to the
    uncounted catalogue on error or when there's no active query.
  • searchFacets.test.js (new) — 16 unit tests covering the service
    module's response handling and the merge/sort/hide-zero logic. All passing.

Testing

  • npm run test:js — 483/483 passing (including the 16 new tests)
  • Manual end-to-end test on local Docker dev environment: stubbed
    /search/facets.json (since feat(search): add GET /search/facets.json — context-aware facet values API #12980 is still WIP) and verified against
    the real /languages.json catalogue. Confirmed counts render correctly,
    comma-formatted, sorted descending, zero-count items hidden, loading
    state visible during fetch.
Screenshot 2026-06-30 164935
  • Verified no-query fallback (no facet request fires when q= is empty)
  • Verified graceful fallback to the uncounted list on a rejected fetch

Open question for @RayBB

Confirmed locally that the frontend's language catalogue keys value on
MARC code (eng, fre, ger...) via /languages.json, not the
display name. Please make sure /search/facets.json's value field
returns matching MARC codes, not full language names — otherwise the
merge in SearchFilterBar.js will silently match nothing and the dropper
will render empty. Happy to pair on this if useful.

Stakeholders

@lokesh @RayBB

Armansiddiqui9 and others added 3 commits June 30, 2026 17:29
Extends item shape from {value, label} to {value, label, count?}.
Renders count as a muted, comma-formatted trailing number when present.
Adds host-driven loading boolean attribute, swaps suggestions list for
a spinner while true. No fetch or domain logic added - stays a pure
UI component per the building-components architecture guide.

Part of internetarchive#13060
@openlibrary-bot

Copy link
Copy Markdown
Collaborator

Thanks for the contribution, @Armansiddiqui9!

🤖 Copilot has been assigned for an initial review.

The linked issue (#13060) hasn't been triaged yet — triage happens on Mondays and Fridays. There are currently 27 open non-draft PRs ahead of yours.

PR triage checklist (maintainers / Richy)
  • PR description — not empty; explains what the change does and how to verify it
  • References an issue — PR body contains a #NNN reference
    • Linked issue is triaged — has a Priority: * label (not just Needs: Triage); currently has Needs: Response
    • Linked issue is assigned — has at least one assignee
  • Commit history clean — no WIP/fixup/conflict noise; commit messages are meaningful
  • CI passing — checks still pending
  • Test cases present — 16 new unit tests in searchFacets.test.js
  • Proof of testing — PR body includes a screenshot and a description of what was tested

Note

This comment was automatically generated by PAM, Open Library's Project AI Manager. PAM provides status visibility, performs basic project management functions, and gives actionable feedback so contributors aren't left waiting.

@lokesh

lokesh commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

This is looking great. Once the backend work is complete we'll do one more pass. At that point, if you could record a short video of the feature working, including the new loading indicator, that would be really helpful.

I'm encouraging all of us to add videos to UI-related PRs. They convey a lot about the scope of a feature, get people excited, and give reviewers confidence that the change has actually been tested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(search): Wire facet droppers to context-aware counts (frontend for #12978)

3 participants