From b91ebf6148dea055d9fb730d3c425a436c06bdff Mon Sep 17 00:00:00 2001 From: Closure Team Date: Wed, 19 Jul 2023 08:20:30 -0700 Subject: [PATCH] Correct hit detection for cases when the scroll container is not the body but the document body is not scrolled to (0,0). RELNOTES: Hit-detection fixes for DragScrollSupport class. PiperOrigin-RevId: 549321784 Change-Id: Ic45cdc01ec177cce4332b5c223281637a401a991 --- closure/goog/fx/dragscrollsupport.js | 32 +++++++++++-- closure/goog/fx/dragscrollsupport_test.js | 57 +++++++++++++++++++++++ 2 files changed, 85 insertions(+), 4 deletions(-) diff --git a/closure/goog/fx/dragscrollsupport.js b/closure/goog/fx/dragscrollsupport.js index 28c95a066e..469ba284f6 100644 --- a/closure/goog/fx/dragscrollsupport.js +++ b/closure/goog/fx/dragscrollsupport.js @@ -89,13 +89,24 @@ goog.fx.DragScrollSupport = function( */ this.scrollDelta_ = new goog.math.Coordinate(); + /** + * Whether the container actually represents the scrollable content instead of + * the parent of the scrollable content. For the entire page (BODY/HTML), + * the behavior for hit detection is different because we care about events + * relative to the viewport, which serves as the actual container. + * @private + * @const + */ + this.containerIsActuallyContent_ = + containerNode.tagName === 'BODY' || containerNode.tagName === 'HTML'; + /** * The container bounds. * @type {goog.math.Rect} * @private */ this.containerBounds_ = goog.style.getBounds(containerNode); - if (containerNode.tagName === 'BODY' || containerNode.tagName === 'HTML') { + if (this.containerIsActuallyContent_) { var size = goog.dom.getViewportSize(); this.containerBounds_.height = size.height; this.containerBounds_.width = size.width; @@ -137,6 +148,12 @@ goog.fx.DragScrollSupport.TIMER_STEP_ = 50; */ goog.fx.DragScrollSupport.SCROLL_STEP_ = 8; +/** + * @type {!goog.math.Coordinate} + * @private + * @const + */ +goog.fx.DragScrollSupport.ORIGIN_COORDINATE_ = new goog.math.Coordinate(0, 0); /** * The suggested scrolling margin. @@ -237,14 +254,20 @@ goog.fx.DragScrollSupport.prototype.onTick_ = function(event) { */ goog.fx.DragScrollSupport.prototype.onMouseMove = function(event) { 'use strict'; + let eventOffset = this.containerIsActuallyContent_ ? + goog.fx.DragScrollSupport.ORIGIN_COORDINATE_ : + goog.dom.getDomHelper(this.containerNode_).getDocumentScroll(); + /** @suppress {strictMissingProperties} Added to tighten compiler checks */ var deltaX = this.horizontalScrolling_ ? this.calculateScrollDelta( - event.clientX, this.scrollBounds_.left, this.scrollBounds_.width) : + event.clientX + eventOffset.x, this.scrollBounds_.left, + this.scrollBounds_.width) : 0; /** @suppress {strictMissingProperties} Added to tighten compiler checks */ var deltaY = this.calculateScrollDelta( - event.clientY, this.scrollBounds_.top, this.scrollBounds_.height); + event.clientY + eventOffset.y, this.scrollBounds_.top, + this.scrollBounds_.height); this.scrollDelta_.x = deltaX; this.scrollDelta_.y = deltaY; @@ -252,7 +275,8 @@ goog.fx.DragScrollSupport.prototype.onMouseMove = function(event) { // bounds of the container node. if ((!deltaX && !deltaY) || (this.constrainScroll_ && - !this.isInContainerBounds_(event.clientX, event.clientY))) { + !this.isInContainerBounds_( + event.clientX + eventOffset.x, event.clientY + eventOffset.y))) { this.scrollTimer_.stop(); } else if (!this.scrollTimer_.enabled) { this.scrollTimer_.start(); diff --git a/closure/goog/fx/dragscrollsupport_test.js b/closure/goog/fx/dragscrollsupport_test.js index 55d786de5e..5cbe72c506 100644 --- a/closure/goog/fx/dragscrollsupport_test.js +++ b/closure/goog/fx/dragscrollsupport_test.js @@ -88,6 +88,63 @@ testSuite({ dsc.dispose(); }, + /** @suppress {visibility} suppression added to enable type checking */ + testBodyScrollDragZeroMarginDivVContainer() { + const dsc = new DragScrollSupport(vContainerDiv); + + document.body.setAttribute('style', 'height: 1500px;'); + if (document.scrollingElement) { + document.scrollingElement.scrollTop = 20; + } else { + // IE doesn't have scrollingElement API. + window.scrollTo(0, 20); + } + + // Set initial scroll state. + let scrollTop = 50; + vContainerDiv.scrollTop = scrollTop; + + // Mouse move events are relative to the viewport, so this is (mostly) the + // same as the testDragZeroMarginDivVContainer test width adjust coordinates + events.fireMouseMoveEvent(vContainerDiv, new Coordinate(50, 50)); + clock.tick(DragScrollSupport.TIMER_STEP_ + 1); + assertEquals( + 'Mousing inside the vContainer should not trigger scrolling.', + scrollTop, vContainerDiv.scrollTop); + assertEquals( + 'Scroll timer should not tick yet', 0, clock.getTimeoutsMade()); + + scrollTop = vContainerDiv.scrollTop; + events.fireMouseMoveEvent(vContainerDiv, new Coordinate(50, 110)); + clock.tick(DragScrollSupport.TIMER_STEP_ + 1); + assertTrue( + 'Mousing below the vContainer should trigger scrolling down.', + scrollTop < vContainerDiv.scrollTop); + scrollTop = vContainerDiv.scrollTop; + clock.tick(DragScrollSupport.TIMER_STEP_ + 1); + assertTrue( + 'Mousing below the vContainer should trigger scrolling down.', + scrollTop < vContainerDiv.scrollTop); + + scrollTop = vContainerDiv.scrollTop; + events.fireMouseMoveEvent(vContainerDiv, new Coordinate(50, 50)); + clock.tick(DragScrollSupport.TIMER_STEP_ + 1); + assertEquals( + 'Mousing inside the vContainer should stop scrolling.', scrollTop, + vContainerDiv.scrollTop); + + clock.tick(DragScrollSupport.TIMER_STEP_ + 1); + + dsc.dispose(); + if (document.scrollingElement) { + document.scrollingElement.scrollTop = 0; + } else { + // IE doesn't have scrollingElement API. + window.scrollTo(0, 0); + } + document.body.removeAttribute('style'); + }, + /** @suppress {visibility} suppression added to enable type checking */ testDragZeroMarginDivHContainer() { const dsc = new DragScrollSupport(hContainerDiv);