Change to use topOffset and leftOffset

- this allows for symmetric use for vertical and horizontal offsets

Author: jdkahn

addon-test-support/scroll-into-view-mock.js

@@ -28,25 +28,34 @@ export default function mockScrollIntoView() {
       element = selector;
     }
 
-    if (!options.offset) {
+    if (options?.topOffset === undefined && options?.leftOffset === undefined) {
       return elementsInvokedOn.includes(element);
     }
 
     if (!element || !document) {
       return;
     }
-    const { behavior = 'smooth', offset = 0, left = 0 } = options;
+    const { behavior = 'smooth', leftOffset, topOffset } = options;
+
+    const elementLeft =
+      leftOffset === undefined
+        ? 0
+        : element.getBoundingClientRect().left -
+          document.body.getBoundingClientRect().left -
+          leftOffset;
 
     const elementTop =
-      element.getBoundingClientRect().top -
-      document.body.getBoundingClientRect().top -
-      offset;
+      topOffset === undefined
+        ? 0
+        : element.getBoundingClientRect().top -
+          document.body.getBoundingClientRect().top -
+          topOffset;
 
     return elementsInvokedOn.some((calledOptions) => {
       return (
         behavior === calledOptions.behavior &&
         elementTop === calledOptions.top &&
-        left === calledOptions.left
+        elementLeft === calledOptions.left
       );
     });
   };

addon/modifiers/scroll-into-view.js

