refactor new activity button's additional offset

This is a small, independant preparation step for the larger refactor.

closes ADMIN-850

test plan:
- scrolling to new activity should still work as before.

Change-Id: I65375977ae3924cb425edc33bb649354b4803052
Reviewed-on: https://gerrit.instructure.com/143267
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/components/PlannerApp/__tests__/PlannerApp.spec.js

@@ -76,6 +76,7 @@ it('notifies the UI to perform dynamic updates', () => {
     {lifecycleExperimental: true}); // so componentDidUpdate gets called on setProps
   wrapper.setProps({isLoading: false});
   expect(mockUpdate).toHaveBeenCalledTimes(1);
+  expect(mockUpdate).toHaveBeenCalledWith(0);
 });
 
 it('shows new activity button when new activity is indicated', () => {

packages/canvas-planner/src/components/PlannerApp/index.js

@@ -77,7 +77,10 @@ export class PlannerApp extends Component {
   }
 
   componentDidUpdate () {
-    this.props.triggerDynamicUiUpdates(this.fixedElement);
+    const additionalOffset = this.newActivityButtonRef ?
+      this.newActivityButtonRef.getBoundingClientRect().height :
+      0;
+    this.props.triggerDynamicUiUpdates(additionalOffset);
   }
 
   fixedElementRef = (elt) => {
@@ -85,9 +88,7 @@ export class PlannerApp extends Component {
   }
 
   handleNewActivityClick = () => {
-    let additionalOffset = 0;
+    this.props.scrollToNewActivity();
-    if (this.newActivityButtonRef) additionalOffset = this.newActivityButtonRef.getBoundingClientRect().height;
-    this.props.scrollToNewActivity({additionalOffset});
   }
 
   renderLoading () {

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

@@ -164,7 +164,8 @@ describe('getting new activity', () => {
     const {manager, animator, store} = createManagerWithMocks();
     registerStandardDays(manager);
     animator.isAboveScreen.mockReturnValueOnce(false).mockReturnValueOnce(true);
-    manager.handleAction(scrollToNewActivity({additionalOffset: 53}));
+    manager.triggerUpdates(53);
+    manager.handleAction(scrollToNewActivity());
     expect(animator.isAboveScreen).toHaveBeenCalledWith('scrollable-nai-day-2-group-2', 42 + 53);
     expect(animator.isAboveScreen).toHaveBeenCalledWith('scrollable-nai-day-2-group-1', 42 + 53);
     expect(animator.scrollTo).toHaveBeenCalledWith('scrollable-nai-day-2-group-1', 42 + 53);
@@ -181,9 +182,9 @@ describe('getting new activity', () => {
 
   it('does animations when getting new activity requires loading', () => {
     const {manager, animator} = createManagerWithMocks();
-    manager.handleAction(scrollToNewActivity({additionalOffset: 53}));
+    manager.handleAction(scrollToNewActivity());
     manager.preTriggerUpdates('fixed-element', 'app');
-    manager.triggerUpdates();
+    manager.triggerUpdates(53);
     expect(animator.maintainViewportPosition).toHaveBeenCalledWith('fixed-element');
     expect(animator.scrollToTop).toHaveBeenCalled();
 
@@ -194,7 +195,7 @@ describe('getting new activity', () => {
     ]));
     registerStandardDays(manager);
     manager.preTriggerUpdates('fixed-element-again', 'app');
-    manager.triggerUpdates();
+    manager.triggerUpdates(53);
     expect(animator.maintainViewportPosition).toHaveBeenCalledWith('fixed-element-again');
     expect(animator.focusElement).toHaveBeenCalledWith('focusable-day-0-group-1');
     expect(animator.scrollTo).toHaveBeenCalledWith('scrollable-nai-day-0-group-1', 42 + 53);

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

@@ -38,6 +38,7 @@ export class DynamicUiManager {
     this.animatableRegistry = new AnimatableRegistry();
     this.animationPlan = {};
     this.stickyOffset = 0;
+    this.additionalOffset = 0;
   }
 
   setStickyOffset (offset) {
@@ -48,6 +49,10 @@ export class DynamicUiManager {
     this.store = store;
   }
 
+  totalOffset () {
+    return this.stickyOffset + this.additionalOffset;
+  }
+
   // If you want to register a fallback focus component when all the things in a list are deleted,
   // register that component with a -1 index and a special unique componentId that looks like
   // this: `~~~${registryName}-fallback-focus~~~` where registryName is one of the
@@ -94,7 +99,9 @@ export class DynamicUiManager {
     }
   }
 
-  triggerUpdates = () => {
+  triggerUpdates = (additionalOffset) => {
+    if (additionalOffset != null) this.additionalOffset = additionalOffset;
+
     const animationPlan = this.animationPlan;
     if (!animationPlan.ready) return;
 
@@ -137,7 +144,7 @@ export class DynamicUiManager {
       this.animatableRegistry.getLastComponent('group', newActivityGroupComponentIds);
 
     this.animator.focusElement(newActivityComponent.getFocusable());
-    this.animator.scrollTo(newActivityIndicator.getScrollable(), this.stickyOffset + this.animationPlan.additionalOffset);
+    this.animator.scrollTo(newActivityIndicator.getScrollable(), this.totalOffset());
   }
 
   triggerFocusItemComponent () {
@@ -236,16 +243,14 @@ export class DynamicUiManager {
   }
 
   handleScrollToNewActivity = (action) => {
-    const totalOffset = this.stickyOffset + action.payload.additionalOffset;
     const newActivityIndicators = this.animatableRegistry.getAllNewActivityIndicatorsSorted();
     const lastOffscreenIndicator = newActivityIndicators.reverse().find(indicator => {
-      return this.animator.isAboveScreen(indicator.component.getScrollable(), totalOffset);
+      return this.animator.isAboveScreen(indicator.component.getScrollable(), this.totalOffset());
     });
     if (lastOffscreenIndicator) {
       // there's no state update, so we can just do it now and not muck with the animationPlan
-      this.animator.scrollTo(lastOffscreenIndicator.component.getScrollable(), totalOffset);
+      this.animator.scrollTo(lastOffscreenIndicator.component.getScrollable(), this.totalOffset());
     } else {
-      this.animationPlan.additionalOffset = action.payload.additionalOffset;
       // if there's more we could load, then we should do that.
       // we're assuming there is more to load if this action happens.
       this.store.dispatch(loadPastUntilNewActivity());