feat: add scroll debounce #383
Conversation
e26e10b to
0c78e99
Compare
0c78e99 to
556fab0
Compare
src/js/default-options.js
Outdated
| // Options are: "debounce" or "throttle" | ||
| // If not set,use debounce less than 333ms , other use throttle. | ||
| // for ios browser can't use history.pushState() more than 100 times per 30 seconds reason | ||
| scrollHandlerType: undefined, |
There was a problem hiding this comment.
I feel like maybe we should just make the default here debounce since it should be better than throttle and setting that as the default here will make it more obvious whats being used and the ux between the two should be similar enough that I don't think it would be a breaking change. That way it also doesn't take extra config for users to avoid this problem on ios which I feel could be fairly common. What do you think?
There was a problem hiding this comment.
actually，current is like debounce default，maybe use undefined is not clear enough，maybe better to use 'auto',
i think the default type should auto change by timeout,i think it is slow if timeout greater than 334ms+ when use debounce
how about this?
There was a problem hiding this comment.
Yeah that’s what I thought, having as denounce would be a bit clearer. But it seemed like it was checking for the exact string of “debounce”. Anyway, I like the idea of auto too if you think that would be better, I think as long as there is a comment explaining the threshold it should be good. Thanks!
a7e79e9 to
c456cc2
Compare
hi , i add debounce type , i'm not sure it is suitable , review it , please
i will follow this after that to update version or build
