ArchiveLabs / ArchiveLabs/openlibrary-components

refactor: extract shared ol-facet-bar component from ol-search-bar and ol-search-page

Open
#30 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
0
Forks
0
PR merge metrics
No merged PRs in 30d

Description

## Problem

`ol-search-bar` has `_renderFacetBar()` with `.pf-*` CSS class names.
`ol-search-page` has `_renderFilterBar()` with `.rf-*` CSS class names.

They do the same thing: six facet buttons with auto-alignment, `ol-facet-drop` children, active-state badges, and a cog/help button. Changes to facet behaviour (alignment logic, ARIA, new facet types) must be made in both places.

## Proposed approach

Extract a new `ol-facet-bar` LitElement that:
- Accepts a `facets` array prop (name, label, active-count, right-align flag)
- Renders the button row and owns the `.pf-bar` / dropdown coordination
- Fires `ol-facet-toggle` and `ol-facet-change` events upward
- Owns the `.pf-bar overflow: visible` mobile fix internally

Both `ol-search-bar` and `ol-search-page` import and render ``, removing ~80 lines of duplicated template logic each.

## Tests needed

- Unit: `ol-facet-bar` renders correct number of buttons
- Unit: first-half buttons have no `[right]` attr, second-half do
- Unit: active-count badge appears when filter is set
- Playwright: existing facet-alignment tests in `mobile-facet-layout.spec.js` continue to pass unchanged (they test the rendered output, not the implementation)

## Risk

Low–medium. The refactor is purely internal — external event names and prop shapes don't change. The main risk is the CSS class rename (`.pf-*` → shared names); any consumer CSS targeting those classes would break.

**Best done after the component split so the shared component has a clean home.**

Contributor guide

Open the contributing guide

Research direction

Start with _renderFacetBar() in ol-search-bar and _renderFilterBar() in ol-search-page, then compare their duplicated rendering and coordination logic. Implement the shared ol-facet-bar around the listed props and events, and run the unit tests plus mobile-facet-layout.spec.js. Done means both components use the shared element, the specified unit cases pass, and existing facet-alignment tests remain unchanged and passing.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
frontend
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.