Add scrollend event test for scroll combination #24028
Conversation
|
Hi @NavidZ , |
There was a problem hiding this comment.
I'm no longer working on this project. But let me cc folks who are in charge now. Particularly this test may need some more in depth discussion and possibly some spec changes that is better to be discussed with @mustaqahmed and @flackr at this point.
| await waitFor(() => { return user_scrollend_arrived; }, | ||
| 'target_div did not receive two scrollend events after pressing scrollbar and performing programmatic scroll.'); | ||
| assert_true(target_div.scrollTop > 0); | ||
| await checkFinalPosition(target_div, { x: target_div.scrollLeft, y: target_div.scrollTop }, 300); |
There was a problem hiding this comment.
My expectation here is that if scrollend event is received the offsets are updated as well on the main thread. Do we need to wait for 300ms before checking?
There was a problem hiding this comment.
Yes, that's true. The intent is to make sure there's no more scroll position change after receiving user scroll scrollend event.
There was a problem hiding this comment.
Right, presumably line 92 makes sure that we have scrolled and line 93 is asserting that we don't continue to scroll after the scrollend event.
| // Perform programmatic scroll during user scrolling. | ||
| if (target_div.scrollTop > 50) { | ||
| target_div.scrollTop = programmatic_scroll_top; | ||
| programmatic_scroll_done = true; |
There was a problem hiding this comment.
This is an interesting situation that is not clear in the spec the way it is written now. Particularly I'm not sure whether sending just one scrollend after all scrollings are done is better or sending two as you have here.
There was a problem hiding this comment.
Yes, make sense. Maybe we can discuss it on the issue? WICG/overscroll-scrollend-events#4
|
Hi NavidZ, thanks for the review:) |
Hi @NavidZ ,
This patch adds test for scrollend event with scroll combination: press scrollbar + programmatic scroll.
PTAL, thanks:)