Do not reset horizontal scroll position when moving cursor

When cursors scroll the page, they were resetting the horizontal scroll
position to zero. Upgrade the cursor to preserve the scroll-position in
the x-direction when it updates the y-direction.

Since these values are partly based on the layout of the window, the
cursor is updated to indirectly query the window object for dimensions.
This allows tests to confirm that the x-position is preserved, as well
as extend coverage to the existing `_calculateScrollToValue` method.

Bug: Issue 6791
Change-Id: Id4a5819a1d772c0c7bf6eedb381d388df9350a00

polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.js

@@ -262,13 +262,15 @@
      * @return {boolean}
      */
     _targetIsVisible(top) {
+      const dims = this._getWindowDims();
       return this.scrollBehavior === ScrollBehavior.KEEP_VISIBLE &&
-          top > window.pageYOffset &&
-          top < window.pageYOffset + window.innerHeight;
+          top > dims.pageYOffset &&
+          top < dims.pageYOffset + dims.innerHeight;
     },
 
     _calculateScrollToValue(top, target) {
-      return top - (window.innerHeight / 3) + (target.offsetHeight / 2);
+      const dims = this._getWindowDims();
+      return top - (dims.innerHeight / 3) + (target.offsetHeight / 2);
     },
 
     _scrollToTarget() {
@@ -276,6 +278,7 @@
         return;
       }
 
+      const dims = this._getWindowDims();
       const top = this._getTop(this.target);
       const bottomIsVisible = this._targetHeight ?
           this._targetIsVisible(top + this._targetHeight) : true;
@@ -286,7 +289,7 @@
         // would get scrolled to is higher up than the current position. this
         // woulld cause less of the target content to be displayed than is
         // already.
-        if (bottomIsVisible || scrollToValue < window.scrollY) {
+        if (bottomIsVisible || scrollToValue < dims.scrollY) {
           return;
         }
       }
@@ -295,7 +298,16 @@
       // instead of half the inner height feels a bit better otherwise the
       // element appears to be below the center of the window even when it
       // isn't.
-      window.scrollTo(0, scrollToValue);
+      window.scrollTo(dims.scrollX, scrollToValue);
+    },
+
+    _getWindowDims() {
+      return {
+        scrollX: window.scrollX,
+        scrollY: window.scrollY,
+        innerHeight: window.innerHeight,
+        pageYOffset: window.pageYOffset,
+      };
     },
   });
 })();

polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager_test.html

@@ -222,17 +222,13 @@ limitations under the License.
       });
 
       test('Called when top and bottom not visible', () => {
-        sandbox.stub(element, '_targetIsVisible', () => {
-          return false;
-        });
+        sandbox.stub(element, '_targetIsVisible').returns(false);
         element._scrollToTarget();
         assert.isTrue(scrollStub.called);
       });
 
       test('Not called when top and bottom visible', () => {
-        sandbox.stub(element, '_targetIsVisible', () => {
-          return true;
-        });
+        sandbox.stub(element, '_targetIsVisible').returns(true);
         element._scrollToTarget();
         assert.isFalse(scrollStub.called);
       });
@@ -240,26 +236,44 @@ limitations under the License.
       test('Called when top is visible, bottom is not, scroll is lower', () => {
         const visibleStub = sandbox.stub(element, '_targetIsVisible',
             () => visibleStub.callCount === 2);
-        window.scrollY = 15;
-        sandbox.stub(element, '_calculateScrollToValue', () => {
-          return 20;
+        sandbox.stub(element, '_getWindowDims').returns({
+          scrollX: 123,
+          scrollY: 15,
+          innerHeight: 1000,
+          pageYOffset: 0,
         });
+        sandbox.stub(element, '_calculateScrollToValue').returns(20);
         element._scrollToTarget();
         assert.isTrue(scrollStub.called);
+        assert.isTrue(scrollStub.calledWithExactly(123, 20));
         assert.equal(visibleStub.callCount, 2);
       });
 
       test('Called when top is visible, bottom not, scroll is higher', () => {
         const visibleStub = sandbox.stub(element, '_targetIsVisible',
             () => visibleStub.callCount === 2);
-        window.scrollY = 25;
-        sandbox.stub(element, '_calculateScrollToValue', () => {
-          return 20;
+        sandbox.stub(element, '_getWindowDims').returns({
+          scrollX: 123,
+          scrollY: 25,
+          innerHeight: 1000,
+          pageYOffset: 0,
         });
+        sandbox.stub(element, '_calculateScrollToValue').returns(20);
         element._scrollToTarget();
         assert.isFalse(scrollStub.called);
         assert.equal(visibleStub.callCount, 2);
       });
+
+      test('_calculateScrollToValue', () => {
+        sandbox.stub(element, '_getWindowDims').returns({
+          scrollX: 123,
+          scrollY: 25,
+          innerHeight: 300,
+          pageYOffset: 0,
+        });
+        assert.equal(element._calculateScrollToValue(1000, {offsetHeight: 10}),
+            905);
+      });
     });
   });
 </script>