Conversation
|
This isn't quite right yet - there's an issue I'm still trying to track down - but I'd be surprised if the final implementation was very different. |
|
The recursive call to |
|
OK, figured it out. I suspect this didn't (and still doesn't) work if you nest scrollable elements arbitrarily deep, but that's a terrible UX anyways and practically nobody does it. |
|
oh dang. This looks solid, @DanielHeath. Sorry for neglecting it for so long. Would you be alright picking this back up after #54 lands? I’ll get you a proper review then if you’re game. |
|
I'm about to go on paternity leave (woot) so probably won't be about much. There's a bug on chrome >= 56 that stops it cleaning up after itself properly - after scrolling to something it breaks further attempts to scroll. Haven't had time to investigate yet. |
|
Oh awesome! Congratulations, @DanielHeath! Enjoy dadmode! 😄 |
|
@iamdustan @DanielHeath guys, could you actualise this PR and merge it? It's a crucial feature for me to use. UPD: I created a PR #88 based on the most recent |
I've broken my work into four logically separate commits for easy review.
The test harness renders a sequence of iframes with different setups. I found it invaluable when comparing behavior across browsers.
Adding support for overriding browser builtins made comparative testing much easier. I also plan to use it in my application because firefox's implementation has bugs.
Renaming
shouldBailOuttoscrollIsInstantmakes theblock: endstuff read much better.The final change actually implements the
block: 'end'scroll behavior.