Skip to content

Commit

Permalink
Update FullScreenLoadingIndicator with PR suggestions
Browse files Browse the repository at this point in the history
- remove `name` and `timeout` props. Use fixed timeout and a `logDetail` prop
- use fixed strings in log messages and pass details as additional data
- extract timeout duration to CONST
- throw if logging is not configured correctly
- trigger a special CONST.ERROR.ENSURE_BUGBOT alert
  • Loading branch information
kidroca committed Mar 30, 2022
1 parent 48ebd2a commit cd397ae
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 19 deletions.
1 change: 1 addition & 0 deletions src/CONST.js
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,7 @@ const CONST = {
COLD: 'cold',
REPORT_ACTION_ITEM_LAYOUT_DEBOUNCE_TIME: 1500,
TOOLTIP_SENSE: 1000,
SPINNER_TIMEOUT: 15 * 1000,
},
PRIORITY_MODE: {
GSD: 'gsd',
Expand Down
39 changes: 23 additions & 16 deletions src/components/FullscreenLoadingIndicator.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,22 +6,25 @@ import styles from '../styles/styles';
import themeColors from '../styles/themes/default';
import stylePropTypes from '../styles/stylePropTypes';
import Log from '../libs/Log';
import CONST from '../CONST';

const propTypes = {
/** Name used in the logs if the loader is displayed for too long time */
name: PropTypes.string,

/** Optional duration (ms) after which a message would be logged if the loader is still covering the screen */
timeout: PropTypes.number,
/**
* Context info printed in timing log.
* Providing this prop would capture logs for mounting/unmounting and staying visible for too long
*/
logDetail: PropTypes.shape({
/** Name is used to distinct the loader in captured logs. */
name: PropTypes.string.isRequired,
}),

/** Additional style props */
style: stylePropTypes,
};

const defaultProps = {
timeout: 0,
name: '',
style: [],
logDetail: null,
};

/**
Expand All @@ -32,29 +35,33 @@ const defaultProps = {
*/
class FullScreenLoadingIndicator extends React.Component {
componentDidMount() {
if (!this.props.name || !this.props.timeout) {
if (!this.props.logDetail) {
return;
}

Log.info(`[LoadingIndicator] "${this.props.name}" became visible`);
if (!this.props.logDetail.name) {
throw new Error('A name should be set to distinct logged messages. Please check the `logDetails` prop');
}

Log.info('[LoadingIndicator] Became visible', false, this.props.logDetail);

this.timeoutId = setTimeout(
this.timeoutID = setTimeout(
() => Log.alert(
`[LoadingIndicator] "${this.props.name}" is still visible after ${this.props.timeout} ms`,
'',
`${CONST.ERROR.ENSURE_BUGBOT} [LoadingIndicator] Visible after timeout`,
{timeout: CONST.TIMING.SPINNER_TIMEOUT, ...this.props.logDetail},
false,
),
this.props.timeout,
CONST.TIMING.SPINNER_TIMEOUT,
);
}

componentWillUnmount() {
if (!this.timeoutId) {
if (!this.timeoutID) {
return;
}

clearTimeout(this.timeoutId);
Log.info(`[LoadingIndicator] "${this.props.name}" disappeared`);
clearTimeout(this.timeoutID);
Log.info('[LoadingIndicator] Disappeared', false, this.props.logDetail);
}

render() {
Expand Down
2 changes: 1 addition & 1 deletion src/libs/Navigation/AppNavigator/MainDrawerNavigator.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ const MainDrawerNavigator = (props) => {

// Wait until reports are fetched and there is a reportID in initialParams
if (!initialParams.reportID) {
return <FullScreenLoadingIndicator name="Main Drawer Loader" timeout={15 * 1000} />;
return <FullScreenLoadingIndicator logDetail={{name: 'Main Drawer Loader', initialParams}} />;
}

// After the app initializes and reports are available the home navigation is mounted
Expand Down
3 changes: 1 addition & 2 deletions src/libs/Navigation/NavigationRoot.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,7 @@ class NavigationRoot extends Component {
<NavigationContainer
fallback={(
<FullScreenLoadingIndicator
name="Navigation Fallback Loader"
timeout={15 * 1000}
logDetail={{name: 'Navigation Fallback Loader', authenticated: this.props.authenticated}}
style={styles.navigatorFullScreenLoading}
/>
)}
Expand Down

0 comments on commit cd397ae

Please sign in to comment.