[RN] Cache avatars and provide a default in case load fails (2)

Refactors the previous "[RN] Cache avatars and provide a default in
case load fails" for the purposes of simplification but also modifies
its functionality at the same time. For example:

- Always displays the default avatar immediately which may be seen if
  the remote avatar needs to be downloaded.
- Does not use random colors.
- Uses a default avatar image which is not transparent and ugly but at
  least it's the same image that's used on Web. I've started talks to
  have images/avatar2.png replaced with a transparent and beautiful
  so that will land later on and we'll see the automagic colors in all
  their glory then.

react/features/base/participants/components/Avatar.native.js

@@ -1,7 +1,22 @@
 import React, { Component } from 'react';
-import { CustomCachedImage } from 'react-native-img-cache';
+import { View } from 'react-native';
+import { CachedImage, ImageCache } from 'react-native-img-cache';
 
-import AvatarImage from './AvatarImage';
+import { Platform } from '../../react';
+import { ColorPalette } from '../../styles';
+
+// FIXME @lyubomir: The string images/avatar2.png appears three times in our
+// source code at the time of this writing. Firstly, it presents a maintenance
+// obstacle which increases the risks of inconsistency. Secondly, it is
+// repulsive (when enlarged, especially, on mobile/React Native, for example).
+/**
+ * The default image/source to be used in case none is specified or the
+ * specified one fails to load.
+ *
+ * @private
+ * @type {string}
+ */
+const _DEFAULT_SOURCE = require('../../../../../images/avatar2.png');
 
 /**
  * Implements an avatar as a React Native/mobile {@link Component}.
@@ -60,6 +75,8 @@ export default class Avatar extends Component {
 
         if (prevURI !== nextURI || !this.state) {
             const nextState = {
+                backgroundColor: this._getBackgroundColor(nextProps),
+
                 /**
                  * The source of the {@link Image} which is the actual
                  * representation of this {@link Avatar}. The state
@@ -70,9 +87,7 @@ export default class Avatar extends Component {
                  *     uri: string
                  * }}
                  */
-                source: {
-                    uri: nextURI
-                }
+                source: _DEFAULT_SOURCE
             };
 
             if (this.state) {
@@ -80,9 +95,95 @@ export default class Avatar extends Component {
             } else {
                 this.state = nextState;
             }
+
+            // XXX @lyubomir: My logic for the character # bellow is as follows:
+            // - Technically, URI is supposed to start with a scheme and scheme
+            //   cannot contain the character #.
+            // - Technically, the character # in URI signals the start of the
+            //   fragment/hash.
+            // - Technically, the fragment/hash does not imply a retrieval
+            //   action.
+            // - Practically, the fragment/hash does not always mandate a
+            //   retrieval action. For example, an HTML anchor with an href that
+            //   starts with the character # does not cause a Web browser to
+            //   initiate a retrieval action.
+            // So I'll use the character # at the start of URI to not initiate
+            // an image retrieval action.
+            if (nextURI && !nextURI.startsWith('#')) {
+                const nextSource = { uri: nextURI };
+
+                // Wait for the source/URI to load.
+                ImageCache.get().on(
+                    nextSource,
+                    /* observer */ () => {
+                        this._unmounted || this.setState((prevState, props) => {
+                            if (props.uri === nextURI
+                                    && (!prevState.source
+                                        || prevState.source.uri !== nextURI)) {
+                                return { source: nextSource };
+                            }
+
+                            return {};
+                        });
+                    },
+                    /* immutable */ true);
+            }
         }
     }
 
