From 03349168f67f7a38a28aa92c82a0c11a71e9929b Mon Sep 17 00:00:00 2001
From: Vladimir Ananiev <vladeto97@gmail.com>
Date: Mon, 30 Sep 2019 23:01:21 +0300
Subject: [PATCH 1/4] Adjust modal opening according to scrollParent

---
 src/js/components/shepherd-modal.svelte | 56 +++++++++++++++++++++++--
 1 file changed, 52 insertions(+), 4 deletions(-)

diff --git a/src/js/components/shepherd-modal.svelte b/src/js/components/shepherd-modal.svelte
index b28daf01d..2abe4b14d 100644
--- a/src/js/components/shepherd-modal.svelte
+++ b/src/js/components/shepherd-modal.svelte
@@ -32,16 +32,18 @@
   /**
    * Uses the bounds of the element we want the opening overtop of to set the dimensions of the opening and position it
    * @param {HTMLElement} targetElement The element the opening will expose
+   * @param {HTMLElement} scrollParent The scrollable parent of the target element
    * @param {Number} modalOverlayOpeningPadding An amount of padding to add around the modal overlay opening
    */
-  export function positionModalOpening(targetElement, modalOverlayOpeningPadding = 0) {
+  export function positionModalOpening(targetElement, scrollParent, modalOverlayOpeningPadding = 0) {
     if (targetElement.getBoundingClientRect) {
-      const { x, y, width, height, left, top } = targetElement.getBoundingClientRect();
+      const { y, height } = _getVisibleHeight(targetElement, scrollParent);
+      const { x, width, left } = targetElement.getBoundingClientRect();
 
       // getBoundingClientRect is not consistent. Some browsers use x and y, while others use left and top
       openingProperties = {
         x: (x || left) - modalOverlayOpeningPadding,
-        y: (y || top) - modalOverlayOpeningPadding,
+        y: y - modalOverlayOpeningPadding,
         width: (width + (modalOverlayOpeningPadding * 2)),
         height: (height + (modalOverlayOpeningPadding * 2))
       };
@@ -114,10 +116,12 @@
     const { modalOverlayOpeningPadding } = step.options;
 
     if (step.target) {
+      const scrollParent = _getScrollParent(step.target);
+
       // Setup recursive function to call requestAnimationFrame to update the modal opening position
       const rafLoop = () => {
         rafId = undefined;
-        positionModalOpening(step.target, modalOverlayOpeningPadding);
+        positionModalOpening(step.target, scrollParent, modalOverlayOpeningPadding);
         rafId = requestAnimationFrame(rafLoop);
       };
 
@@ -128,6 +132,50 @@
       closeModalOpening();
     }
   }
+
+  /**
+  * Find the closest scrollable parent element
+  * @param {HTMLElement} element The target element
+  * @returns {HTMLElement}
+  * @private
+  */
+  function _getScrollParent(element) {
+    const isHtmlElement = element instanceof HTMLElement;
+    const overflowY = isHtmlElement && window.getComputedStyle(element).overflowY;
+    const isScrollable = overflowY !== 'hidden' && overflowY !== 'visible';
+
+
+    if (!element) {
+       return document.body;
+    } if (isScrollable && element.scrollHeight >= element.clientHeight) {
+       return element;
+    }
+
+    return _getScrollParent(element.parentElement);
+  }
+
+  /**
+  * Get the visible height of the target element relative to its scrollParent
+  * @param {HTMLElement} element The target element
+  * @param {HTMLElement} scrollParent The scrollable parent element
+  * @returns {{y: number, height: number}}
+  * @private
+  */
+  function _getVisibleHeight(element, scrollParent) {
+    const elementRect = element.getBoundingClientRect();
+    const elementTop = elementRect.y || elementRect.top;
+    const elementBottom = elementRect.bottom || elementTop + elementRect.height;
+
+    const scrollRect = scrollParent.getBoundingClientRect();
+    const scrollTop = scrollRect.y || scrollRect.top;
+    const scrollBottom = scrollRect.bottom || scrollTop + scrollRect.height;
+
+    const y = Math.max(elementTop, scrollTop);
+    const bottom = Math.min(elementBottom, scrollBottom);
+    const height = Math.max(bottom - y, 0); // Default to 0 if height is negative
+
+    return {y, height};
+  }
 </script>
 
 <style global>

From a8f27da3aeca4d3da4d5d6c3c76b35b01dfa32c5 Mon Sep 17 00:00:00 2001
From: Vladimir Ananiev <vladeto97@gmail.com>
Date: Tue, 1 Oct 2019 00:00:39 +0300
Subject: [PATCH 2/4] Fix failing modal tests, add test for overflowing target

---
 test/unit/components/shepherd-modal.spec.js | 62 +++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/test/unit/components/shepherd-modal.spec.js b/test/unit/components/shepherd-modal.spec.js
index 0c2e21cea..ee812eba1 100644
--- a/test/unit/components/shepherd-modal.spec.js
+++ b/test/unit/components/shepherd-modal.spec.js
@@ -24,6 +24,15 @@ describe('components/ShepherdModal', () => {
             width: 500
           };
         }
+      }, {
+        getBoundingClientRect() {
+          return {
+            height: 250,
+            x: 20,
+            y: 20,
+            width: 500
+          };
+        }
       });
 
       let modalPath = modalComponent.getElement().querySelector('path');
