Merge pull request #91 from alfrdmalr/refactor/should-update-routine

Refactor/should update routine
alfrdmalr · Dec 3, 2019 · 9100317 · 9100317
2 parents 4a9a6df + 6cb045d
commit 9100317
Show file tree

Hide file tree

Showing 17 changed files with 131 additions and 63 deletions.
diff --git a/demo/index.tsx b/demo/index.tsx
@@ -15,14 +15,14 @@ const App = () => {
         steps={secondarySteps()}
         identifier={"2"}
         rootSelector={"#demo-container"}
+        // disableListeners
       />
       <Walktour
         steps={primaryIntoSecondary()}
         identifier={"1"}
         isOpen={tourOpen}
         customCloseFunc={(logic: WalktourLogic) => { setTourOpen(false); logic.close(); }}
         disableCloseOnClick
-        debug
       />
     </>
   )

diff --git a/docs/demo.b853f291.js → docs/demo.5ce037f3.js b/docs/demo.b853f291.js → docs/demo.5ce037f3.js
diff --git a/docs/demo.5ce037f3.js.map b/docs/demo.5ce037f3.js.map
diff --git a/docs/demo.b853f291.js.map b/docs/demo.b853f291.js.map
diff --git a/docs/demo.html b/docs/demo.html
@@ -1 +1 @@
-<!DOCTYPE html><html lang="en"><style>#app,body,html{height:100%}</style><head><title>Tour Guide Demo</title><meta charset="UTF-8"><meta name="viewport" content="width=device-width, initial-scale=1.0"></head><body> <div id="app">Loading...</div> <script src="/walktour/demo.b853f291.js"></script> </body></html>
+<!DOCTYPE html><html lang="en"><style>#app,body,html{height:100%}</style><head><title>Tour Guide Demo</title><meta charset="UTF-8"><meta name="viewport" content="width=device-width, initial-scale=1.0"></head><body> <div id="app">Loading...</div> <script src="/walktour/demo.5ce037f3.js"></script> </body></html>
diff --git a/docs/iframe.html b/docs/iframe.html
@@ -70,4 +70,4 @@
   }</script><style>#root[hidden],
       #docs-root[hidden] {
         display: none !important;
-      }</style></head><body><div class="sb-nopreview sb-wrapper"><div class="sb-nopreview_main"><h1 class="sb-nopreview_heading sb-heading">No Preview</h1><p>Sorry, but you either have no stories or none are selected somehow.</p><ul><li>Please check the Storybook config.</li><li>Try reloading the page.</li></ul><p>If the problem persists, check the browser console, or the terminal you've run Storybook from.</p></div></div><div class="sb-errordisplay sb-wrapper"><pre id="error-message" class="sb-heading"></pre><pre class="sb-errordisplay_code"><code id="error-stack"></code></pre></div><div id="root"></div><div id="docs-root"></div><script src="runtime~main.0c326ff7296018ffe52e.bundle.js"></script><script src="vendors~main.0c326ff7296018ffe52e.bundle.js"></script><script src="main.0c326ff7296018ffe52e.bundle.js"></script></body></html>
+      }</style></head><body><div class="sb-nopreview sb-wrapper"><div class="sb-nopreview_main"><h1 class="sb-nopreview_heading sb-heading">No Preview</h1><p>Sorry, but you either have no stories or none are selected somehow.</p><ul><li>Please check the Storybook config.</li><li>Try reloading the page.</li></ul><p>If the problem persists, check the browser console, or the terminal you've run Storybook from.</p></div></div><div class="sb-errordisplay sb-wrapper"><pre id="error-message" class="sb-heading"></pre><pre class="sb-errordisplay_code"><code id="error-stack"></code></pre></div><div id="root"></div><div id="docs-root"></div><script src="runtime~main.cf40354a33082f6fd44d.bundle.js"></script><script src="vendors~main.cf40354a33082f6fd44d.bundle.js"></script><script src="main.cf40354a33082f6fd44d.bundle.js"></script></body></html>
diff --git a/docs/main.0c326ff7296018ffe52e.bundle.js b/docs/main.0c326ff7296018ffe52e.bundle.js
diff --git a/docs/main.0c326ff7296018ffe52e.bundle.js.map b/docs/main.0c326ff7296018ffe52e.bundle.js.map
diff --git a/docs/main.cf40354a33082f6fd44d.bundle.js b/docs/main.cf40354a33082f6fd44d.bundle.js
diff --git a/docs/main.cf40354a33082f6fd44d.bundle.js.map b/docs/main.cf40354a33082f6fd44d.bundle.js.map
diff --git a/...ntime~main.0c326ff7296018ffe52e.bundle.js → ...ntime~main.cf40354a33082f6fd44d.bundle.js b/...ntime~main.0c326ff7296018ffe52e.bundle.js → ...ntime~main.cf40354a33082f6fd44d.bundle.js
diff --git a/...e~main.0c326ff7296018ffe52e.bundle.js.map → ...e~main.cf40354a33082f6fd44d.bundle.js.map b/...e~main.0c326ff7296018ffe52e.bundle.js.map → ...e~main.cf40354a33082f6fd44d.bundle.js.map
diff --git a/...ndors~main.0c326ff7296018ffe52e.bundle.js → ...ndors~main.cf40354a33082f6fd44d.bundle.js b/...ndors~main.0c326ff7296018ffe52e.bundle.js → ...ndors~main.cf40354a33082f6fd44d.bundle.js
diff --git a/...s~main.0c326ff7296018ffe52e.bundle.js.map → ...s~main.cf40354a33082f6fd44d.bundle.js.map b/...s~main.0c326ff7296018ffe52e.bundle.js.map → ...s~main.cf40354a33082f6fd44d.bundle.js.map
diff --git a/src/components/Walktour.tsx b/src/components/Walktour.tsx
@@ -6,8 +6,7 @@ import { CardinalOrientation, OrientationCoords, getTargetPosition, getTooltipPo
 import { Coords, getNearestScrollAncestor, getValidPortalRoot, Dims, getElementDims } from '../utils/dom';
 import { scrollToDestination } from '../utils/scroll';
 import { centerViewportAroundElements } from '../utils/offset';
-import { isElementInView } from '../utils/viewport';
-import { debounce, getIdString, shouldUpdate, setFocusTrap, setTargetWatcher, setTourUpdateListener } from '../utils/tour';
+import { debounce, getIdString, shouldUpdate, setFocusTrap, setTargetWatcher, setTourUpdateListener, shouldScroll } from '../utils/tour';
 
 
 export interface WalktourLogic {
@@ -114,6 +113,7 @@ export const Walktour = (props: WalktourProps) => {
   }
 
   const {
+    selector,
     maskPadding,
     disableMaskInteraction,
     disableCloseOnClick,
@@ -162,7 +162,6 @@ export const Walktour = (props: WalktourProps) => {
     }
   }, [rootSelector, portal.current, tourOpen])
 
-
   // update tour when step changes
   React.useEffect(() => {
     if (debug) {
@@ -181,13 +180,14 @@ export const Walktour = (props: WalktourProps) => {
       })
     }
     if (tooltip.current && tourOpen) {
-      cleanup();
+      tooltip.current.focus();
       updateTour();
     }
   }, [currentStepIndex, currentStepContent, tourOpen, tourRoot, tooltip.current])
