preactjs / preact-router

:earth_americas: URL router for Preact.
http://npm.im/preact-router
MIT License
1.01k stars 154 forks source link

Problems with link anchors #260

Open trsh opened 6 years ago

trsh commented 6 years ago

Whenever I click on a anchor link (for example 'href="#123"'), this

addEventListener('popstate', function () {
                routeTo(getCurrentUrl());
            });

fires and reloads the according route component, what some-have screws up the anchor and anyways is redundant.

trsh commented 6 years ago

Maybe a option for route component, like <Features path="/features" ignoreHashChange="true"> ?

trsh commented 6 years ago

If anyone run is same problem, a quick dirty workaround is like:

xxx(e){
     e.preventDefault();
      document.getElementById("'yyy").scrollIntoView();
    }
marlun78 commented 6 years ago

Sorry @trsh for hijacking your issue, but this is related.

I’m also having troubles with anchors. When clicking an anchor link the components re-renders. But if I connect a route component to a Redux store and then click the anchor link, the components are remounted! This is not what I want, I want the browser to handle the anchors the “normal” way, my app shouldn’t care. Any ideas how to solve this? Do I need to create a custom history?

Here is a demo: http://jsbin.com/pulajoqusa/1/edit?js,console,output

trsh commented 6 years ago

@marlun78 I think it's kind of bug/'missing feature' to handle anchors native. Duno!.. For now solutions I see are: a) Yes, custom history handler b) Do the hack I wrote in comments above/below

p.s please leave the demo as it is, it might be a starting point for preact devs to fix this

trsh commented 6 years ago
jumpTo(id, e){
     e.preventDefault();
      document.getElementById(id).scrollIntoView();
    }

<a href="#" onClick={this.jumpTo.bind(this, 'id')}>Anchor link</a>

More of the workaround example.

trsh commented 6 years ago

But yeah, if you actually want the hash to change, then you are doomed for custom history handler :D

developit commented 6 years ago

Hmm - no need for custom code here really - you can use the native prop to bypass preact-router for any link, including anchors. It'll just use the browser's default behavior:

<a href="#" native>Anchor link</a>
marlun78 commented 6 years ago

Thanks for input @developit! But the native attribute only prevents the click handler to run and the route() and setUrl() to be called. It doesn’t prevent routeTo() to be called from the popstate handler. Which in turn leads to instance.setState() and instance.forceUpdate(). Is there a way to prevent this?

trsh commented 6 years ago

@developit you should check the whole conversation. Native doesn't help here, as the re-route is fired from 'popstate' event.

developit commented 6 years ago

Ah, I didn't realize that would fire popstate. Perhaps changing Router's routing logic ignore the URL hash would fix this since the routeTo() would be a no-op?

marlun78 commented 6 years ago

@developit We are battle testing a fix in our fork. If successful, I’ll open a PR. I added the following line as the first line in the Router’s routeTo-method:

    /** Re-render children with a new URL to match against. */
    routeTo(url) {
        // marlun78: if url is unchanged or only the hash fragment changed, skip update
        if (typeof url!='string' || url.charAt(0)=='#' || this.state.url==url.replace(/#.*$/, '')) return false;

        this._didRoute = false;
        this.setState({ url });
studentIvan commented 6 years ago

If #265 PR be applied you will able to write your own pop state trigger

import { Router as PreactRouter, customHistory, getCurrentUrl, routeTo, delegateLinkHandler, setCustomHistory } from 'preact-router';

let eventListenersInitialized = false;

function initEventListeners() {
  console.log('local initEventListeners initialized');
  if (eventListenersInitialized) return;

  if (typeof window.addEventListener === 'function') {
    if (!customHistory) {
      window.addEventListener('popstate', () => {
        console.log('this is function popstate event');
        routeTo(getCurrentUrl());
      });
    }
    window.addEventListener('click', delegateLinkHandler);
  }
  eventListenersInitialized = true;
}

/* eslint no-underscore-dangle: "off" */

export default class ExtendedPreactRouter extends PreactRouter {
  constructor(props) {
    super(props);
    if (props.history) {
      setCustomHistory(props.history);
    }

    this.state = {
      url: props.url || getCurrentUrl(),
    };

    initEventListeners();
  }

  routeTo(url) {
    this._didRoute = false;
    this.setState({ url });
    console.log('here we are setting the url');

    // if we're in the middle of an update, don't synchronously re-route.
    if (this.updating) return this.canRoute(url);

    this.forceUpdate();
    return this._didRoute;
  }
}
developit commented 6 years ago

@studentIvan you can already do that using a custom history, since it lets you specify your own listen() observer.

developit commented 6 years ago

hey @marlun78 - are you able to PR your fix here? I'd be happy to merge.

zhenyanghua commented 3 years ago

Before the fix is made in a PR, here is what I did to work around it - the core is DOM manipulation, not tied to any library. Hope it helps. But if you have shadow DOM with anchor, you need to write a custom query selector to walk the available shadow roots of the entire DOM to find the element.

Add the following manual scroll to any page.

export function useAnchor() {
  useEffect(() => {
    let scrollTop = 0;
    if (location.hash) {
      scrollTop = document.querySelector(location.hash).offsetTop;
    }
    window.scrollTo({ top: scrollTop });
  }, []);
}