Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problems with link anchors #260

Open
trsh opened this issue Jan 10, 2018 · 15 comments
Open

Problems with link anchors #260

trsh opened this issue Jan 10, 2018 · 15 comments

Comments

@trsh
Copy link

trsh commented Jan 10, 2018

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
Copy link
Author

trsh commented Jan 10, 2018

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

@trsh
Copy link
Author

trsh commented Jan 12, 2018

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

xxx(e){
     e.preventDefault();
      document.getElementById("'yyy").scrollIntoView();
    }

@marlun78
Copy link

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
Copy link
Author

trsh commented Jan 12, 2018

@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
Copy link
Author

trsh commented Jan 12, 2018

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
Copy link
Author

trsh commented Jan 12, 2018

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

@developit
Copy link
Member

developit commented Jan 16, 2018

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
Copy link

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
Copy link
Author

trsh commented Jan 17, 2018

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

@developit
Copy link
Member

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
Copy link

@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
Copy link

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
Copy link
Member

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

@developit
Copy link
Member

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

@zhenyanghua
Copy link

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 });
  }, []);
}

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

No branches or pull requests

5 participants