Add original click event to 'turbo:click' details#611
Add original click event to 'turbo:click' details#611dhh merged 2 commits intohotwired:mainfrom manuelpuyol:add-click-event-to-turbo-click
Conversation
| const event = this.notifyApplicationAfterClickingLinkToLocation(link, location) | ||
| applicationAllowsFollowingLinkToLocation(link: Element, location: URL, ev: MouseEvent) { | ||
| const event = this.notifyApplicationAfterClickingLinkToLocation(link, location, ev) | ||
| return !event.defaultPrevented |
There was a problem hiding this comment.
As an alternative to dispatching the event with a reference to the original, would explicitly preventing the default if event.defaultPrevented serve the same purpose without expanding the surface area of the CustomEvent.detail?
applicationAllowsFollowingLinkToLocation(link: Element, location: URL, mouseEvent: MouseEvent) {
const event = this.notifyApplicationAfterClickingLinkToLocation(link, location)
if (event.defaultPrevented) {
mouseEvent.preventDefault()
return false
} else {
return true
}
}There was a problem hiding this comment.
I don't think they are the same behavior. We don't always want to prevent the browser click when preventing turbo:click
There was a problem hiding this comment.
the use cases would be:
- I want to prevent Turbo from navigating and let the browser do it -> prevent
turbo:click - I want to prevent all navigations -> prevent both events
There was a problem hiding this comment.
With that in mind, could we make Frame navigation internally aware of the cancellation?
There was a problem hiding this comment.
from my understanding, this is where we trigger the Frame navigation:
turbo/src/core/frames/link_interceptor.ts
Lines 37 to 43 in 98cdc40
this is listening to the turbo:click event, so it should be possible for it to know if the event is defaultPrevented. The problem is that the interceptor runs before any addEventListener I add, so the it always evaluates the event first, and it is never prevented.
maybe we could update
Lines 153 to 159 in 98cdc40
to dispatch another event (if the click isn't prevented) that the LinkInterceptor listens to
There was a problem hiding this comment.
@seanpdoyle would adding another internal event make sense here?
|
Is this related to #610? If it is, could we add a test case that exercises a call to |
No, this is just something I've been thinking about for some time 😅 |
|
@seanpdoyle wdyt of moving forward with only this change and fixing the frame issue in another PR? |
|
Let's get a doc PR going to explain this as well for the 7.2.0 release 👍 |
* main: Allow frames to scroll smoothly into view (hotwired#607) Export Type declarations for `turbo:` events (hotwired#452) Add .php as a valid isHTML extension (hotwired#629) Add original click event to 'turbo:click' details (hotwired#611) Drive Browser tests with `playwright` (hotwired#609) Allow Turbo Streams w/ GET via `data-turbo-stream` (hotwired#612) Only update history when Turbo visit is renderable (hotwired#601) Support development ChromeDriver version overrides (hotwired#606) Turbo stream source (hotwired#415) Expose Frame load state via `[complete]` attribute (hotwired#487) fix(ie/edge): form.method='delete', raises Invalid argument. (hotwired#586) Do not declare global types/constants (hotwired#524) Defensively create custom turbo elements (hotwired#483) Use `replaceChildren` in StreamActions.update (hotwired#534)
Right now, if you
preventDefaultaturbo:click, it will still fallback to the original click, causing a full-page reload. Sending the original click event will allow users to completely cancel a click if they need/want to.