improve loading logic

package.json

@@ -89,9 +89,6 @@
     "babel-preset-env": "^1.6.1",
     "babel-preset-react": "^6.24.1",
     "babel-preset-stage-2": "^6.18.0",
-    "danger": "^3.6.0",
-    "danger-plugin-eslint": "^0.1.0",
-    "danger-plugin-yarn": "^1.3.0",
     "decompress": "^4.2.0",
     "del": "^3.0.0",
     "devtron": "^1.4.0",

src/renderer/component/page/view.jsx

@@ -2,10 +2,9 @@
 import * as React from 'react';
 import classnames from 'classnames';
 import Spinner from 'component/spinner';
-import { isShowingChildren } from 'util/dom';
 
 // time in ms to wait to show loading spinner
-const LOADER_TIMEOUT = 1500;
+const LOADER_TIMEOUT = 1000;
 
 type Props = {
   children: React.Node | Array<React.Node>,
@@ -21,23 +20,6 @@ type State = {
 };
 
 class Page extends React.PureComponent<Props, State> {
-  static getDerivedStateFromProps(nextProps: Props, prevState: State) {
-    const { children } = nextProps;
-    const { showLoader } = prevState;
-
-    // If we aren't showing the loader, don't bother updating
-    if (!showLoader) {
-      return null;
-    }
-
-    if (isShowingChildren(children)) {
-      return {
-        showLoader: false,
-      };
-    }
-    return null;
-  }
-
   constructor() {
     super();
 
@@ -49,17 +31,30 @@ class Page extends React.PureComponent<Props, State> {
   }
 
   componentDidMount() {
-    const { children } = this.props;
+    const { loading } = this.props;
+    if (loading) {
+      this.beginLoadingTimeout();
+    }
+  }
 
-    if (!isShowingChildren(children))
-      this.loaderTimeout = setTimeout(() => {
-        this.setState({ showLoader: true });
-      }, LOADER_TIMEOUT);
+  componentDidUpdate(prevProps: Props) {
+    const { loading } = this.props;
+    if (!this.loaderTimeout && !prevProps.loading && loading) {
+      this.beginLoadingTimeout();
+    }
   }
 
   componentWillUnmount() {
+    if (this.loaderTimeout) {
       this.loaderTimeout = null;
     }
+  }
+
+  beginLoadingTimeout() {
+    this.loaderTimeout = setTimeout(() => {
+      this.setState({ showLoader: true });
+    }, LOADER_TIMEOUT);
+  }
 
   loaderTimeout: ?TimeoutID;
 
@@ -67,10 +62,6 @@ class Page extends React.PureComponent<Props, State> {
     const { pageTitle, children, noPadding, extraPadding, notContained, loading } = this.props;
     const { showLoader } = this.state;
 
-    // We don't want to show the loading spinner right away if it will only flash on the
-    // screen for a short time, wait until we know it will be loading for a bit before showing it
-    const shouldShowLoader = loading || (!isShowingChildren(children) && showLoader);
-
     return (
       <main
         className={classnames('main', {
@@ -85,7 +76,7 @@ class Page extends React.PureComponent<Props, State> {
           </div>
         )}
         {!loading && children}
-        {shouldShowLoader && (
+        {showLoader && (
           <div className="page__empty">
             <Spinner />
           </div>

src/renderer/page/subscriptions/index.js

@@ -11,15 +11,20 @@ import { setSubscriptionNotifications, doFetchMySubscriptions } from 'redux/acti
 import SubscriptionsPage from './view';
 
 const select = state => ({
-  isFetchingSubscriptions: selectIsFetchingSubscriptions(state),
+  loading:
+    selectIsFetchingSubscriptions(state) ||
+    Object.keys(selectSubscriptionsBeingFetched(state)).length,
   subscriptionsBeingFetched: selectSubscriptionsBeingFetched(state),
   subscriptions: selectSubscriptions(state),
   subscriptionClaims: selectSubscriptionClaims(state),
   notifications: selectNotifications(state),
 });
 
-export default connect(select, {
+export default connect(
+  select,
+  {
     doFetchClaimsByChannel,
     setSubscriptionNotifications,
     doFetchMySubscriptions,
-})(SubscriptionsPage);
+  }
+)(SubscriptionsPage);

src/renderer/page/subscriptions/view.jsx

@@ -14,10 +14,10 @@ type Props = {
   doFetchMySubscriptions: () => void,
   setSubscriptionNotifications: ({}) => void,
   subscriptions: Array<Subscription>,
-  isFetchingSubscriptions: boolean,
   subscriptionClaims: Array<{ uri: string, claims: Array<Claim> }>,
   subscriptionsBeingFetched: {},
   notifications: {},
+  loading: boolean,
 };
 
 export default class extends React.PureComponent<Props> {
@@ -52,10 +52,13 @@ export default class extends React.PureComponent<Props> {
       if (claim.claims.length) {
         subscriptionClaimMap[claim.uri] = 1;
       } else if (isDev) {
-        console
-          .error
-          // `claim for ${claim.uri} was added to byId in redux but there are no loaded fetched claims`
-          ();
+        // eslint-disable no-console
+        console.error(
+          `Claim for ${
+            claim.uri
+          } was added to byId in redux but there are no loaded fetched claims. This shouldn't happen because a subscription should have claims attached to it.`
+        );
+        // eslint-enable no-console
       }
     });
 
@@ -67,7 +70,7 @@ export default class extends React.PureComponent<Props> {
   }
 
   render() {
-    const { subscriptions, subscriptionClaims, isFetchingSubscriptions } = this.props;
+    const { subscriptions, subscriptionClaims, loading } = this.props;
 
     let claimList = [];
     subscriptionClaims.forEach(claimData => {
@@ -77,7 +80,7 @@ export default class extends React.PureComponent<Props> {
     const subscriptionUris = claimList.map(claim => `lbry://${claim.name}#${claim.claim_id}`);
 
     return (
-      <Page notContained loading={isFetchingSubscriptions}>
+      <Page notContained loading={loading}>
         <HiddenNsfwClaims uris={subscriptionUris} />
         {!subscriptions.length && (
           <div className="page__empty">

src/renderer/util/dom.js

@@ -1,12 +0,0 @@
-// @flow
-import * as React from 'react';
-
-// is a child component being rendered?
-export const isShowingChildren = (children: React.Node): boolean => {
-  if (Array.isArray(children)) {
-    const firstChildIndex = children.findIndex(child => child);
-    return firstChildIndex > -1;
-  }
-
-  return !!children;
-};