+    /**
+     * Notifies this <tt>Component</tt> that it will be unmounted and destroyed
+     * and, most importantly, that it should no longer call
+     * {@link #setState(Object)}. <tt>Avatar</tt> needs it because it downloads
+     * images via {@link ImageCache} which will asynchronously notify about
+     * success.
+     *
+     * @inheritdoc
+     * @returns {void}
+     */
+    componentWillUnmount() {
+        this._unmounted = true;
+    }
+
+    /**
+     * Computes a hash over the URI and returns a HSL background color. We use
+     * 75% as lightness, for nice pastel style colors.
+     *
+     * @param {Object} props - The read-only React <tt>Component</tt> props from
+     * which the background color is to be generated.
+     * @private
+     * @returns {string} - The HSL CSS property.
+     */
+    _getBackgroundColor({ uri }) {
+        if (!uri) {
+            // @lyubomir: I'm leaving @saghul's implementation which picks up a
+            // random color bellow so that we have it in the source code in
+            // case we decide to use it in the future. However, I think at the
+            // time of this writing that the randomness reduces the
+            // predictability which React is supposed to bring to our app.
+            return ColorPalette.white;
+        }
+
+        let hash = 0;
+
+        if (typeof uri === 'string') {
+            /* eslint-disable no-bitwise */
+
+            for (let i = 0; i < uri.length; i++) {
+                hash = uri.charCodeAt(i) + ((hash << 5) - hash);
+                hash |= 0;  // Convert to 32-bit integer
+            }
+
+            /* eslint-enable no-bitwise */
+        } else {
+            // @saghul: If we have no URI yet, we have no data to hash from. So
+            // use a random value.
+            hash = Math.floor(Math.random() * 360);
+        }
+
+        return `hsl(${hash % 360}, 100%, 75%)`;
+    }
+
     /**
      * Implements React's {@link Component#render()}.
      *
@@ -91,16 +192,73 @@ export default class Avatar extends Component {
     render() {
         // Propagate all props of this Avatar but the ones consumed by this
         // Avatar to the Image it renders.
+        const {
+            /* eslint-disable no-unused-vars */
+
+            // The following are forked in state:
+            uri: forked0,
+
+            /* eslint-enable no-unused-vars */
+
+            style,
+            ...props
+        } = this.props;
+        const {
+            backgroundColor,
+            source
+        } = this.state;
+
+        // If we're rendering the _DEFAULT_SOURCE, then we want to do some
+        // additional fu like having automagical colors generated per
+        // participant, transparency to make the intermediate state while
+        // downloading the remote image a little less "in your face", etc.
+        let styleWithBackgroundColor;
+
+        if (source === _DEFAULT_SOURCE && backgroundColor) {
+            styleWithBackgroundColor = {
+                ...style,
+
+                backgroundColor,
 
-        // eslint-disable-next-line no-unused-vars
-        const { uri, ...props } = this.props;
+                // FIXME @lyubomir: Without the opacity bellow I feel like the
+                // avatar colors are too strong. Besides, we use opacity for the
+                // ToolbarButtons. That's where I copied the value from and we
+                // may want to think about "standardizing" the opacity in the
+                // app in a way similar to ColorPalette.
+                opacity: 0.1,
+                overflow: 'hidden'
+            };
+        }
+
+        // If we're styling with backgroundColor, we need to wrap the Image in a
+        // View because of a bug in React Native for Android:
+        // https://github.com/facebook/react-native/issues/3198
+        let imageStyle;
+        let viewStyle;
+
+        if (styleWithBackgroundColor) {
+            if (Platform.OS === 'android') {
+                imageStyle = style;
+                viewStyle = styleWithBackgroundColor;
+            } else {
+                imageStyle = styleWithBackgroundColor;
+            }
+        } else {
+            imageStyle = style;
+        }
+
+        let element = React.createElement(CachedImage, {
+            ...props,
+
+            resizeMode: 'contain',
+            source,
+            style: imageStyle
+        });
+
+        if (viewStyle) {
+            element = React.createElement(View, { style: viewStyle }, element);
+        }
 
-        return (
-            <CustomCachedImage
-                { ...props }
-                component = { AvatarImage }
-                resizeMode = 'contain'
-                source = { this.state.source } />
-        );
+        return element;
     }
 }

react/features/base/participants/components/AvatarImage.native.js

