Render loading states with skeleton text, not spinners

[kumar303]

We currently use spinners to render loading states. These are problematic because the page flickers when it loads quickly and looks jumpy when it loads less quickly.

Let's switch to skeleton text for loading states. This approach would render a full layout as much as possible, using placeholders for any text that has not yet loaded.

• It will address the flicker problem since the layout will be close to final. Transitioning to a fully loaded layout should trigger only a minimal re-rendering.
• It should mostly address the jumpiness problem because each component will be fully laid out. There will still be some jumpiness for things like file content when the final content only includes one or two lines of code (it will jump from many lines to a few).
• It will provide an illusion of speed. When you click on something, it will render immediately before the data has loaded.
• We used this technique on addons-frontend and it worked well

Some considerations:

• It's important to render a skeleton screen as close to the final screen as possible. This means everything: line-height, margins, headers. Any deviation creates jumpiness.
• On addons-frontend we used an inline-block with a background color as the skeleton text. Because a div is not text, this sometimes caused a major difference in layout.
• The width of skeleton text needs to be realistic for what the final text will look like.

┆Issue is synchronized with this Jira Task

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. If you think this bug should stay open, please comment on the issue with further details. Thank you for your contributions.

[kumar303]

I played around with this but it needs work. The idea is to save an imprint of the current file and use that as a skeleton when transitioning to the next file. For the very first file load, it has to generate a random skeleton.

The spacing of the imprint lines doesn't match the code view exactly so it would be nice to fix that.

Screencast (with simulated slowness)

This was an older patch but I just got it working again on top of cb35fdfbca9619236e2f988178c17826750dce36 :

Patch

diff --git a/src/components/CodeLineShapes/index.tsx b/src/components/CodeLineShapes/index.tsx
index 7f0d1c6..933fa9a 100644
--- a/src/components/CodeLineShapes/index.tsx
+++ b/src/components/CodeLineShapes/index.tsx
@@ -1,23 +1,33 @@
 import * as React from 'react';
+import makeClassName from 'classnames';
 
+import { AnyReactNode } from '../../typeUtils';
 import { LineShapes, Token } from './utils';
 import styles from './styles.module.scss';
 
 export type PublicProps = {
+  className?: string;
+  codeClassName?: string;
+  codeChild?: AnyReactNode;
   lineShapes: LineShapes;
 };
 
 type Props = PublicProps;
 