@@ -12,17 +12,31 @@ export default modifier(function scrollIntoView(
 
   shouldScrollPromise.then((shouldScrollValue) => {
     if (shouldScrollValue && element && !hasBeenRemoved) {
-      if (!options?.offset) {
+      if (
+        options?.topOffset === undefined &&
+        options?.leftOffset === undefined
+      ) {
         element.scrollIntoView(options);
       } else {
-        const { behavior = 'auto', offset = 0, left = 0 } = options;
+        const { behavior = 'auto', topOffset, leftOffset } = options;
+
+        const left =
+          leftOffset === undefined
+            ? 0
+            : element.getBoundingClientRect().left -
+              document.body.getBoundingClientRect().left -
+              leftOffset;
+
+        const top =
+          topOffset === undefined
+            ? 0
+            : element.getBoundingClientRect().top -
+              document.body.getBoundingClientRect().top -
+              topOffset;
 
         window?.scrollTo({
           behavior,
-          top:
-            element.getBoundingClientRect().top -
-            document.body.getBoundingClientRect().top -
-            offset,
+          top,
           left,
         });
       }

docs/modifiers/scroll-into-view.js

@@ -5,7 +5,8 @@ import { action } from '@ember/object';
 export default class EsButtonComponent extends Component {
   @tracked shouldScrollWithOffset;
   @tracked shouldScroll;
-  @tracked offset = 25;
+  @tracked topOffset = 25;
+  @tracked leftOffset = 25;
 
   @action
   onScrollIntoView() {
@@ -18,9 +19,16 @@ export default class EsButtonComponent extends Component {
   }
 
   @action
-  onOffsetChange(event) {
+  onTopOffsetChange(event) {
     // clear the shouldScroll value to prevent scrolling on offset change
     this.shouldScrollWithOffset = false;
-    this.offset = event.target.value;
+    this.topOffset = event.target.value;
+  }
+
+  @action
+  onLeftOffsetChange(event) {
+    // clear the shouldScroll value to prevent scrolling on offset change
+    this.shouldScrollWithOffset = false;
+    this.leftOffset = event.target.value;
   }
 }

docs/modifiers/scroll-into-view.md

@@ -1,11 +1,11 @@
 # scroll-into-view
 
-This modifier scrolls to the associated element. By default it uses `scrollIntoView`, but if an offset is passed as an option it uses `scrollTo` and calculates the `options.top` attribute.
+This modifier scrolls to the associated element. By default it uses `scrollIntoView`, but if a top or left offset is passed as an option it uses `scrollTo` and calculates the `options.top` and/or `options.left` attribute.
 
 
 ## When you should use this modifier
 
-You should use this modifier whenever you need to have an element scrolled into view. If there is a element, such as a fixed header, passing in an `offset` will scroll to the element minus the `offset` value.
+You should use this modifier whenever you need to have an element scrolled into view. If there is a element, such as a fixed header or sidebar, passing in a `topOffset` or `leftOffset` will scroll to the element minus the that offset value.
 
 
 
@@ -27,11 +27,19 @@ You should use this modifier whenever you need to have an element scrolled into
 
 ### Usage with offset
 
-When passing in an offset, it will call `scrollTo`. See [scrollTo](https://developer.mozilla.org/en-US/docs/Web/API/Element/scrollTo) for the list of possible values and properties of `options`.
+When passing in an offset, it will call [scrollTo](https://developer.mozilla.org/en-US/docs/Web/API/Element/scrollTo), and the `options` parameter is designed to correspond to its `options`. The `options.behavior` operates the same, however, instead of `top` and `left` there are `topOffset` and `leftOffset`, respectively. If an offset value is not set then the value passed to `scrollTo` is 0, e.g. `options = { topOffset: 10 }` results in `element.scrollTo({ top: [computedValue], left: 0 })`. Experiment with the below example, you may need to zoom and resize the window to see a horizontal scrollbar.
+
 
 ```handlebars
-  <div {{scroll-into-view shouldScroll=this.shouldScrollWithOffset options=(hash offset=this.offset behavior="smooth")}}>
-    <input type="number" value={{this.offset}} {{on "change" this.onOffsetChange}}>
+  <div {{scroll-into-view shouldScroll=this.shouldScrollWithOffset options=(hash topOffset=this.topOffset leftOffset=this.leftOffset behavior="smooth")}}>
+    <div>
+      <label for="topOffset">Top Offset: </label>
+      <input name="topOffset" type="number" value={{this.topOffset}} {{on "change" this.onTopOffsetChange}}>
+    </div>
+    <div>
+      <label for="leftOffset">Left Offset: </label>
+      <input name="leftOffset" type="number" value={{this.leftOffset}} {{on "change" this.onLeftOffsetChange}}>
+    </div>
     <button type="button" {{on "click" this.onScrollIntoViewWithOffset}}>
       Trigger scroll-into-view with offset on click
     </button>

tests/integration/modifiers/scroll-into-view-mock-test.js

@@ -49,9 +49,9 @@ module(
 
     test('scroll into view returns correct parameters with using offset', async function (assert) {
       this.options = {
-        offset: 10,
         behavior: 'auto',
-        left: 10,
+        leftOffset: 10,
+        topOffset: 20,
       };
 
       await render(

tests/integration/modifiers/scroll-into-view-test.js

@@ -98,25 +98,20 @@ module('Integration | Modifier | scroll-into-view', function (hooks) {
     );
   });
 
-  module('with offset', function (_hooks) {
-    _hooks.beforeEach(function () {
-      this.scrollToSpy = sandbox.spy(window, 'scrollTo');
-      this.getBoundingClientRectStub = sandbox.stub(
+  module('with offsets', function (offsetHooks) {
+    offsetHooks.beforeEach(function () {
+      this.scrollToSpy = sinon.spy(window, 'scrollTo');
+      this.getBoundingClientRectStub = sinon.stub(
         Element.prototype,
         'getBoundingClientRect'
       );
-      this.getBoundingClientRectStub.onCall(0).returns({ top: 100 });
-      this.getBoundingClientRectStub.onCall(1).returns({ top: 25 });
-    });
 
-    _hooks.afterEach(function () {
-      this.scrollToSpy = null;
-      sandbox.restore();
+      this.getBoundingClientRectStub.returns({ left: 100, top: 100 });
     });
 
-    test('it renders and passes default options to scrollTo', async function (assert) {
+    test('it renders and passes default `behavior` to scrollTo', async function (assert) {
       this.options = {
-        offset: 50,
+        topOffset: 50,
       };
 
       await render(
@@ -128,19 +123,12 @@ module('Integration | Modifier | scroll-into-view', function (hooks) {
         'auto',
         'scrollTo was called with correct params'
       );
-
-      assert.strictEqual(
-        this.scrollToSpy.args[0][0].left,
-        0,
-        'scrollTo was called with correct params'
-      );
     });
 
-    test('it renders and passes behavior and left options to scrollTo', async function (assert) {
+    test('it renders and passes behavior to scrollTo', async function (assert) {
       this.options = {
-        offset: 50,
         behavior: 'smooth',
-        left: 10,
+        topOffset: 50,
       };
 
       await render(
@@ -152,19 +140,20 @@ module('Integration | Modifier | scroll-into-view', function (hooks) {
         'smooth',
         'scrollTo was called with correct params'
       );
-
-      assert.strictEqual(
-        this.scrollToSpy.args[0][0].left,
-        10,
-        'scrollTo was called with correct params'
-      );
     });
 
     test('it renders and calculates correct top offset for scrollTo when offset is passed in', async function (assert) {
       this.options = {
-        offset: 50,
+        topOffset: 50,
+        leftOffset: 40,
       };
 
+      this.getBoundingClientRectStub.onCall(0).returns({ left: 100 });
+      this.getBoundingClientRectStub.onCall(1).returns({ left: 25 });
+
+      this.getBoundingClientRectStub.onCall(2).returns({ top: 100 });
+      this.getBoundingClientRectStub.onCall(3).returns({ top: 25 });
+
       await render(
         hbs`<div id="test" {{scroll-into-view shouldScroll=true options=this.options}}></div>`
       );
@@ -173,7 +162,7 @@ module('Integration | Modifier | scroll-into-view', function (hooks) {
         this.scrollToSpy.args[0][0],
         {
           behavior: 'auto',
-          left: 0,
+          left: 35,
           top: 25,
         },
         'scrollTo was called with correct params'
@@ -182,7 +171,8 @@ module('Integration | Modifier | scroll-into-view', function (hooks) {
 
     test('it does not call scrollTo when shouldScroll is false', async function (assert) {
       this.options = {
-        offset: 50,
+        topOffset: 50,
+        leftOffset: 40,
       };
 
       await render(
@@ -194,7 +184,8 @@ module('Integration | Modifier | scroll-into-view', function (hooks) {
 
     test('it renders when shouldScroll resolves to true', async function (assert) {
       this.options = {
-        offset: 50,
+        topOffset: 50,
+        leftOffset: 40,
       };
 
       let resolvePromise;
@@ -213,7 +204,8 @@ module('Integration | Modifier | scroll-into-view', function (hooks) {
 
     test('it does not render when shouldScroll resolves to false', async function (assert) {
       this.options = {
-        offset: 50,
+        topOffset: 50,
+        leftOffset: 40,
       };
 
       let resolvePromise;