@@ -68,6 +77,15 @@ describe('components/ShepherdModal', () => {
             width: 500
           };
         }
+      }, {
+        getBoundingClientRect() {
+          return {
+            height: 250,
+            x: 20,
+            y: 20,
+            width: 500
+          };
+        }
       });
 
       modalPath = modalComponent.getElement().querySelector('path');
@@ -98,6 +116,15 @@ describe('components/ShepherdModal', () => {
             width: 500
           };
         }
+      }, {
+        getBoundingClientRect() {
+          return {
+            height: 250,
+            x: 20,
+            y: 20,
+            width: 500
+          };
+        }
       }, 10);
 
       modalPath = modalComponent.getElement().querySelector('path');
@@ -106,6 +133,41 @@ describe('components/ShepherdModal', () => {
 
       modalComponent.$destroy();
     });
+
+    it('sets the correct attributes when target is overflowing from scroll parent', async () => {
+      const modalComponent = new ShepherdModal({
+        target: document.body,
+        props:
+            {
+              classPrefix
+            }
+      });
+
+      await modalComponent.positionModalOpening({
+        getBoundingClientRect() {
+          return {
+            height: 500,
+            x: 10,
+            y: 10,
+            width: 500
+          };
+        }
+      }, {
+        getBoundingClientRect() {
+          return {
+            height: 250,
+            x: 10,
+            y: 100,
+            width: 500
+          };
+        }
+      }, 0);
+
+      const modalPath = modalComponent.getElement().querySelector('path');
+      expect(modalPath).toHaveAttribute('d', 'M 10 100 H 510 V 350 H 10 L 10 0 Z M 0 0 H 1024 V 768 H 0 L 0 0 Z');
+
+      modalComponent.$destroy();
+    });
   });
 
   describe('setupForStep()', function() {

From b536e2fc4cab16a0d256768493302f3d4d65c38d Mon Sep 17 00:00:00 2001
From: Vladimir Ananiev <vladeto97@gmail.com>
Date: Wed, 2 Oct 2019 21:25:52 +0300
Subject: [PATCH 3/4] Fix modal opening clipping when no scrollable parent is
 available

---
 src/js/components/shepherd-modal.svelte     | 35 ++++++++++++---------
 test/unit/components/shepherd-modal.spec.js | 35 +++------------------
 2 files changed, 25 insertions(+), 45 deletions(-)

diff --git a/src/js/components/shepherd-modal.svelte b/src/js/components/shepherd-modal.svelte
index 2abe4b14d..2d4dcdd96 100644
--- a/src/js/components/shepherd-modal.svelte
+++ b/src/js/components/shepherd-modal.svelte
@@ -140,14 +140,16 @@
   * @private
   */
   function _getScrollParent(element) {
+    if (!element) {
+      return null;
+    }
+
     const isHtmlElement = element instanceof HTMLElement;
     const overflowY = isHtmlElement && window.getComputedStyle(element).overflowY;
     const isScrollable = overflowY !== 'hidden' && overflowY !== 'visible';
 
 
-    if (!element) {
-       return document.body;
-    } if (isScrollable && element.scrollHeight >= element.clientHeight) {
+    if (isScrollable && element.scrollHeight >= element.clientHeight) {
        return element;
     }
 
@@ -155,26 +157,31 @@
   }
 
   /**
-  * Get the visible height of the target element relative to its scrollParent
+  * Get the visible height of the target element relative to its scrollParent.
+  * If there is no scroll parent, the height of the element is returned.
+  *
   * @param {HTMLElement} element The target element
-  * @param {HTMLElement} scrollParent The scrollable parent element
+  * @param {HTMLElement} [scrollParent] The scrollable parent element
   * @returns {{y: number, height: number}}
   * @private
   */
   function _getVisibleHeight(element, scrollParent) {
     const elementRect = element.getBoundingClientRect();
-    const elementTop = elementRect.y || elementRect.top;
-    const elementBottom = elementRect.bottom || elementTop + elementRect.height;
+    let top = elementRect.y || elementRect.top;
+    let bottom = elementRect.bottom || top + elementRect.height;
+
+    if (scrollParent) {
+      const scrollRect = scrollParent.getBoundingClientRect();
+      const scrollTop = scrollRect.y || scrollRect.top;
+      const scrollBottom = scrollRect.bottom || scrollTop + scrollRect.height;
 
-    const scrollRect = scrollParent.getBoundingClientRect();
-    const scrollTop = scrollRect.y || scrollRect.top;
-    const scrollBottom = scrollRect.bottom || scrollTop + scrollRect.height;
+      top = Math.max(top, scrollTop);
+      bottom = Math.min(bottom, scrollBottom);
+    }
 
-    const y = Math.max(elementTop, scrollTop);
-    const bottom = Math.min(elementBottom, scrollBottom);
-    const height = Math.max(bottom - y, 0); // Default to 0 if height is negative
+    const height = Math.max(bottom - top, 0); // Default to 0 if height is negative
 
-    return {y, height};
+    return {y: top, height};
   }
 </script>
 
diff --git a/test/unit/components/shepherd-modal.spec.js b/test/unit/components/shepherd-modal.spec.js
index ee812eba1..df31385c4 100644
--- a/test/unit/components/shepherd-modal.spec.js
+++ b/test/unit/components/shepherd-modal.spec.js
@@ -24,16 +24,7 @@ describe('components/ShepherdModal', () => {
             width: 500
           };
         }
-      }, {
-        getBoundingClientRect() {
-          return {
-            height: 250,
-            x: 20,
-            y: 20,
-            width: 500
-          };
-        }
-      });
+      }, null);
 
       let modalPath = modalComponent.getElement().querySelector('path');
       expect(modalPath)
