ArchiveLabs / ArchiveLabs/openlibrary-components

refactor: split ol-search-bar into two focused components

Open
#29 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` is a 950-line component with a `showFacets` prop that selects between two fundamentally different UIs:

- **Embedded mode** (`showFacets=false`): compact input row with chip bar, used inline on the search results page
- **Droppable mode** (`showFacets=true`): trigger button + viewport-fixed overlay panel, used inside `ol-header`

About half the code, CSS, and state only runs in one mode. CSS specificity fights (e.g. `:host(.mobile-exp)` overriding media blocks) exist because styles written for one mode bleed into the other. Every refactor requires mentally tracking which branch you're in.

## Proposed split

| New component | Role | Replaces |
|---|---|---|
| `ol-search-bar` | Embedded input + chip bar only | current `!showFacets` branch |
| `ol-header-search` | Trigger + overlay panel + mobile full-screen | current `showFacets` branch |

`ol-header` would import `ol-header-search`. The embedded `ol-search-bar` stays where it is.

## Migration plan

1. Identify all props, state, and methods that are mode-specific vs. shared
2. Extract shared logic (autocomplete fetch, filter helpers) — already partially done via `utils/facets.js`
3. Create `ol-header-search.js` with just the droppable-mode render path, CSS, and state
4. Update `ol-header.js` to use `ol-header-search`
5. Strip the `showFacets` branch and dead CSS from `ol-search-bar.js`
6. Update all tests to target the correct component

## Tests needed

- Unit: `ol-header-search` panel positioning contract (currently in `ol-search-bar.panel-overlay.test.js`)
- Unit: `ol-header-search` mobile overlay contract (currently in `ol-search-bar.mobile-overlay.test.js`)
- Playwright: existing `mobile-overlay.spec.js` and `facet-and-submit.spec.js` target `ol-header > ol-header-search` instead of `ol-header > ol-search-bar`

## Risk

Medium. The render logic is well-tested, but consumers outside this repo may import `ol-search-bar` expecting droppable mode. Check for external usage before removing the `showFacets` branch.

Contributor guide

Open the contributing guide

Research direction

Start with ol-search-bar.js and map the mode-specific props, state, methods, and CSS, then check for external usage of showFacets. Review utils/facets.js and the existing panel-overlay and mobile-overlay tests before creating ol-header-search.js. Done means ol-header uses the new component, ol-search-bar contains only embedded mode, and the unit and Playwright tests target the correct component.

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
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.