feat(shop): animation polish, sticky filter bar & custom dropdowns#898
feat(shop): animation polish, sticky filter bar & custom dropdowns#898
Conversation
- Fix ProductDrawer open artifact: use derived-state pattern so displayHandle updates synchronously (no empty-frame flash on first open) - Fix ProductDrawer close glitch: always set inline width so the panel doesn't snap to min-width while sliding out - Delay drawer open animation until product data is pre-fetched, so it never opens on a loading skeleton - Replace native <select> with a fully themed custom ShopSelect dropdown that respects shop-scope light/dark tokens, with keyboard navigation - Switch ShopTab to twMerge so className overrides compose correctly - Add sticky filter + sort bar to shop index that stays visible on scroll, with frosted-glass background and smaller compact sizing - Product card hover background is #EFEFE3 in light mode, shop-bg-2 in dark Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
✅ Deploy Preview for tanstack ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughThis PR refactors the shop page UI by replacing a native select element with a custom accessible dropdown component, enhancing class name composition, reorganizing the page layout with a sticky filter/sort bar, improving product drawer animation state management with prefetch gating, and updating product card hover styling for light-mode contrast. ChangesShop Page UI and Animation Refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/components/shop/ProductDrawer.tsx (1)
169-173: 💤 Low value
displayHandle ?? ''— prefer passingdisplayHandledirectly to the query key.TanStack Query serializes
nullcorrectly in query keys. The''fallback adds a phantom cache entry['shopify', 'product', '']on every render wheredisplayHandleis null (harmless becauseenabled: falseblocks execution, but semantically misleading).🔧 Optional cleanup
- queryKey: ['shopify', 'product', displayHandle ?? ''], - queryFn: () => getProduct({ data: { handle: displayHandle! } }), + queryKey: ['shopify', 'product', displayHandle], + queryFn: () => getProduct({ data: { handle: displayHandle! } }),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/shop/ProductDrawer.tsx` around lines 169 - 173, The queryKey for the useQuery call should use displayHandle directly instead of the empty-string fallback to avoid creating a phantom ['shopify','product',''] cache entry; update the queryKey from ['shopify', 'product', displayHandle ?? ''] to ['shopify', 'product', displayHandle] and keep enabled: !!displayHandle (and the queryFn getProduct({ data: { handle: displayHandle! } }) as-is) so the query only runs when displayHandle is non-null.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/shop/ProductDrawer.tsx`:
- Line 177: The scrim is currently tied to isAnimatedOpen (const isAnimatedOpen
= isOpen && !!prefetchedProduct) causing no immediate click feedback when
prefetchedProduct is null; change the rendering so the scrim uses isOpen (show
scrim immediately when isOpen is true) while keeping the drawer slide transition
gated by isAnimatedOpen (only apply the translate-x-full → translate-x-0
transition/class when isAnimatedOpen is true). Update any conditional
rendering/className logic that references isAnimatedOpen for the scrim to use
isOpen instead and keep isAnimatedOpen for the drawer element (the element that
toggles translate-x-full/translate-x-0).
In `@src/components/shop/ui/Select.tsx`:
- Line 97: The onKeyDown handler is currently attached to a non-interactive
wrapper div in the Select component (the element using twMerge('relative',
className)), causing an a11y/lint error; move the onKeyDown binding off that
static wrapper and attach it to the interactive elements instead (the
trigger/button element that opens the dropdown and/or the dropdown/menu listbox
element used by this Select) so keyboard events are handled on interactive
controls; update references to the onKeyDown prop/handler used in this component
(and the similar occurrences around the menu/trigger logic later in the file) to
ensure the same behavior is preserved while satisfying
jsx-a11y/no-static-element-interactions.
In `@src/routes/shop.index.tsx`:
- Line 130: Replace the invalid Tailwind spacing classes pb-5.5 and gap-y-5.5
with valid values: either use arbitrary-value notation pb-[1.375rem] and
gap-y-[1.375rem] where those classes appear in the JSX (e.g., the div with
className containing "pb-5.5" and the container using "gap-y-5.5"), or
alternatively add a --spacing-5.5 entry in your `@theme` block and use a named
spacing token; update the className strings accordingly so the padding and gap
render correctly.
---
Nitpick comments:
In `@src/components/shop/ProductDrawer.tsx`:
- Around line 169-173: The queryKey for the useQuery call should use
displayHandle directly instead of the empty-string fallback to avoid creating a
phantom ['shopify','product',''] cache entry; update the queryKey from
['shopify', 'product', displayHandle ?? ''] to ['shopify', 'product',
displayHandle] and keep enabled: !!displayHandle (and the queryFn getProduct({
data: { handle: displayHandle! } }) as-is) so the query only runs when
displayHandle is non-null.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: aa223b01-44bc-43b8-a522-c234a4daa1fe
📒 Files selected for processing (5)
src/components/shop/ProductCard.tsxsrc/components/shop/ProductDrawer.tsxsrc/components/shop/ui/Select.tsxsrc/components/shop/ui/Tab.tsxsrc/routes/shop.index.tsx
| }) | ||
|
|
||
| // Delay the open animation until data is in cache. Close animates immediately. | ||
| const isAnimatedOpen = isOpen && !!prefetchedProduct |
There was a problem hiding this comment.
Scrim gated on isAnimatedOpen leaves users with zero click feedback on slow connections.
isAnimatedOpen = isOpen && !!prefetchedProduct means that while data is fetching (cold cache), both the scrim and the drawer stay fully invisible. A user clicking a product card sees nothing happen for the entire round-trip duration — a regression compared to the previous skeleton approach.
The scrim dimming the background is itself a cheap, immediate affordance that doesn't require product data. Only the drawer slide needs to wait for isAnimatedOpen. A simple split:
💡 Proposed fix — show scrim immediately, gate only the drawer on `isAnimatedOpen`
- tabIndex={isAnimatedOpen ? 0 : -1}
+ tabIndex={isOpen ? 0 : -1}
onClick={onClose}
className={twMerge(
'fixed inset-0 z-[60] bg-black/50 backdrop-blur-sm transition-opacity duration-300',
- isAnimatedOpen
+ isOpen
? 'opacity-100 pointer-events-auto'
: 'opacity-0 pointer-events-none',
)}The drawer's translate-x-full → translate-x-0 transition still only fires when isAnimatedOpen is true, so the drawer slides in with content already rendered and no skeleton flash.
Also applies to: 241-251
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/shop/ProductDrawer.tsx` at line 177, The scrim is currently
tied to isAnimatedOpen (const isAnimatedOpen = isOpen && !!prefetchedProduct)
causing no immediate click feedback when prefetchedProduct is null; change the
rendering so the scrim uses isOpen (show scrim immediately when isOpen is true)
while keeping the drawer slide transition gated by isAnimatedOpen (only apply
the translate-x-full → translate-x-0 transition/class when isAnimatedOpen is
true). Update any conditional rendering/className logic that references
isAnimatedOpen for the scrim to use isOpen instead and keep isAnimatedOpen for
the drawer element (the element that toggles translate-x-full/translate-x-0).
| } | ||
|
|
||
| return ( | ||
| <div className={twMerge('relative', className)} onKeyDown={onKeyDown}> |
There was a problem hiding this comment.
Move keyboard handler off the static wrapper to fix a11y/lint blocker.
At Line 97, onKeyDown is attached to a non-interactive <div>, which triggers jsx-a11y/no-static-element-interactions and can block CI. Bind the handler to interactive elements (trigger/menu) instead.
Suggested fix
- <div className={twMerge('relative', className)} onKeyDown={onKeyDown}>
+ <div className={twMerge('relative', className)}>
<button
ref={triggerRef}
type="button"
+ onKeyDown={onKeyDown}
aria-haspopup="listbox"
aria-expanded={open}
@@
{open ? (
<div
ref={menuRef}
role="listbox"
+ onKeyDown={onKeyDown}
className={twMerge(Also applies to: 138-140
🧰 Tools
🪛 GitHub Check: PR
[failure] 97-97: eslint-plugin-jsx-a11y(no-static-element-interactions)
Static HTML elements with event handlers require a role.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/shop/ui/Select.tsx` at line 97, The onKeyDown handler is
currently attached to a non-interactive wrapper div in the Select component (the
element using twMerge('relative', className)), causing an a11y/lint error; move
the onKeyDown binding off that static wrapper and attach it to the interactive
elements instead (the trigger/button element that opens the dropdown and/or the
dropdown/menu listbox element used by this Select) so keyboard events are
handled on interactive controls; update references to the onKeyDown prop/handler
used in this component (and the similar occurrences around the menu/trigger
logic later in the file) to ensure the same behavior is preserved while
satisfying jsx-a11y/no-static-element-interactions.
| <div className="pb-24"> | ||
| {/* Hero */} | ||
| <div className="px-6 md:px-11 pt-6 md:pt-11 max-w-[1280px] mx-auto"> | ||
| <div className="pb-5.5 border-b border-shop-line-2 mb-7"> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check whether the Tailwind config defines a '5.5' spacing token.
# Locate tailwind config files
fd -e cjs -e ts -e js -e mjs 'tailwind.config' | xargs grep -n "'5.5'\|\"5.5\"\|5\.5" 2>/dev/null
# Also check for any explicit spacing extension
fd -e cjs -e ts -e js -e mjs 'tailwind.config' | xargs grep -A 40 'extend' 2>/dev/null | grep -A 5 'spacing'Repository: TanStack/tanstack.com
Length of output: 47
🏁 Script executed:
# First, locate the tailwind config file
fd 'tailwind' --type fRepository: TanStack/tanstack.com
Length of output: 47
🏁 Script executed:
# Also check common config locations
ls -la tailwind.config.* 2>/dev/null || echo "No tailwind config in root"
find . -maxdepth 2 -name 'tailwind.config.*' -type fRepository: TanStack/tanstack.com
Length of output: 92
🏁 Script executed:
# Check the file mentioned in the review
head -n 150 src/routes/shop.index.tsx | tail -n 30Repository: TanStack/tanstack.com
Length of output: 1264
🏁 Script executed:
# Search more broadly for tailwind config anywhere in repo
find . -name 'tailwind.config*' -o -name 'tailwind.js' 2>/dev/nullRepository: TanStack/tanstack.com
Length of output: 47
🏁 Script executed:
# Check if config is in a packages directory
find ./packages -name 'tailwind.config*' 2>/dev/null | head -20Repository: TanStack/tanstack.com
Length of output: 47
🏁 Script executed:
# Check the shop.index.tsx file for line 207
sed -n '200,215p' src/routes/shop.index.tsxRepository: TanStack/tanstack.com
Length of output: 707
🏁 Script executed:
# Search for any references to spacing in config or package.json
rg 'spacing.*5\.5|5\.5.*spacing' --type ts --type js --type jsonRepository: TanStack/tanstack.com
Length of output: 47
🏁 Script executed:
# Check package.json for tailwind config
cat package.json | grep -A 20 -B 5 'tailwind'Repository: TanStack/tanstack.com
Length of output: 2609
🏁 Script executed:
# Look for any config directory structure
ls -la config/ 2>/dev/null || ls -la configs/ 2>/dev/null || echo "No config dir"Repository: TanStack/tanstack.com
Length of output: 79
🏁 Script executed:
# Search the entire repo for where theme or spacing might be defined
rg 'theme.*extend|extend.*theme' --type ts --type js --type json -A 10 | head -50Repository: TanStack/tanstack.com
Length of output: 47
🏁 Script executed:
# Check if tailwind.config is in the root of any package
find . -maxdepth 3 -name '*.config.*' | grep -i tailRepository: TanStack/tanstack.com
Length of output: 47
🏁 Script executed:
# Look for any CSS files that might define these spacing values
rg 'pb-5\.5|gap-y-5\.5' --type css --type scssRepository: TanStack/tanstack.com
Length of output: 94
🏁 Script executed:
# Check vite config for tailwind configuration
find . -name 'vite.config.*' -type f | head -5Repository: TanStack/tanstack.com
Length of output: 82
🏁 Script executed:
# Look for CSS/SCSS files that might have tailwind config
find . -name '*.css' -o -name '*.scss' | grep -E '(global|main|tailwind|style)' | head -10Repository: TanStack/tanstack.com
Length of output: 108
🏁 Script executed:
# Search for CSS files in the source directory
fd -e css -e scss src/ 2>/dev/null | head -20Repository: TanStack/tanstack.com
Length of output: 47
🏁 Script executed:
# Check if vite.config.ts exists and look for tailwind config
cat vite.config.ts 2>/dev/null | head -100Repository: TanStack/tanstack.com
Length of output: 3678
🏁 Script executed:
# Read the main CSS file
cat src/styles/app.cssRepository: TanStack/tanstack.com
Length of output: 27722
🏁 Script executed:
# Read the shop CSS file
cat src/styles/shop.cssRepository: TanStack/tanstack.com
Length of output: 3072
pb-5.5 and gap-y-5.5 are undefined spacing values; these classes produce no CSS.
In Tailwind v4, spacing values must be explicitly defined in the @theme block. The values 5.5 do not exist in the theme (checked app.css and shop.css), so both pb-5.5 on line 130 and gap-y-5.5 on line 207 silently produce no padding/gap, causing layout regression. Use arbitrary-value notation instead: pb-[1.375rem] and gap-y-[1.375rem] (or define --spacing-5.5 in the @theme block if this spacing value should be reusable).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/routes/shop.index.tsx` at line 130, Replace the invalid Tailwind spacing
classes pb-5.5 and gap-y-5.5 with valid values: either use arbitrary-value
notation pb-[1.375rem] and gap-y-[1.375rem] where those classes appear in the
JSX (e.g., the div with className containing "pb-5.5" and the container using
"gap-y-5.5"), or alternatively add a --spacing-5.5 entry in your `@theme` block
and use a named spacing token; update the className strings accordingly so the
padding and gap render correctly.
1 participant
Summary
displayHandlenow updates synchronously via React's derived-state pattern instead ofuseEffect, eliminating the empty-frame on first click; (2) inlinewidthis now always set so the panel doesn't snap tomin-widthwhile sliding out on closeShopSelectdropdown: Replaced the native browser<select>with a fully themed custom dropdown that respectsshop-scopelight/dark tokens, includes keyboard navigation (arrows, Enter, Escape), and has an accent dot indicator for the active option. SameonChangeAPI so call-sites are unchangedShopTab→twMerge: Switched from string concatenation totwMergesoclassNameoverrides compose correctlybg-shop-bg/95 backdrop-blur-sm). Inputs are compact-sized in the bar#EFEFE3(darker than page bg); dark mode keeps the originalshop-bg-2tokenReviewer notes
.env.localfile (Shopify tokens) is gitignored and not includedShopSelectparses<option>children viaReact.Childrenso existing call-sites inshop.index.tsxandshop.collections.$handle.tsxrequire no changes['shopify', 'product', handle]query key asDrawerBody, so React Query serves from cache with no duplicate requestsSummary by CodeRabbit
Bug Fixes
Style
Refactor