-const CodeLineShapes = ({ lineShapes }: Props) => {
+const CodeLineShapes = ({
+  className,
+  codeChild,
+  codeClassName,
+  lineShapes,
+}: Props) => {
   return (
     <>
       {lineShapes.tokens.map((shape, shapeIndex) => {
-        let className;
+        let internalClassName;
         if (shape.token === Token.whitespace) {
-          className = styles.whitespace;
+          internalClassName = styles.whitespace;
         } else {
-          className = styles.code;
+          internalClassName = makeClassName(styles.code, codeClassName);
         }
 
         return (
@@ -28,9 +38,11 @@ const CodeLineShapes = ({ lineShapes }: Props) => {
               String(shape.percentOfWidth),
               className,
             ].join(':')}
-            className={className}
+            className={makeClassName(internalClassName, className)}
             style={{ width: `${shape.percentOfWidth}%` }}
-          />
+          >
+            {shape.token === Token.code && codeChild ? codeChild : null}
+          </div>
         );
       })}
     </>
diff --git a/src/components/CodeSkeleton/index.tsx b/src/components/CodeSkeleton/index.tsx
new file mode 100644
index 0000000..f02ede6
--- /dev/null
+++ b/src/components/CodeSkeleton/index.tsx
@@ -0,0 +1,94 @@
+import * as React from 'react';
+import { connect } from 'react-redux';
+
+import { ApplicationState } from '../../reducers';
+import { ConnectedReduxProps } from '../../configureStore';
+import { getLines } from '../CodeView/utils';
+import CodeLineShapes from '../CodeLineShapes';
+import { generateLineShapes } from '../CodeLineShapes/utils';
+import Skeleton from '../Skeleton';
+
+import styles from './styles.module.scss';
+
+type PublicProps = {};
+
+type PropsFromState = {
+  content: string | null;
+};
+
+type Props = PublicProps & PropsFromState & ConnectedReduxProps;
+
+export class CodeSkeletonBase extends React.Component<Props> {
+  generateFakeCode() {
+    const allLines = [];
+    for (let line = 1; line <= 200; line++) {
+      const line = [];
+
+      const indents = [
+        '',
+        ' '.repeat(2),
+        ' '.repeat(4),
+        ' '.repeat(6),
+        ' '.repeat(8),
+        ' '.repeat(10),
+        ' '.repeat(12),
+      ];
+      line.push(indents[Math.floor(Math.random() * indents.length)]);
+
+      if (Math.round(Math.random() * 1.0) == 1) {
+        line.push('Z');
+      } else {
+        const wordCount = Math.random() * 10;
+        for (let word = 1; word <= wordCount; word++) {
+          line.push('Z'.repeat(Math.random() * 12));
+          line.push(' ');
+        }
+      }
+
+      allLines.push(line);
+    }
+    return allLines.join('\n');
+  }
+
+  render() {
+    const { content } = this.props;
+
+    const skeletonContent = content || this.generateFakeCode();
+
+    const lines = getLines(skeletonContent);
+    const allLineShapes = generateLineShapes(lines, {
+      maxLineLength: 80,
+    });
+
+    return (
+      <div className={styles.grid}>
+        {allLineShapes.map((lineShapes, index) => {
+          return (
+            <React.Fragment>
+              <div className={styles.lineNumber}>{index + 1}</div>
+              <div className={styles.line}>
+                <CodeLineShapes
+                  className={styles.codeLineShapes}
+                  codeClassName={styles.code}
+                  key={[String(lineShapes.line), String(index)].join(':')}
+                  codeChild={
+                    <div className={styles.skeletonLine}>
+                      <Skeleton className={styles.skeleton} />
+                    </div>
+                  }
+                  lineShapes={lineShapes}
+                />
+              </div>
+            </React.Fragment>
+          );
+        })}
+      </div>
+    );
+  }
+}
+
+const mapStateToProps = (state: ApplicationState) => {
+  return { content: state.codeSkeleton.content };
+};
+
+export default connect(mapStateToProps)(CodeSkeletonBase);
diff --git a/src/components/CodeSkeleton/styles.module.scss b/src/components/CodeSkeleton/styles.module.scss
new file mode 100644
index 0000000..34d26ca
--- /dev/null
+++ b/src/components/CodeSkeleton/styles.module.scss
@@ -0,0 +1,39 @@
+@import '../../scss/variables';
+
+.grid {
+  display: grid;
+  grid-template-columns: auto 1fr;
+}
+
+.codeLineShapes {
+  min-height: $code-font-size;
+}
+
+.lineNumber {
+  color: $gray;
+  font-family: $code-font-family;
+  font-size: $code-font-size;
+  line-height: $code-line-height;
+  padding-right: $default-padding;
+  text-align: right;
+}
+
+.line {
+  display: flex;
+  line-height: 1;
+  width: 100%;
+}
+
+.code {
+  background-color: transparent;
+}
+
+.skeleton {
+  height: $code-font-size;
+}
+
+.skeletonLine {
+  align-items: center;
+  display: flex;
+  height: $code-font-size * $code-line-height;
+}
diff --git a/src/pages/Browse/index.tsx b/src/pages/Browse/index.tsx
index a195db9..810f30f 100644
--- a/src/pages/Browse/index.tsx
+++ b/src/pages/Browse/index.tsx
@@ -7,6 +7,8 @@ import log from 'loglevel';
 import { ApplicationState } from '../../reducers';
 import { ConnectedReduxProps } from '../../configureStore';
 import { ApiState } from '../../reducers/api';
+import { actions as codeSkeletonActions } from '../../reducers/codeSkeleton';
+import CodeSkeleton from '../../components/CodeSkeleton';
 import VersionFileViewer from '../../components/VersionFileViewer';
 import {
   Version,
@@ -50,6 +52,7 @@ type PropsFromRouter = {
 
 type PropsFromState = {
   apiState: ApiState;
+  codeSkeletonContent: string | null;
   file: VersionFile | null | undefined;
   fileIsLoading: boolean;
   nextFilePath: string | undefined;
@@ -86,6 +89,7 @@ export class BrowseBase extends React.Component<Props> {
     const {
       _fetchVersion,
       _fetchVersionFile,
+      codeSkeletonContent,
       currentBaseVersionId,
       currentVersionId,
       dispatch,
@@ -147,6 +151,10 @@ export class BrowseBase extends React.Component<Props> {
         }),
       );
     }
+
+    if (file && codeSkeletonContent !== file.content) {
+      dispatch(codeSkeletonActions.setContent({ content: file.content }));
+    }
   }
 
   viewVersionFile = (path: string) => {
@@ -167,7 +175,8 @@ export class BrowseBase extends React.Component<Props> {
   getContent() {
     const { file, version } = this.props;
     if (!file || !version) {
-      return <Loading message={gettext('Loading content...')} />;
+      return <CodeSkeleton />;
+      // return <Loading message={gettext('Loading content...')} />;
     }
     if (file.type === 'image' && file.downloadURL) {
       return (
@@ -180,6 +189,10 @@ export class BrowseBase extends React.Component<Props> {
         </div>
       );
     }
+
+    // return <CodeSkeleton content={file.content} />;
+    // return <CodeSkeleton />;
+
     return (
       <CodeView
         mimeType={file.mimeType}
@@ -242,6 +255,7 @@ const mapStateToProps = (
 
   return {
     apiState: state.api,
+    codeSkeletonContent: state.codeSkeleton.content,
     file: version
       ? getVersionFile(state.versions, versionId, version.selectedPath)
       : null,
diff --git a/src/reducers/codeSkeleton.tsx b/src/reducers/codeSkeleton.tsx
new file mode 100644
index 0000000..6ca6182
--- /dev/null
+++ b/src/reducers/codeSkeleton.tsx
@@ -0,0 +1,33 @@
+import { Reducer } from 'redux';
+import { ActionType, deprecated, getType } from 'typesafe-actions';
+
+// See: https://github.com/piotrwitek/typesafe-actions/issues/143
+const { createAction } = deprecated;
+
+export const actions = {
+  setContent: createAction('SET_CODE_SKELETON_CONTENT', (resolve) => {
+    return (payload: { content: string }) => resolve(payload);
+  }),
+};
+
+export type CodeSkeletonState = {
+  content: string | null;
+};
+
+export const initialState: CodeSkeletonState = {
+  content: null,
+};
+
+const reducer: Reducer<CodeSkeletonState, ActionType<typeof actions>> = (
+  state = initialState,
+  action,
+): CodeSkeletonState => {
+  switch (action.type) {
+    case getType(actions.setContent):
+      return { ...state, content: action.payload.content };
+    default:
+      return state;
+  }
+};
+
+export default reducer;
diff --git a/src/reducers/index.tsx b/src/reducers/index.tsx
index 3551ee1..a118151 100644
--- a/src/reducers/index.tsx
+++ b/src/reducers/index.tsx
@@ -4,6 +4,7 @@ import { History } from 'history';
 
 import accordionMenu, { AccordionMenuState } from './accordionMenu';
 import api, { ApiState } from './api';
+import codeSkeleton, { CodeSkeletonState } from './codeSkeleton';
 import comments, { CommentsState } from './comments';
 import errors, { ErrorsState } from './errors';
 import fileTree, { FileTreeState } from './fileTree';
@@ -16,6 +17,7 @@ import versions, { VersionsState } from './versions';
 export type ApplicationState = {
   accordionMenu: AccordionMenuState;
   api: ApiState;
+  codeSkeleton: CodeSkeletonState;
   comments: CommentsState;
   errors: ErrorsState;
   fileTree: FileTreeState;
@@ -31,6 +33,7 @@ const createRootReducer = (history: History) => {
   return combineReducers<ApplicationState>({
     accordionMenu,
     api,
+    codeSkeleton,
     comments,
     errors,
     fileTree,