thepassle / app-tools

125 stars 8 forks source link

[router] Clicking on the same link twice causes the page to reload #21

Open Antony92 opened 10 months ago

Antony92 commented 10 months ago

This is happening because _onAnchorClick method has if (this.url.href === url.href) return; Is this intended? I think it is better to just prevent the event in this case:

_onAnchorClick = (e) => {
    if (
      e.defaultPrevented ||
      e.button !== 0 ||
      e.metaKey ||
      e.ctrlKey ||
      e.shiftKey
    ) {
      return;
    }

    const a = e.composedPath().find((el) => el.tagName === 'A');
    if (!a || !a.href) return;

    const url = new URL(a.href);

    // old 
    // if (this.url.href === url.href) return;

    // new
    if (this.url.href === url.href) {
        e.preventDefault();
        return;
    }
    if (url.host !== window.location.host) return;
    if (a.hasAttribute('download') || a.href.includes('mailto:')) return;

    const target = a.getAttribute('target');
    if (target && target !== '' && target !== '_self') return;

    e.preventDefault();
    this.navigate(url);
}
JulianCataldo commented 8 months ago

For an SPA, this can lead to unwanted app state reset. That might not be clear to the user, even if it can be a wanted behaviour in some cases, but I'm not sure it's the majority of them. Maybe having this as an opt-in option?

alemscoder commented 3 months ago

This is happening because _onAnchorClick method has if (this.url.href === url.href) return; Is this intended? I think it is better to just prevent the event in this case:

_onAnchorClick = (e) => {
    if (
      e.defaultPrevented ||
      e.button !== 0 ||
      e.metaKey ||
      e.ctrlKey ||
      e.shiftKey
    ) {
      return;
    }

    const a = e.composedPath().find((el) => el.tagName === 'A');
    if (!a || !a.href) return;

    const url = new URL(a.href);

    // old 
    // if (this.url.href === url.href) return;

    // new
    if (this.url.href === url.href) {
        e.preventDefault();
        return;
    }
    if (url.host !== window.location.host) return;
    if (a.hasAttribute('download') || a.href.includes('mailto:')) return;

    const target = a.getAttribute('target');
    if (target && target !== '' && target !== '_self') return;

    e.preventDefault();
    this.navigate(url);
}

Hello, I recently started using this incredible router and I must comment that I am delighted with the work that the developer @thepassle has done, it really works very well and is super flexible in configurations. I have recently found myself in need of having the same behavior that you needed, that when clicking again on the same Anchor element the page would not reload. The problem is easily solved using the lit-html templates, here is an example of how I did it, I hope this helps anyone who may have this problem in the future. Greetings :)

import { html } from "lit-html"
import router from "../router/router"

export function Logo() {

    const handleAnchorNavigation = (e: MouseEvent) => {
        const anchor = e.currentTarget as HTMLAnchorElement
        const path = anchor.href
        e.preventDefault()
        router.navigate(path)
    }

    const template = html`
        <a @click=${handleAnchorNavigation} href="/">
            <vaadin-icon class="logo" src="/starship.svg"></vaadin-icon>
        </a>

    `

    return html`
        ${template}
    `

}