Skip to content
This repository was archived by the owner on Jan 24, 2018. It is now read-only.

Fix for opening url with hash and scrolling webpage to correct place. #8

Open
adamlukaszczyk wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Fix for opening url with hash and scrolling webpage to correct place. #8

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a9b025d
While opening page with hash function handleLink() was fired before s…
Jul 6, 2013
9f56809
Bring back the fake flag.
Jul 6, 2013
26673f3
If popstate doesn't return e.state object skip setScrollTop().
Jul 6, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 26 additions & 21 deletions src/skrollr.menu.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,13 +138,33 @@

if(supportsHistory) {
window.addEventListener('popstate', function(e) {
var state = e.state || {};
var top = state.top || 0;

defer(function() {
_skrollrInstance.setScrollTop(top);
});
if (e.state) {
Copy link
Copy Markdown
Author

@adamlukaszczyk adamlukaszczyk Jul 7, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have much experience with history manipulation and popstate event but I noticed that popstate event in Chrome is raised every time on page load event.

And on page load the e.state is null so then _skrollrInstance.setScrollTop(top); wants to scroll to top of the page (0px).

This is unwanted behavior as few moments before we scrolled down page to the accurate place pointed by the hash in URL.

Just take a look on it in action on my page:
http://smarttvrocket.com/#/pl/smart-tv
http://smarttvrocket.com/#/pl/oferta/mobileapps
http://smarttvrocket.com/#/pl/kontakt

Although I have not test it in Firefox ;)

var state = e.state || {};
var top = state.top || 0;

defer(function() {
_skrollrInstance.setScrollTop(top);
});
}
}, false);

//In case the page was opened with a hash, prevent jumping to it.
//http://stackoverflow.com/questions/3659072/jquery-disable-anchor-jump-when-loading-a-page
defer(function() {
if(window.location.hash) {

window.scrollTo(0, 0);

if(document.querySelector) {
var link = document.querySelector('a[href="' + window.location.hash + '"]');

if(link) {
handleLink(link, true);
}
}
}
});

}
Copy link
Copy Markdown
Author

@adamlukaszczyk adamlukaszczyk Jul 7, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Prinzhorn regarding your comments i'll put my explanation here as we have here all changes in one place.

So I moved whole block of this code because you execute handleLink method here - it happens at script load time - and this is serious bug as handleLink can be executed only after initialization of module inside skrollr.menu.init()

About window.scrollTo(0, 0); - I was not sure for what is it. So it can stay in old place.

};

Expand All @@ -155,19 +175,4 @@
var _duration;
var _animate;

//In case the page was opened with a hash, prevent jumping to it.
//http://stackoverflow.com/questions/3659072/jquery-disable-anchor-jump-when-loading-a-page
defer(function() {
if(window.location.hash) {
window.scrollTo(0, 0);

if(document.querySelector) {
var link = document.querySelector('a[href="' + window.location.hash + '"]');

if(link) {
handleLink(link, true);
}
}
}
});
}(document, window));