-
+  
   // update tooltip and target position in state
   const updateTour = () => {
+    cleanup();
     const root: Element = tourRoot;
     const tooltipContainer: HTMLElement = tooltip.current;
 
@@ -199,7 +199,7 @@ export const Walktour = (props: WalktourProps) => {
       return;
     }
 
-    const getTarget = (): HTMLElement => document.querySelector(currentStepContent.selector);
+    const getTarget = (): HTMLElement => document.querySelector(selector);
     const currentTarget: HTMLElement = getTarget();
     const currentTargetPosition: Coords = getTargetPosition(root, currentTarget);
     const currentTargetDims: Dims = getElementDims(currentTarget);
@@ -219,31 +219,44 @@ export const Walktour = (props: WalktourProps) => {
     targetPosition.current = currentTargetPosition;
     targetSize.current = currentTargetDims;
 
-    tooltipContainer.focus();
-
     //focus trap subroutine
     const cleanupFocusTrap = setFocusTrap(tooltipContainer, currentTarget, disableMaskInteraction);
     cleanupRefs.current.push(cleanupFocusTrap);
 
-    // if scroll is not disabled, scroll to target if it's out of view or if the tooltip would be placed out of the viewport
-    if (!disableAutoScroll && currentTarget && (!isElementInView(root, currentTarget) || !isElementInView(root, tooltipContainer, tooltipPosition))) {
+    if (shouldScroll({
+      disableAutoScroll,
+      root,
+      target: currentTarget,
+      tooltip: tooltipContainer,
+      tooltipPosition
+    })) {
       scrollToDestination(root, centerViewportAroundElements(root, tooltipContainer, currentTarget, tooltipPosition, currentTargetPosition), disableSmoothScroll)
     }
 
     if (!disableListeners) {
-      const debouncedUpdate = debounce(() => {
+      const conditionalUpdate = () => {
         const availableTarget = getTarget();
-        if (shouldUpdate(root, tooltipContainer, availableTarget, disableAutoScroll, targetPosition.current, targetSize.current, renderTolerance)) {
+
+        if (shouldUpdate({
+          root,
+          tooltipPosition,
+          tooltip: tooltipContainer,
+          target: availableTarget,
+          disableAutoScroll,
+          rerenderTolerance: renderTolerance,
+          targetCoords: targetPosition.current,
+          targetDims: targetSize.current,
+        })) {
           updateTour();
         }
-      })
+      }
 
-      const cleanupUpdateListener = setTourUpdateListener({ update: debouncedUpdate, customSetListener: setUpdateListener, customRemoveListener: removeUpdateListener });
+      const cleanupUpdateListener = setTourUpdateListener({ update: debounce(conditionalUpdate), customSetListener: setUpdateListener, customRemoveListener: removeUpdateListener });
       cleanupRefs.current.push(cleanupUpdateListener)
 
       // if the user requests a watcher and there's supposed to be a target
-      if (movingTarget && (currentTarget || currentStepContent.selector)) {
-        const cleanupWatcher = setTargetWatcher(debouncedUpdate, updateInterval)
+      if (movingTarget && (currentTarget || selector)) {
+        const cleanupWatcher = setTargetWatcher(conditionalUpdate, updateInterval)
         cleanupRefs.current.push(cleanupWatcher);
       }
     }

diff --git a/src/utils/tour.ts b/src/utils/tour.ts
@@ -5,41 +5,20 @@ import { TAB_KEYCODE } from "./constants";
 
 //miscellaneous tour utilities
 
-export function debounce<T extends any[]>(f: (...args: T) => void) {
-  let functionCall: number;
+export function debounce<T extends any[]>(f: (...args: T) => void, interval: number = 300) {
+  let timeoutId: number;
   return (...args: T) => {
-    if (functionCall) {
-      window.cancelAnimationFrame(functionCall);
+    if (timeoutId) {
+      window.clearTimeout(timeoutId);
     }
-    functionCall = window.requestAnimationFrame(() => f(...args));
+    timeoutId = window.setTimeout(() => f(...args), interval);
   }
 }
 
 export function getIdString(base: string, identifier?: string): string {
   return `${base}${identifier ? `-${identifier}` : ``}`
 }
 
-export function shouldUpdate(tourRoot: Element, tooltip: HTMLElement, target: HTMLElement, disableAutoScroll: boolean, targetPosition: Coords, targetSize: Dims, rerenderTolerance: number): boolean {
-  if (!tourRoot || !tooltip) {
-    return false; // bail if these aren't present; need them for calculations
-  } else if (!isElementInView(tourRoot, tooltip)) {
-    return fitsWithin(getElementDims(tooltip), getViewportDims(tourRoot)); //if the tooltip is off screen, update if it CAN fit
-  } else if (!target && !targetPosition && !targetSize) {
-    return false;  // if no target info exists, bail
-  } else if ((!target && targetPosition) || (target && !targetPosition)) {
-    return true; // if the target appeared/disappeared 
-  } else if (!isElementInView(tourRoot, target) && fitsWithin(getElementDims(target), getViewportDims(tourRoot))) {
-    return !disableAutoScroll; // if the target is offscreen and can fit on the screen (and we're allowed to scroll)
-  } else {
-    const currentTargetSize: Dims = { width: target.getBoundingClientRect().width, height: target.getBoundingClientRect().height }; //TODO getelementdims
-    const currentTargetPosition: Coords = getTargetPosition(tourRoot, target);
-    const sizeChanged: boolean = areaDiff(currentTargetSize, targetSize) > rerenderTolerance;
-    const positionChanged: boolean = dist(currentTargetPosition, targetPosition) > rerenderTolerance;
-
-    return sizeChanged || positionChanged;
-  }
-}
-
 export function setTargetWatcher(callback: () => void, interval: number): (() => void) {
   const intervalId: number = window.setInterval(callback, interval);
 
@@ -54,7 +33,7 @@ export interface SetTourUpdateListenerArgs {
 }
 
 export function setTourUpdateListener(args: SetTourUpdateListenerArgs) {
-  const {update, customSetListener, customRemoveListener, event } = {event: 'resize', ...args}
+  const { update, customSetListener, customRemoveListener, event } = { event: 'resize', ...args }
   if (customSetListener && customRemoveListener) {
     customSetListener(update);
     return () => customRemoveListener(update);
@@ -120,7 +99,82 @@ export const setFocusTrap = (tooltipContainer: HTMLElement, target?: HTMLElement
     if (target) {
       target.removeEventListener('keydown', targetTrapHandler);
     }
-    
+
     tooltipContainer.removeEventListener('keydown', tooltipTrapHandler);
   }
 }
+
+interface NaiveShouldScrollArgs {
+  root: Element;
+  tooltip: HTMLElement;
+  tooltipPosition?: Coords;
+  target: HTMLElement;
+}
+
+function naiveShouldScroll(args: NaiveShouldScrollArgs): boolean {
+  const { root, tooltip, tooltipPosition, target } = args;
+
+  if (!isElementInView(root, tooltip, tooltipPosition)) {
+    return true;
+  }
+
+  if (!isElementInView(root, target)) {
+    return fitsWithin(getElementDims(target), getViewportDims(root));
+  }
+
+  return false;
+}
+export interface ShouldScrollArgs extends NaiveShouldScrollArgs {
+  disableAutoScroll: boolean;
+}
+
+export function shouldScroll(args: ShouldScrollArgs): boolean {
+  const { root, tooltip, target, disableAutoScroll } = args;
+  if (!root || !tooltip || !target) {
+    return false;
+  }
+
+  if (disableAutoScroll) {
+    return false;
+  }
+
+  return naiveShouldScroll({ ...args });
+}
+
+export interface TargetChangedArgs {
+  root: Element;
+  target: HTMLElement;
+  targetCoords: Coords;
+  targetDims: Dims;
+  rerenderTolerance: number;
+}
+export function targetChanged(args: TargetChangedArgs): boolean {
+  const { root, target, targetCoords, targetDims, rerenderTolerance } = args;
+  if (!target && !targetCoords && !targetDims) {
+    return false;
+  }
+
+  // when the target / target data are out of sync. usually due to a movingTarget, i.e. the target arg is more up to date than the pos/dims args
+  if ((!target && targetCoords && targetDims) || (target && !targetCoords && !targetDims)) {
+    return true;
+  }
+
+  const currentTargetSize: Dims = getElementDims(target);
+  const currentTargetPosition: Coords = getTargetPosition(root, target);
+
+  const sizeChanged: boolean = areaDiff(currentTargetSize, targetDims) > rerenderTolerance;
+  const positionChanged: boolean = dist(currentTargetPosition, targetCoords) > rerenderTolerance;
+
+  return sizeChanged || positionChanged;
+}
+
+export interface ShouldUpdateArgs extends TargetChangedArgs, ShouldScrollArgs { }
+
+export function shouldUpdate(args: ShouldUpdateArgs): boolean {
+  const { root, tooltip } = args;
+  if (!root || !tooltip) {
+    return false; // bail if these aren't present; need them for calculations
+  }
+
+  return targetChanged({ ...args }) || shouldScroll({ ...args }); // future todo: if no target, check if tooltip is correctly positioned (null selector -> tooltip out of place)
+}
diff --git a/stories/options/options.stories.tsx b/stories/options/options.stories.tsx
@@ -58,5 +58,6 @@ export const all = () => (
     disableMask={boolean('disableMask', true)}
     disableAutoScroll={boolean('disableAutoScroll', true)}
     disableSmoothScroll={boolean('disableSmoothScroll', true)}
+    movingTarget={boolean('movingTarget', false)}
   />
 )
Original file line number	Diff line number	Diff line change
		@@ -1 +1 @@
		<!DOCTYPE html><html lang="en"><style>#app,body,html{height:100%}</style><head><title>Tour Guide Demo</title><meta charset="UTF-8"><meta name="viewport" content="width=device-width, initial-scale=1.0"></head><body> <div id="app">Loading...</div> <script src="/walktour/demo.b853f291.js"></script> </body></html>
		<!DOCTYPE html><html lang="en"><style>#app,body,html{height:100%}</style><head><title>Tour Guide Demo</title><meta charset="UTF-8"><meta name="viewport" content="width=device-width, initial-scale=1.0"></head><body> <div id="app">Loading...</div> <script src="/walktour/demo.5ce037f3.js"></script> </body></html>