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

Fire when scrolling up rapidly to the top of the page #96

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sebastien-fauvel
Copy link

Hi @carlsednaoui,

could you please take a look at this pull request? It's implementing the feature requested in #74, which has been renewed in #84 and also addresses the needs of our own clients at Darwin Pricing. @kevinweber @roblb for your information:

The popup will be fired whenever someone scrolls back rapidly to the top of the page, which is the best guess you can make about someone's willingness to leave the site on touch devices.

I'm not looking at whether we are actually on a touch device, because there are more and more hybrid devices like desktop computers with both a mouse/trackpad and a touch screen.

The change set is the following:

  • I've introduced a configuration parameter scrollSpeed, set to 1000 pixels per second by default. This scrolling speed is the threshold to trigger the popup whenever the user scrolls back to the top of the page.
  • I've refactored a bit the delayed firing of the popup as well as its abortion by introducing two functions for that.
  • I've aborted the delayed firing whenever the user moves the mouse on the page. I've replaced the mouseenter event by mousemove for this purpose.
  • I've aborted the delayed firing whenever the user types something (excepted this Meta-L combination which was triggering it, although I don't know whether this really makes sense...)
  • I'm sampling lazily (i.e. only upon scrolling) the scrolling position in 10 ms steps
  • I'm firing the popup with the usual delay whenever the user scrolls to the top of the page at a scrolling speed above threshold. I'm using window.scrollY <= 0 because some browsers use negative offsets while they render a bounce animation when the user scrolls back to the top of the page.
  • I've increased the height of the test page so that one can scroll properly there.

I'm not sure how to run the test suite, so I haven't added a unit test for that. Feel free to do it if you have some slack!

It would be great if you could review and discuss this pull request in the next few days!

@carlsednaoui
Copy link
Owner

Hi @sebastien-fauvel, thanks for spending the time implementing this for your client — and for sending a PR through. As mentioned in #74 I personally don't like this behavior (more details here).

Not sure what the best way to proceed is, but I'd like ouibounce to keep it's focus on someone bouncing, not for rapid scrolling.

@sebastien-fauvel
Copy link
Author

Hi Carl,

thanks for looking into it! I had already read about your concerns in the original issue, and I think I've addressed them, at least on clients with a mouse/trackpad, by aborting the delayed firing whenever the mouse moves on the page. If you scroll to reach the navigation bar and go to another section, you will probably move the mouse pointer before the end of the firing delay – at least if you have a sensible one, say, 500ms? On smartphones & tablets, though, you will only have a mousemove event whenever you click the navigation menu. With a firing delay of, say, 2 seconds, we could avoid firing too early, too. What do you think? Should we add a separate configuration variable delayScroll with a reasonable default (2000ms) to handle that, or some configuration flag trackScroll set to false by default? Would it make sense to you? I mean, your current solution (firing when the mouse leaves the page, aborting when it moves onto the page again) is not perfect either, it only works fine when the firing delay is adjusted properly – because you also cross the top of the page with the mouse from time to time when you want to use the navigation menu. Isn't this quite the same issue? Also I thought you wanted to implement an open-source replacement for BounceExchange, and they do track scrolling, too...

Anyway, let me know what you think – I'll start a sister project if you want to drop this feature at all costs :)

@aacook
Copy link

aacook commented Nov 9, 2018

Was this never implemented? Seems useful.

Can anyone suggest a similar tool that's mobile-friendly?

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

Successfully merging this pull request may close these issues.

4 participants