Skip to content

fix(ui): prevent duplicate history entry when clicking wall items#6829

Open
speckofthecosmos wants to merge 2 commits intostashapp:developfrom
speckofthecosmos:fix/wall-double-push-navigation
Open

fix(ui): prevent duplicate history entry when clicking wall items#6829
speckofthecosmos wants to merge 2 commits intostashapp:developfrom
speckofthecosmos:fix/wall-double-push-navigation

Conversation

@speckofthecosmos
Copy link
Copy Markdown

Summary

Clicking a scene or marker in the wall view pushes two identical history entries, requiring users to press back twice to return to the wall. This affects both desktop and mobile browsers.

Root Cause

react-photo-gallery dispatches the onClick handler twice for a single click event when using a custom renderImage component. The existing codebase already documents this behavior at SceneWallPanel.tsx:99:

"having a click handler here results in multiple calls to handleClick due to having the same click handler on the parent div"

The onClick handler was previously removed from the <img>/<video> elements to avoid this, but the SceneWall and SceneMarkerWall components' onClick callbacks (which call history.push) still receive duplicate dispatches from the Gallery component.

Evidence

Instrumented history.pushState on a running development instance and confirmed:

  • Two PUSH calls to the identical URL within 1ms of each other
  • Click target confirmed as VIDEO element (not the footer <Link>)
  • Both pushes traverse the same call chain: SceneWallItem.handleClick → Gallery.handleClick → SceneWall.onClick → history.push
  • Reproduced consistently across 3 test runs on both desktop Chrome and mobile Safari

Fix

Adds a timestamp-based guard (useRef) to SceneWallPanel and SceneMarkerWallPanel that deduplicates history.push calls within 100ms. The double dispatch occurs within 1ms, so this threshold is safe for legitimate rapid navigation while preventing the duplicate entry.

Files Changed

  • ui/v2.5/src/components/Scenes/SceneWallPanel.tsx — add click dedup guard
  • ui/v2.5/src/components/Scenes/SceneMarkerWallPanel.tsx — same fix + add useRef import

Test plan

  • Open scenes in wall display mode
  • Click any scene — should navigate to scene page
  • Press browser back button — should return to wall (not reload the scene)
  • Repeat with scene marker wall view
  • Verify rapid legitimate clicks (>100ms apart) still work normally

🤖 Generated with Claude Code

Clicking a scene or marker in the wall view pushes two identical history
entries, requiring the user to press back twice to return to the wall.
This is caused by react-photo-gallery dispatching the onClick handler
twice for a single click event.

Add a timestamp-based guard to deduplicate clicks within 100ms.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@speckofthecosmos
Copy link
Copy Markdown
Author

Reproducibility note: This was consistently reproduced on v0.30.1-55-g8dec195c (Feb 2026 development build). After updating to the latest development build (Apr 2026), the symptom appears harder to trigger — possibly due to rendering timing changes — but the underlying code path is unchanged: SceneWallPanel's onClick still has no guard against the duplicate dispatch from react-photo-gallery, and the same unguarded pattern exists in SceneMarkerWallPanel.

The fix is defensive and low-risk regardless — a 100ms click dedup via useRef that has no impact on normal usage.

@feederbox826
Copy link
Copy Markdown
Member

why not use a debounce instead of storing the last click time in the element?

@feederbox826
Copy link
Copy Markdown
Member

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/now#reduced_time_precision This will also break on firefox where the Date.now() precision is 2ms for default reduction and 100 for strict

@slick-daddy
Copy link
Copy Markdown
Contributor

I think the problem is correctly determined however the solution is not optimal. Hardcoding a delay, which is 100ms, doesn't feel natural or correct to me.

@speckofthecosmos
Copy link
Copy Markdown
Author

Yeah, fair. The hardcoded 100ms bugged me too, and @feederbox826 already flagged that Date.now() has precision issues on Firefox, so a time-based threshold probably isn't the right move regardless.

No strong opinions on the shape though. If you've got a pattern you'd rather see (event-identity dedup, debounce, a fix higher up in the Gallery wrapper, whatever), point me at it and I'll rework.

Copy link
Copy Markdown
Collaborator

@Gykes Gykes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, I agree this is more of a bandaid than a fix. Since it's pushing the URL twice a path check may be the best bet and solves all the issues that people have brought up.

Here's a general example. It's untested and from a general look at the page so take it and run with it.

  const onClick = useCallback(
    (event, { index }) => {
      const link = photos[index].link;
      const current = history.location.pathname + history.location.search;
      if (current !== link) {
        history.push(link);
      }
    },
    [history, photos]
  );

@feederbox826 is better with TS than me so perhaps he can have a gander and correct anything there. I can't remember the exact urls that get used either for current so that may need to be changed up too.

…tries

Replace the 100ms timestamp-based dedup with a path-equality check
against history.location. The previous approach relied on Date.now()
precision, which varies by browser (Firefox reduces precision to 2ms
default / 100ms strict per the spec) and introduced an arbitrary
threshold. Since react-router's history.push updates location
synchronously, the second dispatch from react-photo-gallery sees the
post-push URL and the guard correctly no-ops.

Per review feedback on stashapp#6829 from @Gykes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@speckofthecosmos
Copy link
Copy Markdown
Author

Thanks @Gykes — path-equality is the right shape. The timestamp was arbitrary and @feederbox826's Firefox precision note made it a dead end anyway. Pushed the rework to this branch.

Since react-router's history.push updates history.location synchronously, the second dispatch from the Gallery sees the post-push URL and the check no-ops. Works across both SceneWallPanel and SceneMarkerWallPanel. Also covers the case where the scene wall's link contains a query string (via sceneQueue.makeLink) since we compare pathname + search.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants