ArchiveLabs / ArchiveLabs/openlibrary-components
refactor: split ol-search-bar into two focused components
- 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
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