Merge pull request #750 from seanyesmunt/scroll-issues

refactor scroll navigation/restore

CHANGELOG.md

@@ -12,12 +12,12 @@ Web UI version numbers should always match the corresponding version of LBRY App
   *
 
 ### Changed
-  *
+  * Moved all redux code into /redux folder
   *
 
 ### Fixed
   * Long channel names causing inconsistent thumbnail sizes (#721)
-  *
+  * Fixed scriolling restore/reset/set (#729)
 
 ### Deprecated
   *

ui/js/component/app/index.js

@@ -1,6 +1,10 @@
 import React from "react";
 import { connect } from "react-redux";
-import { selectPageTitle } from "redux/selectors/navigation";
+import {
+  selectPageTitle,
+  selectHistoryIndex,
+  selectActiveHistoryEntry,
+} from "redux/selectors/navigation";
 import { selectUser } from "redux/selectors/user";
 import { doCheckUpgradeAvailable, doAlertError } from "redux/actions/app";
 import { doRecordScroll } from "redux/actions/navigation";
@@ -10,6 +14,8 @@ import App from "./view";
 const select = (state, props) => ({
   pageTitle: selectPageTitle(state),
   user: selectUser(state),
+  currentStackIndex: selectHistoryIndex(state),
+  currentPageAttributes: selectActiveHistoryEntry(state),
 });
 
 const perform = dispatch => ({

ui/js/component/app/view.jsx

@@ -4,8 +4,14 @@ import Header from "component/header";
 import Theme from "component/theme";
 import ModalRouter from "modal/modalRouter";
 import lbry from "lbry";
+import throttle from "util/throttle";
 
 class App extends React.PureComponent {
+  constructor() {
+    super();
+    this.mainContent = undefined;
+  }
+
   componentWillMount() {
     const {
       alertError,
@@ -23,21 +29,36 @@ class App extends React.PureComponent {
 
     fetchRewardedContent();
 
-    this.scrollListener = () => this.props.recordScroll(window.scrollY);
-
-    window.addEventListener("scroll", this.scrollListener);
-
     this.setTitleFromProps(this.props);
   }
 
+  componentDidMount() {
+    const { recordScroll } = this.props;
+    const mainContent = document.getElementById("main-content");
+    this.mainContent = mainContent;
+
+    const scrollListener = () => recordScroll(this.mainContent.scrollTop);
+
+    this.mainContent.addEventListener("scroll", throttle(scrollListener, 750));
+  }
+
   componentWillUnmount() {
-    window.removeEventListener("scroll", this.scrollListener);
+    this.mainContent.removeEventListener("scroll", this.scrollListener);
   }
 
   componentWillReceiveProps(props) {
     this.setTitleFromProps(props);
   }
 
+  componentDidUpdate(prevProps) {
+    const { currentStackIndex: prevStackIndex } = prevProps;
+    const { currentStackIndex, currentPageAttributes } = this.props;
+
+    if (currentStackIndex !== prevStackIndex) {
+      this.mainContent.scrollTop = currentPageAttributes.scrollY || 0;
+    }
+  }
+
   setTitleFromProps(props) {
     window.document.title = props.pageTitle || "LBRY";
   }

ui/js/redux/actions/navigation.js

@@ -21,21 +21,11 @@ export function doNavigate(path, params = {}, options = {}) {
       url += "?" + toQueryString(params);
     }
 
-    const state = getState(),
-      currentPage = selectCurrentPage(state),
-      nextPage = computePageFromPath(path),
-      scrollY = options.scrollY;
-
-    if (currentPage != nextPage) {
-      //I wasn't seeing it scroll to the proper position without this -- possibly because the page isn't fully rendered? Not sure - Jeremy
-      setTimeout(() => {
-        window.scrollTo(0, scrollY ? scrollY : 0);
-      }, 100);
-    }
+    const scrollY = options.scrollY;
 
     dispatch({
       type: types.HISTORY_NAVIGATE,
-      data: { url, index: options.index },
+      data: { url, index: options.index, scrollY },
     });
   };
 }

ui/js/redux/reducers/navigation.js

@@ -32,14 +32,14 @@ reducers[types.CHANGE_AFTER_AUTH_PATH] = function(state, action) {
 
 reducers[types.HISTORY_NAVIGATE] = (state, action) => {
   const { stack, index } = state;
-  const path = action.data.url;
+  const { url: path, index: newIndex, scrollY } = action.data;
 
   let newState = {
     currentPath: path,
   };
 
-  if (action.data.index >= 0) {
-    newState.index = action.data.index;
+  if (newIndex >= 0) {
+    newState.index = newIndex;
   } else if (!stack[index] || stack[index].path !== path) {
     // ^ Check for duplicated
     newState.stack = [...stack.slice(0, index + 1), { path, scrollY: 0 }];
@@ -47,7 +47,6 @@ reducers[types.HISTORY_NAVIGATE] = (state, action) => {
   }
 
   history.replaceState(null, null, "#" + path); //this allows currentPath() to retain the URL on reload
-
   return Object.assign({}, state, newState);
 };
 

ui/js/redux/selectors/navigation.js

@@ -146,3 +146,9 @@ export const selectHistoryStack = createSelector(
   _selectState,
   state => state.stack
 );
+
+// returns current page attributes (scrollY, path)
+export const selectActiveHistoryEntry = createSelector(
+  _selectState,
+  state => state.stack[state.index]
+);

ui/js/util/throttle.js

@@ -0,0 +1,56 @@
+// Taken from underscore.js (slightly modified to add the getNow function and use const/let over var)
+// https://github.com/jashkenas/underscore/blob/master/underscore.js#L830-L874
+
+// Returns a function, that, when invoked, will only be triggered at most once
+// during a given window of time. Normally, the throttled function will run
+// as much as it can, without ever going more than once per `wait` duration;
+// but if you'd like to disable the execution on the leading edge, pass
+// `{leading: false}`. To disable execution on the trailing edge, ditto.
+export default function throttle(func, wait, options) {
+  let timeout, context, args, result;
+  let previous = 0;
+  const getNow = () => new Date().getTime();
+
+  if (!options) options = {};
+
+  const later = function() {
+    previous = options.leading === false ? 0 : getNow();
+    timeout = null;
+    result = func.apply(context, args);
+    if (!timeout) context = args = null;
+  };
+
+  const throttled = function() {
+    const now = getNow();
+
+    if (!previous && options.leading === false) previous = now;
+
+    const remaining = wait - (now - previous);
+
+    context = this;
+    args = arguments;
+
+    if (remaining <= 0 || remaining > wait) {
+      if (timeout) {
+        clearTimeout(timeout);
+        timeout = null;
+      }
+
+      previous = now;
+      result = func.apply(context, args);
+
+      if (!timeout) context = args = null;
+    } else if (!timeout && options.trailing !== false) {
+      timeout = setTimeout(later, remaining);
+    }
+    return result;
+  };
+
+  throttled.cancel = function() {
+    clearTimeout(timeout);
+    previous = 0;
+    timeout = context = args = null;
+  };
+
+  return throttled;
+}