Add collection and search filtering & sorting to skeleton template#3618
Add collection and search filtering & sorting to skeleton template#3618danielqiuu wants to merge 20 commits intomainfrom
Conversation
New features: - Collection filtering with list, swatch, and price range filters - Collection sorting (Featured, Price, Best Selling, Alphabetical, Date) - Search filtering and sorting (Relevance, Price) - CollectionFilters, CollectionSort, PriceRangeFilter components - productFilters, productSort utility libraries Bug fix: - Article search result URLs now correctly include blog handle GraphQL changes: - COLLECTION_QUERY: Added filters, sortKey, reverse variables; returns filter metadata - SEARCH_QUERY: Added productFilters, sortKey, reverse variables; returns productFilters
|
Oxygen deployed a preview of your
Learn more about Hydrogen's GitHub integration. |
product-filters.ts (was productFilters.ts) product-sort.ts (was productSort.ts)
25 tests covering: - parseFiltersFromParams: empty, single, price, multi-select, non-filter, malformed - applyFilter: new, multi-select, dedup, price set, cursor clearing - removeFilter: single, selective, no-op, cursor clearing - parseSortParam: empty, collection, search, invalid, wrong context - applySortParam: set, default removal, cursor clearing, param preservation
Skeleton template has no test infrastructure (no vitest config, no test script). No other skeleton utility has dedicated tests. Test files would also ship to end users who scaffold from this template.
- Added CollectionSort and CollectionFilters to the components table - Added new 'Filtering and sorting' section explaining URL param format - Updated GraphQL query to include productFilters, sortKey, reverse - Updated search fetcher to show filter/sort parsing - Updated SearchPage render to show filter/sort UI - Added 'How to customize filters?' and 'How to add custom sort options?' sections - Fixed component file paths in the components table
Updated 6 patches across 4 recipes to account for new CSS and collections.$handle.tsx changes (filtering/sorting): - bundles: app.css, collections.$handle.tsx - combined-listings: app.css, collections.$handle.tsx - express: app.css - infinite-scroll: collections.$handle.tsx
This reverts commit 875eed1.
This reverts commit ca17409.
Three issues fixed: 1. express/package.json.acbf33.patch: Context lines contained 'catalog:' protocol references which get resolved to actual versions by the validator before patches are applied. Regenerated patch with resolved versions so context matches. 2. express/recipe.yaml: Added new skeleton files (CollectionFilters, CollectionSort, PriceRangeFilter, product-filters, product-sort) to deletedFiles. The express recipe strips the skeleton down to a minimal setup - without deleting these files, typecheck fails because they import CollectionQuery from the regenerated storefrontapi types. 3. infinite-scroll/package.json.acbf33.patch: Same catalog: resolution issue as express. Regenerated with resolved versions.
The package.json patch for infinite-scroll uses catalog: in context lines, matching the unresolved skeleton. This is the same patch that exists on main. The E2E apply command resolves catalog on the original skeleton but applies patches to a copy that retains catalog: literals, so the patch context must use catalog: not resolved versions.
…ate version Restored from main's patch and updated only the version (2026.1.1 → 2026.1.2) and CLI version (3.85.4 → 3.91.1). Context lines must use catalog: and workspace:* since the E2E apply command patches a skeleton copy before catalog resolution happens on it.
fredericoo
left a comment
There was a problem hiding this comment.
this is adding quite a bit of functionality
please add some e2e tests to ensure this works well and allows us to test it besides manually
agents should be able to write most if not all of it
New test files: - e2e/specs/skeleton/collections.spec.ts — 9 tests covering sort dropdown, filter application, clear all, aria-pressed, and filter+sort interaction on collection pages - e2e/specs/skeleton/search.spec.ts — 9 tests covering basic search, sort dropdown, filter application, clear all, search term preservation, and article URL correctness Tests use the hydrogenPreviewStorefront test store and follow existing patterns: accessible selectors (getByRole, getByLabel), URL assertions, and no test-specific data attributes in the skeleton code.
Following the pattern from smoke/home.spec.ts, both test files now verify pages render without console errors. The collections test also asserts product items appear in the grid.
- Remove console error assertions: the inline SVG data URI in the sort dropdown CSS triggers a CSP violation that is not an application error - Fix sort reload test: navigate directly to URL with sort param instead of selecting then reloading (avoids race between client nav and reload) - Fix search button selector: scope to main content area to avoid ambiguity with the header search button - Fix search sort test: use waitForURL instead of toHaveURL to handle async navigation after selectOption
|
|
||
| if (priceParam) { | ||
| try { | ||
| const parsed = JSON.parse(priceParam) as {min?: number; max?: number}; |
There was a problem hiding this comment.
non-blocking: six as casts on JSON.parse results across this file and CollectionFilters.tsx override TypeScript's type inference without runtime validation. If the ProductFilter type or filter input shape changes upstream, these silently pass the wrong shape through.
ideal: use filter.price.min and filter.price.max if possible
same for all the other filter and sorts so we dont rely on json parsing stuff
if too much lift we can leave it like this for now
| @@ -0,0 +1,169 @@ | |||
| import {useNavigate, useLocation} from 'react-router'; | |||
There was a problem hiding this comment.
blocking: this component is also used on the search page (search.tsx imports it), but the name CollectionFilters implies it's collection-specific. Same for CollectionSort.
The skeleton template is a generator - every name here gets copy-pasted into thousands of projects where developers will see CollectionFilters inside search.tsx and be confused. Renaming later is strictly harder (changeset + version bump + migration note in the upgrade guide).
Let's rename to ProductFilters and ProductSort since they're context-agnostic. The CSS classes (.collection-filters, .collection-sort, etc.) and the search guide references should follow too.
| await page.goto(COLLECTION_URL); | ||
|
|
||
| // Click the first available filter option | ||
| const filterButton = page.locator('.collection-filter-option').first(); |
There was a problem hiding this comment.
blocking: these tests use CSS class selectors (.collection-filter-group, .collection-filter-option, .products-grid .product-item) throughout. Our e2e/CLAUDE.md guidelines say "Use role-based locators - Never CSS classes unless absolutely necessary."
The filter buttons already have aria-pressed and aria-label attributes, so getByRole('button', {pressed: false}) or getByRole('button', {name: /products/i}) would work. This also makes the tests fragile to the CSS rename above.
e2e/specs/skeleton/search.spec.ts
Outdated
| }); | ||
|
|
||
| // Only test article links if the store has articles | ||
| if (await articlesSection.isVisible()) { |
There was a problem hiding this comment.
blocking: this test can pass if we break search
because you are using hydrogen.shop as the store domain, you can assume there will be articles there to look at
| const searchParams = new URLSearchParams(location.search); | ||
|
|
||
| try { | ||
| const filter = JSON.parse(filterInput) as ProductFilter; |
There was a problem hiding this comment.
non-blocking: both toggleFilter and isFilterApplied independently parse the filter JSON, extract entries, construct the filter.{key} param key, and JSON-stringify for comparison. This is the same knowledge already encoded in product-filters.ts via FILTER_URL_PREFIX and applyFilter/removeFilter.
An isFilterActive(filterInput: string, searchParams: URLSearchParams): boolean utility in product-filters.ts would eliminate this duplication and keep the component thin.
| ): URLSearchParams { | ||
| const params = new URLSearchParams(searchParams); | ||
|
|
||
| if (sortKey === 'FEATURED' || sortKey === 'RELEVANCE') { |
There was a problem hiding this comment.
non-blocking: this hardcodes 'FEATURED' and 'RELEVANCE' as default sort keys, but the function doesn't know whether it's in a collection or search context. If a collection somehow used RELEVANCE, it would incorrectly be treated as a default.
A cleaner design would accept a defaultKey parameter or have the caller handle default-removal.
| } | ||
| } | ||
|
|
||
| const minNum = min ? parseFloat(min) : undefined; |
There was a problem hiding this comment.
non-blocking: parseFloat(min) and parseFloat(max) here accept NaN, Infinity, and negative numbers. The min="0" HTML attribute on the inputs is advisory only - a user can type -50 or abc. Basic input sanitisation (clamp to zero, reject NaN) would prevent confusing Storefront API errors downstream.
| className={`collection-filter-option${hasSwatches ? ' has-swatch' : ''}`} | ||
| onClick={() => toggleFilter(inputString)} | ||
| style={{ | ||
| border: isApplied ? '2px solid black' : '1px solid #ccc', |
There was a problem hiding this comment.
non-blocking: mixing inline styles for the active border with CSS classes makes the active state harder to discover for developers customising the template. Let's use a CSS selector like .collection-filter-option[aria-pressed="true"] instead - it leverages the existing aria-pressed attribute and keeps all styling in one place.
| const hasValue = min || max; | ||
|
|
||
| return ( | ||
| <div className="price-range-filter" data-max={maxPrice}> |
There was a problem hiding this comment.
question: maxPrice is stored as data-max here but isn't used for input validation. Is this deliberate?
| {articles?.nodes?.map((article) => { | ||
| const articleUrl = urlWithTrackingParams({ | ||
| baseUrl: `/blogs/${article.handle}`, | ||
| baseUrl: `/blogs/${article.blog.handle}/${article.handle}`, |
New features:
Bug fix:
GraphQL changes:
WHY are these changes introduced?
The skeleton template currently has no support for filtering or sorting on collection or search pages. These are fundamental storefront features that nearly every Hydrogen merchant needs, and the Storefront API already supports them via the productFilters, sortKey, and reverse arguments. Without this, developers
building from the skeleton must implement filtering and sorting from scratch.
Additionally, article links in search results were broken — they navigated to /blogs/{articleHandle} instead of /blogs/{blogHandle}/{articleHandle}.
WHAT is this pull request doing?
New components:
New utilities:
Route changes:
GraphQL changes:
Other:
HOW to test your changes?
dev serverfrom the skeleton templatePost-merge steps
Checklist