@@ -1,208 +0,0 @@
-import React, { Component } from 'react';
-import { Image, View } from 'react-native';
-
-import { Platform } from '../../react';
-
-/**
- * The default avatar to be used, in case the requested URI is not available
- * or fails to load. It is an inline version of images/avatar2.png.
- *
- * @type {string}
- */
-const DEFAULT_AVATAR = require('./defaultAvatar.png');
-
-/**
- * The number of milliseconds to wait when the avatar URI is undefined before we
- * start showing a default locally generated one. Note that since we have no
- * URI, we have nothing we can cache, so the color will be random.
- *
- * @type {number}
- */
-const UNDEFINED_AVATAR_TIMEOUT = 1000;
-
-/**
- * Implements an Image component wrapper, which returns a default image if the
- * requested one fails to load. The default image background is chosen by
- * hashing the URL of the image.
- */
-export default class AvatarImage extends Component {
-    /**
-     * AvatarImage component's property types.
-     *
-     * @static
-     */
-    static propTypes = {
-        /**
-         * If set to <tt>true</tt> it will not load the URL, but will use the
-         * default instead.
-         */
-        forceDefault: React.PropTypes.bool,
-
-        /**
-         * The source the {@link Image}.
-         */
-        source: React.PropTypes.object,
-
-        /**
-         * The optional style to add to the {@link Image} in order to customize
-         * its base look (and feel).
-         */
-        style: React.PropTypes.object
-    };
-
-    /**
-     * Initializes new AvatarImage component.
-     *
-     * @param {Object} props - Component props.
-     */
-    constructor(props) {
-        super(props);
-
-        this.state = {
-            failed: false,
-            showDefault: false
-        };
-
-        this.componentWillReceiveProps(props);
-
-        this._onError = this._onError.bind(this);
-    }
-
-    /**
-     * Notifies this mounted React Component that it will receive new props.
-     * If the URI is undefined, wait {@code UNDEFINED_AVATAR_TIMEOUT} ms and
-     * start showing a default locally generated avatar afterwards.
-     *
-     * Once a URI is passed, it will be rendered instead, except if loading it
-     * fails, in which case we fallback to a locally generated avatar again.
-     *
-     * @inheritdoc
-     * @param {Object} nextProps - The read-only React Component props that this
-     * instance will receive.
-     * @returns {void}
-     */
-    componentWillReceiveProps(nextProps) {
-        const prevSource = this.props.source;
-        const prevURI = prevSource && prevSource.uri;
-        const nextSource = nextProps.source;
-        const nextURI = nextSource && nextSource.uri;
-
-        if (typeof prevURI === 'undefined') {
-            clearTimeout(this._timeout);
-            if (typeof nextURI === 'undefined') {
-                this._timeout
-                    = setTimeout(
-                        () => this.setState({ showDefault: true }),
-                        UNDEFINED_AVATAR_TIMEOUT);
-            } else {
-                this.setState({ showDefault: nextProps.forceDefault });
-            }
-        }
-    }
-
-    /**
-     * Clear the timer just in case. See {@code componentWillReceiveProps} for
-     * details.
-     *
-     * @inheritdoc
-     */
-    componentWillUnmount() {
-        clearTimeout(this._timeout);
-    }
-
-    /**
-     * Computes a hash over the URI and returns a HSL background color. We use
-     * 75% as lightness, for nice pastel style colors.
-     *
-     * @private
-     * @returns {string} - The HSL CSS property.
-     */
-    _getBackgroundColor() {
-        const uri = this.props.source.uri;
-        let hash = 0;
-
-        // If we have no URI yet we have no data to hash from, so use a random
-        // value.
-        if (typeof uri === 'undefined') {
-            hash = Math.floor(Math.random() * 360);
-        } else {
-            /* eslint-disable no-bitwise */
-
-            for (let i = 0; i < uri.length; i++) {
-                hash = uri.charCodeAt(i) + ((hash << 5) - hash);
-                hash |= 0;  // Convert to 32bit integer
-            }
-
-            /* eslint-enable no-bitwise */
-        }
-
-        return `hsl(${hash % 360}, 100%, 75%)`;
-    }
-
-    /**
-     * Error handler for image loading. When an image fails to load we'll mark
-     * it as failed and load the default URI instead.
-     *
-     * @private
-     * @returns {void}
-     */
-    _onError() {
-        this.setState({ failed: true });
-    }
-
-    /**
-     * Implements React's {@link Component#render()}.
-     *
-     * @inheritdoc
-     */
-    render() {
-        const { failed, showDefault } = this.state;
-        const {
-            // The following is/are forked in state:
-            forceDefault, // eslint-disable-line no-unused-vars
-
-            source,
-            style,
-            ...props
-        } = this.props;
-
-        if (failed || showDefault) {
-            const coloredBackground = {
-                ...style,
-                backgroundColor: this._getBackgroundColor(),
-                overflow: 'hidden'
-            };
-
-            // We need to wrap the Image in a View because of a bug in React
-            // Native for Android:
-            // https://github.com/facebook/react-native/issues/3198
-            const workaround3198 = Platform.OS === 'android';
-            let element = React.createElement(Image, {
-                ...props,
-                source: DEFAULT_AVATAR,
-                style: workaround3198 ? style : coloredBackground
-            });
-
-            if (workaround3198) {
-                element
-                    = React.createElement(
-                        View,
-                        { style: coloredBackground },
-                        element);
-            }
-
-            return element;
-        } else if (typeof source.uri === 'undefined') {
-            return null;
-        }
-
-        // We have a URI and it's time to render it.
-        return (
-            <Image
-                { ...props }
-                onError = { this._onError }
-                source = { source }
-                style = { style } />
-        );
-    }
-}

react/features/base/participants/components/ParticipantView.native.js

@@ -198,6 +198,17 @@ function _mapStateToProps(state, ownProps) {
     if (participant) {
         avatar = getAvatarURL(participant);
         connectionStatus = participant.connectionStatus;
+
+        // Avatar (on React Native) now has the ability to generate an
+        // automatically-colored default image when no URI/URL is specified or
+        // when it fails to load. In order to make the coloring permanent(ish)
+        // per participant, Avatar will need something permanent(ish) per
+        // perticipant, obviously. A participant's ID is such a piece of data.
+        // But the local participant changes her ID as she joins, leaves.
+        // TODO @lyubomir: The participants may change their avatar URLs at
+        // runtime which means that, if their old and new avatar URLs fail to
+        // download, Avatar will change their automatically-generated colors.
+        avatar || participant.local || (avatar = `#${participant.id}`);
     }
 
     return {