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 intoPrinzhorn:masterfrom
adamlukaszczyk:master
Open

Fix for opening url with hash and scrolling webpage to correct place.#8
adamlukaszczyk wants to merge 3 commits intoPrinzhorn:masterfrom
adamlukaszczyk:master

Conversation

@adamlukaszczyk
Copy link
Copy Markdown

While opening page with hash function handleLink() was fired before skroll.menu.init(),
so _skrollrInstance.setScrollTop(top) throw an error.

Dobiatowski! added 3 commits July 7, 2013 00:14
…kroll.menu.init(),

so _skrollrInstance.setScrollTop(top) throw an error.

Removed 'fake' parameter so now everything works nicely.
@Prinzhorn
Copy link
Copy Markdown
Owner

Good catch.

Could you explain that last commit? How is it related?

Copy link
Copy Markdown
Author

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.

@adamlukaszczyk
Copy link
Copy Markdown
Author

Hello @Prinzhorn. Have you tried this changes ?

@Prinzhorn
Copy link
Copy Markdown
Owner

Sorry, I haven't. I'm busy with non-open source stuff. But I'll probably find a day in the next weeks which I can spend completely on skrollr.

@ghost
Copy link
Copy Markdown

ghost commented Jul 27, 2013

Your fix is working for me @dobiatowski => thank you (even in FF).

@ajohnh
Copy link
Copy Markdown

ajohnh commented Aug 1, 2013

@dobiatowski - Yes, this fix works wonderfully in FF and webkit browsers (thank you!), but seems to break in iOS Safari. Results in something like: Prinzhorn/skrollr#104. Have you noticed this behavior or is it just on my end?

@Prinzhorn
Copy link
Copy Markdown
Owner

Have you noticed this behavior or is it just on my end?

That's the point here. skrollr doesn't do native scrolling on mobile, so you can't rely on any native hash behavior.

@Prinzhorn
Copy link
Copy Markdown
Owner

Please try version 0.1.6. It should work on mobile as well.

@neufeind
Copy link
Copy Markdown

@Prinzhorn: I tried the current version and stumbled across a problem with popstate in certain cases with Firefox. Adding the check for:
if (e.state) {
before actually looking at the state and doing the defer-call to scroll to top (which would actually scroll to position 0 since there was no position found) solved the problem for me.

Those cases worked fine for me on other browsers. There seem to be cases when FF triggers the popstate which we might want to ignore. For me the problem was that one scroll happened that scrolled to the desired position and then the scroll here kicked in which scrolled to 0 (top). Since both might be defered (using a window.setTimeout) I wonder if they could maybe even overlap since they don't necessarily have a defined order or so.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants