chore: scroll should not trigger press - #3695
Conversation
📊 Code Quality Score: 24/100
Was this score accurate? 👍 Yes · 👎 No Scored by GitVelocity · How are scores calculated? |
There was a problem hiding this comment.
🟢 Approval recommended
The behavior change is localized, matches the stated requirements, and is supported by focused tests covering key timing and gesture edge cases.
Pull request overview
This PR refines mobile-web interaction on the search results page by preventing “pressed/highlight” feedback from appearing during scroll gestures, while still ensuring quick taps get visible feedback.
Changes:
- Reworked
SearchResultCardtouch handling to delay showing the pressed state, cancel it on scroll-like movement, and enforce a minimum visible pressed duration for fast taps. - Updated styling so the pressed visual matches the intended spec (shadow-only), gates
:hoverto true-hover devices, and disables the default mobile tap highlight. - Added Jest/jsdom coverage for tap vs scroll vs cancel behaviors around the pressed state timing.
File summaries
| File | Description |
|---|---|
| static/js/SearchResultCard.jsx | Implements timer-based touch press detection to avoid scroll-triggered pressed state while keeping tap feedback. |
| static/js/tests/searchResultCardPressState.test.js | Adds automated coverage for press-state timing and scroll/tap/cancel edge cases. |
| static/css/s2.css | Aligns pressed/hover visuals and suppresses native tap highlight to match the intended UI feedback. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| .searchResultCard:hover { | ||
| /* Hover and pressed are one and the same state in the spec: a darker shadow, no change of | ||
| background. `.is-pressed` is set by usePressState in SearchResultCard.jsx for taps only. */ | ||
| .searchResultCard.is-pressed { |
There was a problem hiding this comment.
Re: the CSS changes here — worth calling out for reviewers who might read .searchResultCard.is-pressed and :hover as two different visual treatments being merged into one by accident.
They're not accidentally the same — the spec calls for hover (mouse) and pressed (touch tap) to look identical: same box-shadow, no background-color change. Previously :hover gave a shadow and .is-pressed gave a grey background — two different effects for what the design treats as one state. This PR unifies the visual style and keeps two separate triggers for it:
.is-pressed— set by JS (usePressStateinSearchResultCard.jsx) for touch only.:hover— native CSS, now wrapped in@media (hover: hover)so touch devices (which lack a real hover concept but still fire:hoverand can get it stuck on the last-tapped element) never match this rule.
So mouse users get the shadow via :hover, touch users get the identical shadow via .is-pressed, and neither path can leak into the other device type.
— Claude
Description
This PR introduces several UI improvements to the search results page in mobile web. The main one is that when users scroll, they first have to click, but we don't want any click behavior (i.e. the card gets highlighted upon being clicked) to trigger if they're scrolling.
Code Changes
The solution uses timers:
Two timers in [SearchResultCard.jsx:19-84]:
showTimer— delays turning the highlight on (100ms). Starts on touchstart. If the finger moves >10px first, the timer is cancelled and the card never lights up. That's what stops scrolls from highlighting cards.hideTimer— delays turning the highlight off (150ms floor). Set on touchend. Without it, a fast tap would show and hide the highlight in a few milliseconds — it'd be invisible. This timer keeps the highlight on screen for at least 150ms from when it appeared.The edge case they cover together: if the finger lifts before 100ms, then showTimer hasn't fired yet, so
onEndshows the highlight immediately, thenhideTimerholds it 150ms.The two timers pull in opposite directions on purpose: showTimer says don't light up too eagerly (that's the scroll fix), and hideTimer says once you have lit up, stay long enough to be seen (that's the fast-tap fix).