change timing of maintainViewportPosition call

Makes it so this call can happen after the UI update. Then the
animations don't have to worry so much about the timing of it.

closes ADMIN-851

test plan:
- when scrolling into the past, the viewport position is maintained
as before.
- when clicking new activity, the scrolling is still animated as before.

Change-Id: I773f8731775796b434b1e5768f8a600a091b8f9a
Reviewed-on: https://gerrit.instructure.com/143400
Reviewed-by: Ed Schiebel <eschiebel@instructure.com>
Tested-by: Jenkins
QA-Review: Deepeeca Soundarrajan <dsoundarrajan@instructure.com>
Product-Review: Jon Willesen <jonw+gerrit@instructure.com>

packages/canvas-planner/src/dynamic-ui/__tests__/animator.spec.js

@@ -109,6 +109,7 @@ it('maintains scroll position of element', () => {
   mocks.document.documentElement.getBoundingClientRect.mockReturnValueOnce({
     top: -5, left: 0, bottom: 123, right: 50,
   });
+  animator.recordFixedElement(elt);
   animator.maintainViewportPosition(elt);
   expect(mocks.window.scroll).not.toHaveBeenCalled();
   mocks.window.runAnimationFrames();

packages/canvas-planner/src/dynamic-ui/__tests__/manager.spec.js

@@ -24,6 +24,7 @@ import { initialize as alertInitialize } from '../../utilities/alertUtils';
 class MockAnimator {
   animationOrder = []
   isAboveScreen = jest.fn()
+  recordFixedElement = jest.fn()
   constructor () {
     ['maintainViewportPosition', 'focusElement', 'scrollTo', 'scrollToTop'].forEach(fnName => {
       this[fnName] = jest.fn(() => {
@@ -154,8 +155,9 @@ describe('getting past items', () => {
     ));
     registerStandardDays(manager);
     manager.preTriggerUpdates('fixed-element', 'app');
+    expect(animator.recordFixedElement).toHaveBeenCalledWith('fixed-element');
     manager.triggerUpdates();
-    expect(animator.maintainViewportPosition).toHaveBeenCalledWith('fixed-element');
+    expect(animator.maintainViewportPosition).toHaveBeenCalled();
   });
 });
 
@@ -185,7 +187,7 @@ describe('getting new activity', () => {
     manager.handleAction(scrollToNewActivity());
     manager.preTriggerUpdates('fixed-element', 'app');
     manager.triggerUpdates(53);
-    expect(animator.maintainViewportPosition).toHaveBeenCalledWith('fixed-element');
+    expect(animator.maintainViewportPosition).toHaveBeenCalled();
     expect(animator.scrollToTop).toHaveBeenCalled();
 
     manager.handleAction(gettingPastItems({seekingNewActivity: true}));
@@ -196,7 +198,7 @@ describe('getting new activity', () => {
     registerStandardDays(manager);
     manager.preTriggerUpdates('fixed-element-again', 'app');
     manager.triggerUpdates(53);
-    expect(animator.maintainViewportPosition).toHaveBeenCalledWith('fixed-element-again');
+    expect(animator.maintainViewportPosition).toHaveBeenCalled();
     expect(animator.focusElement).toHaveBeenCalledWith('focusable-day-0-group-1');
     expect(animator.scrollTo).toHaveBeenCalledWith('scrollable-nai-day-0-group-1', 42 + 53);
   });
@@ -212,7 +214,7 @@ describe('getting new activity', () => {
     manager.preTriggerUpdates('fixed-element', 'app');
     manager.triggerUpdates();
     // can still maintain the viewport position for the new load
-    expect(animator.maintainViewportPosition).toHaveBeenCalledWith('fixed-element');
+    expect(animator.maintainViewportPosition).toHaveBeenCalled();
     // other animations don't happen because we don't know what to animate to.
     expect(animator.focusElement).not.toHaveBeenCalled();
     expect(animator.scrollTo).not.toHaveBeenCalled();
@@ -229,7 +231,7 @@ describe('manipulating items', () => {
     manager.triggerUpdates();
     expect(animator.focusElement).toHaveBeenCalledWith(doc.activeElement);
     // maintain and scrolling works around a chrome bug
-    expect(animator.maintainViewportPosition).toHaveBeenCalledWith('fixed-element');
+    expect(animator.maintainViewportPosition).toHaveBeenCalled();
     expect(animator.scrollTo).toHaveBeenCalledWith(doc.activeElement, 42);
   });
 
@@ -242,7 +244,7 @@ describe('manipulating items', () => {
     manager.triggerUpdates();
     expect(animator.focusElement).toHaveBeenCalledWith(doc.activeElement);
     // maintain and scrolling works around a chrome bug
-    expect(animator.maintainViewportPosition).toHaveBeenCalledWith('fixed-element');
+    expect(animator.maintainViewportPosition).toHaveBeenCalled();
     expect(animator.scrollTo).not.toHaveBeenCalled();
   });
 
@@ -255,7 +257,7 @@ describe('manipulating items', () => {
     manager.triggerUpdates();
     expect(animator.focusElement).toHaveBeenCalledWith('focusable-day-0-group-0-item-0');
     // maintain and scrolling works around a chrome bug
-    expect(animator.maintainViewportPosition).toHaveBeenCalledWith('fixed-element');
+    expect(animator.maintainViewportPosition).toHaveBeenCalled();
     expect(animator.scrollTo).toHaveBeenCalledWith('scrollable-day-0-group-0-item-0', 42);
   });
 
@@ -267,7 +269,7 @@ describe('manipulating items', () => {
     manager.preTriggerUpdates('fixed-element', 'app');
     manager.triggerUpdates();
     expect(animator.focusElement).toHaveBeenCalledWith('focusable-day-0-group-0-item-0');
-    expect(animator.maintainViewportPosition).toHaveBeenCalledWith('fixed-element');
+    expect(animator.maintainViewportPosition).toHaveBeenCalled();
     expect(animator.scrollTo).toHaveBeenCalledWith('scrollable-day-0-group-0-item-0', 42);
   });
 

packages/canvas-planner/src/dynamic-ui/animator.js

@@ -29,6 +29,8 @@ export class Animator {
     );
   }
   animationQueue = [];
+  fixedElement = null;
+  fixedElementsInitialPositionInViewport = null;
 
   focusElement (elt) {
     // focusing an element causes it to scroll into view, so do the focus first so it doesn't
@@ -37,17 +39,24 @@ export class Animator {
     else this.queueAnimation(() => elt.focus(), 'unshift');
   }
 
+  recordFixedElement (elt) {
+    if (elt) {
+      this.fixedElement = elt;
+      this.fixedElementsInitialPositionInViewport = elt.getBoundingClientRect().top;
+    }
+  }
+
   // Based on this formula:
   // element's position in the viewport + the window's scroll position === the element's position in the document
   // so if we want the scroll position that will maintain the element in it's current viewport position,
   // window scroll position = element's current document position - element's initial viewport position
-  maintainViewportPosition (elt) {
-    const elementsInitialPositionInViewport = elt.getBoundingClientRect().top;
+  maintainViewportPosition () {
+    if (this.fixedElement == null) return;
     this.queueAnimation(() => {
-      const elementsNewPositionInViewport = elt.getBoundingClientRect().top;
+      const fixedElementsNewPositionInViewport = this.fixedElement.getBoundingClientRect().top;
       const documentPositionInViewport = this.document.documentElement.getBoundingClientRect().top;
-      const elementPositionInDocument = elementsNewPositionInViewport - documentPositionInViewport;
-      const newWindowScrollPosition = elementPositionInDocument - elementsInitialPositionInViewport;
+      const fixedElementsPositionInDocument = fixedElementsNewPositionInViewport - documentPositionInViewport;
+      const newWindowScrollPosition = fixedElementsPositionInDocument - this.fixedElementsInitialPositionInViewport;
       this.window.scroll(0, newWindowScrollPosition);
     }, 'push');
   }

packages/canvas-planner/src/dynamic-ui/manager.js

@@ -88,14 +88,9 @@ export class DynamicUiManager {
   }
 
   preTriggerUpdates = (fixedElement, triggerer) => {
-    const animationPlan = this.animationPlan;
-    if (!animationPlan.ready) return;
-
     // only the app should be allowed to muck with the scroll position (the header should not).
-    if (triggerer !== 'app') return;
-
-    if (fixedElement && this.shouldMaintainCurrentScrollingPosition()) {
-      this.animator.maintainViewportPosition(fixedElement);
+    if (triggerer === 'app') {
+      this.animator.recordFixedElement(fixedElement);
     }
   }
 
@@ -105,6 +100,10 @@ export class DynamicUiManager {
     const animationPlan = this.animationPlan;
     if (!animationPlan.ready) return;
 
+    if (this.shouldMaintainCurrentScrollingPosition()) {
+      this.animator.maintainViewportPosition();
+    }
+
     if (this.animationPlan.scrollToTop) {
       this.triggerScrollToTop();
     } else if (this.animationPlan.scrollToLastNewActivity) {