@@ -77,16 +68,7 @@ describe('components/ShepherdModal', () => {
             width: 500
           };
         }
-      }, {
-        getBoundingClientRect() {
-          return {
-            height: 250,
-            x: 20,
-            y: 20,
-            width: 500
-          };
-        }
-      });
+      }, null);
 
       modalPath = modalComponent.getElement().querySelector('path');
       expect(modalPath)
@@ -116,16 +98,7 @@ describe('components/ShepherdModal', () => {
             width: 500
           };
         }
-      }, {
-        getBoundingClientRect() {
-          return {
-            height: 250,
-            x: 20,
-            y: 20,
-            width: 500
-          };
-        }
-      }, 10);
+      }, null, 10);
 
       modalPath = modalComponent.getElement().querySelector('path');
       expect(modalPath)
@@ -134,7 +107,7 @@ describe('components/ShepherdModal', () => {
       modalComponent.$destroy();
     });
 
-    it('sets the correct attributes when target is overflowing from scroll parent', async () => {
+    it('sets the correct attributes when target is overflowing from scroll parent', async() => {
       const modalComponent = new ShepherdModal({
         target: document.body,
         props:

From 50dcfc23ecbf9fdbc06d56ab7de1055d9a79f9bf Mon Sep 17 00:00:00 2001
From: Vladimir Ananiev <vladeto97@gmail.com>
Date: Wed, 2 Oct 2019 21:39:27 +0300
Subject: [PATCH 4/4] Add additional test with scroll parent

---
 test/unit/components/shepherd-modal.spec.js | 53 +++++++++++++++++----
 1 file changed, 43 insertions(+), 10 deletions(-)

diff --git a/test/unit/components/shepherd-modal.spec.js b/test/unit/components/shepherd-modal.spec.js
index df31385c4..45b6b9706 100644
--- a/test/unit/components/shepherd-modal.spec.js
+++ b/test/unit/components/shepherd-modal.spec.js
@@ -110,10 +110,9 @@ describe('components/ShepherdModal', () => {
     it('sets the correct attributes when target is overflowing from scroll parent', async() => {
       const modalComponent = new ShepherdModal({
         target: document.body,
-        props:
-            {
-              classPrefix
-            }
+        props: {
+          classPrefix
+        }
       });
 
       await modalComponent.positionModalOpening({
@@ -141,6 +140,40 @@ describe('components/ShepherdModal', () => {
 
       modalComponent.$destroy();
     });
+
+    it('sets the correct attributes when target fits inside scroll parent', async() => {
+      const modalComponent = new ShepherdModal({
+        target: document.body,
+        props: {
+          classPrefix
+        }
+      });
+
+      await modalComponent.positionModalOpening({
+        getBoundingClientRect() {
+          return {
+            height: 250,
+            x: 10,
+            y: 100,
+            width: 500
+          };
+        }
+      }, {
+        getBoundingClientRect() {
+          return {
+            height: 500,
+            x: 10,
+            y: 10,
+            width: 500
+          };
+        }
+      }, 0);
+
+      const modalPath = modalComponent.getElement().querySelector('path');
+      expect(modalPath).toHaveAttribute('d', 'M 10 100 H 510 V 350 H 10 L 10 0 Z M 0 0 H 1024 V 768 H 0 L 0 0 Z');
+
+      modalComponent.$destroy();
+    });
   });
 
   describe('setupForStep()', function() {
@@ -155,9 +188,9 @@ describe('components/ShepherdModal', () => {
       const modalComponent = new ShepherdModal({
         target: document.body,
         props:
-          {
-            classPrefix
-          }
+                    {
+                      classPrefix
+                    }
       });
 
       const step = {
@@ -182,9 +215,9 @@ describe('components/ShepherdModal', () => {
       const modalComponent = new ShepherdModal({
         target: document.body,
         props:
-          {
-            classPrefix
-          }
+                    {
+                      classPrefix
+                    }
       });
 
       const step = {