mozilla / addons-code-manager

A web application to manage add-on source code
Mozilla Public License 2.0
27 stars 36 forks source link

Render loading states with skeleton text, not spinners #379

Open kumar303 opened 5 years ago

kumar303 commented 5 years ago

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.

Some considerations:

┆Issue is synchronized with this Jira Task

stale[bot] commented 4 years ago

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 commented 4 years ago

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) ![2019-12-03 22 20 38](https://user-images.githubusercontent.com/55398/70112550-4e314000-161b-11ea-8c43-227082cbdeb8.gif)

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

Patch ```diff 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} +
); })} 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 { + 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 ( +
+ {allLineShapes.map((lineShapes, index) => { + return ( + +
{index + 1}
+
+ + +
+ } + lineShapes={lineShapes} + /> +
+ + ); + })} +
+ ); + } +} + +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 { const { _fetchVersion, _fetchVersionFile, + codeSkeletonContent, currentBaseVersionId, currentVersionId, dispatch, @@ -147,6 +151,10 @@ export class BrowseBase extends React.Component { }), ); } + + if (file && codeSkeletonContent !== file.content) { + dispatch(codeSkeletonActions.setContent({ content: file.content })); + } } viewVersionFile = (path: string) => { @@ -167,7 +175,8 @@ export class BrowseBase extends React.Component { getContent() { const { file, version } = this.props; if (!file || !version) { - return ; + return ; + // return ; } if (file.type === 'image' && file.downloadURL) { return ( @@ -180,6 +189,10 @@ export class BrowseBase extends React.Component { ); } + + // return ; + // return ; + return ( { + return (payload: { content: string }) => resolve(payload); + }), +}; + +export type CodeSkeletonState = { + content: string | null; +}; + +export const initialState: CodeSkeletonState = { + content: null, +}; + +const reducer: Reducer> = ( + 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({ accordionMenu, api, + codeSkeleton, comments, errors, fileTree, ```