[chaploud]

Summary

• Html5History takes over an anchor click after checking modifier keys, the mouse button, the target attribute and contenteditable, but it does not look at defaultPrevented. A listener closer to the anchor that cancels the navigation is ignored and the router still pushes a history entry.
• Skip the click in that case. Preventing the default action means the browser would not navigate either, so neither should the router.
• Add a test that clicks a real anchor carrying a preventDefault listener and asserts the location does not change. It fails without the fix.

Why this matters

Without it, a link that should open something else when clicked cannot keep its href:

[:a {:href (rfe/href ::manual {:id 1})
     :on-click (fn [e] (.preventDefault e) (open-sheet!))}
 "Open"]

The sheet opens and the router navigates away at the same time. Dropping the href works around it, but the anchor then stops being focusable, so keyboard users skip it and "open in new tab" disappears from the context menu. The same applies to a navigation item that cancels a click on the entry for the current page: today every click grows the history stack.

Prior art

React Router and Vue Router both skip the click when defaultPrevented is set. With the Navigation API the navigate event is not fired at all for a click whose default action was prevented, so the platform encodes the same rule.

Notes

• The listener is registered on document, so listeners on the anchor run first and defaultPrevented is already set when the predicate runs. goog.events.BrowserEvent copies the flag from the native event when it wraps it, so this also covers handlers that call the native preventDefault, such as React's.
• ./scripts/test.sh cljs passes (73 tests, node-test and both karma runs).

[opqdonut]

now that 0.11.0 is out, this changelog line will land in the wrong place. please merge/rebase master

[opqdonut]

I need the opinion of someone with more frontend knowledge than me to be able to merge this. Can some existing usecases / workarounds for this break?

[Deraen]

The one line change to the click handling condition does make sense. Test case looks ok.

Interestingly, I did look at the React-router logic back in 2019 when this was added (#256) but for some reason I didn't add it then, though it seems react-router checked the defaultPrevented back then. Perhaps I looked at the other cljs routing libs more and they didn't check this.

Anyway, LGTM.

[opqdonut]

fixed the changelog!