From cfb81a4aecdc6ae20866a7cd0e7ff1a5e77626ad Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Fri, 15 Jan 2016 12:02:28 +0000 Subject: [PATCH 1/3] Factor out avatar stuff to BaseAvatar. Make MemberAvatar use it instead. --- src/component-index.js | 1 + src/components/views/avatars/BaseAvatar.js | 131 +++++++++++++++++++ src/components/views/avatars/MemberAvatar.js | 86 +++--------- 3 files changed, 148 insertions(+), 70 deletions(-) create mode 100644 src/components/views/avatars/BaseAvatar.js diff --git a/src/component-index.js b/src/component-index.js index 7ae15ba12c..5fcf8e1ce0 100644 --- a/src/component-index.js +++ b/src/component-index.js @@ -32,6 +32,7 @@ module.exports.components['structures.RoomView'] = require('./components/structu module.exports.components['structures.ScrollPanel'] = require('./components/structures/ScrollPanel'); module.exports.components['structures.UploadBar'] = require('./components/structures/UploadBar'); module.exports.components['structures.UserSettings'] = require('./components/structures/UserSettings'); +module.exports.components['views.avatars.BaseAvatar'] = require('./components/views/avatars/BaseAvatar'); module.exports.components['views.avatars.MemberAvatar'] = require('./components/views/avatars/MemberAvatar'); module.exports.components['views.avatars.RoomAvatar'] = require('./components/views/avatars/RoomAvatar'); module.exports.components['views.create_room.CreateRoomButton'] = require('./components/views/create_room/CreateRoomButton'); diff --git a/src/components/views/avatars/BaseAvatar.js b/src/components/views/avatars/BaseAvatar.js new file mode 100644 index 0000000000..0472b1c651 --- /dev/null +++ b/src/components/views/avatars/BaseAvatar.js @@ -0,0 +1,131 @@ +/* +Copyright 2015, 2016 OpenMarket Ltd + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +'use strict'; + +var React = require('react'); +var AvatarLogic = require("../../../Avatar"); + +module.exports = React.createClass({ + displayName: 'BaseAvatar', + + propTypes: { + name: React.PropTypes.string.isRequired, + idName: React.PropTypes.string, // ID for generating hash colours + title: React.PropTypes.string, + url: React.PropTypes.string, // highest priority of them all + urls: React.PropTypes.array, // [highest_priority, ... , lowest_priority] + width: React.PropTypes.number, + height: React.PropTypes.number, + resizeMethod: React.PropTypes.string, + defaultToInitialLetter: React.PropTypes.bool + }, + + getDefaultProps: function() { + return { + width: 40, + height: 40, + resizeMethod: 'crop', + defaultToInitialLetter: true + } + }, + + getInitialState: function() { + var defaultImageUrl = null; + if (this.props.defaultToInitialLetter) { + defaultImageUrl = AvatarLogic.defaultAvatarUrlForString( + this.props.idName || this.props.name + ); + } + return { + imageUrl: this.props.url || (this.props.urls ? this.props.urls[0] : null), + defaultImageUrl: defaultImageUrl, + urlsIndex: 0 + }; + }, + + componentWillReceiveProps: function(nextProps) { + // retry all the urls again, they may have changed. + if (this.props.urls && this.state.urlsIndex > 0) { + this.setState({ + urlsIndex: 0, + imageUrl: this.props.urls[0] + }); + } + }, + + onError: function(ev) { + var failedUrl = ev.target.src; + + if (this.props.urls) { + var nextIndex = this.state.urlsIndex + 1; + if (nextIndex < this.props.urls.length) { + // try another + this.setState({ + urlsIndex: nextIndex, + imageUrl: this.props.urls[nextIndex] + }); + return; + } + } + + // either no urls array or we've reached the end of it, we may have a default + // we can use... + if (this.props.defaultToInitialLetter) { + if (failedUrl === this.state.defaultImageUrl) { + return; // don't tightloop if the browser can't load the default URL + } + this.setState({ imageUrl: this.state.defaultImageUrl }) + } + }, + + _getInitialLetter: function() { + var name = this.props.name; + var initial = name[0]; + if (initial === '@' && name[1]) { + initial = name[1]; + } + return initial.toUpperCase(); + }, + + render: function() { + var name = this.props.name; + + if (this.state.imageUrl === this.state.defaultImageUrl) { + var initialLetter = this._getInitialLetter(); + return ( + + + + + ); + } + return ( + + ); + } +}); diff --git a/src/components/views/avatars/MemberAvatar.js b/src/components/views/avatars/MemberAvatar.js index f209006b1c..5e2dbbb23a 100644 --- a/src/components/views/avatars/MemberAvatar.js +++ b/src/components/views/avatars/MemberAvatar.js @@ -18,22 +18,16 @@ limitations under the License. var React = require('react'); var Avatar = require('../../../Avatar'); -var MatrixClientPeg = require('../../../MatrixClientPeg'); +var sdk = require("../../../index"); module.exports = React.createClass({ displayName: 'MemberAvatar', propTypes: { - member: React.PropTypes.object, + member: React.PropTypes.object.isRequired, width: React.PropTypes.number, height: React.PropTypes.number, - resizeMethod: React.PropTypes.string, - /** - * The custom display name to use for this member. This can serve as a - * drop in replacement for RoomMember objects, or as a clobber name on - * an existing RoomMember. Used for 3pid invites. - */ - customDisplayName: React.PropTypes.string + resizeMethod: React.PropTypes.string }, getDefaultProps: function() { @@ -45,77 +39,29 @@ module.exports = React.createClass({ }, getInitialState: function() { - var defaultImageUrl = Avatar.defaultAvatarUrlForString( - this.props.customDisplayName || this.props.member.userId - ) - return { - imageUrl: this._getMemberImageUrl() || defaultImageUrl, - defaultImageUrl: defaultImageUrl - }; + return this._getState(this.props); }, componentWillReceiveProps: function(nextProps) { - this.refreshUrl(); + this.setState(this._getState(nextProps)); }, - onError: function(ev) { - // don't tightloop if the browser can't load a data url - if (ev.target.src == this.state.defaultImageUrl) { - return; - } - this.setState({ - imageUrl: this.state.defaultImageUrl - }); - }, - - _getMemberImageUrl: function() { - if (!this.props.member) { return null; } - - return Avatar.avatarUrlForMember(this.props.member, - this.props.width, - this.props.height, - this.props.resizeMethod); - }, - - _getInitialLetter: function() { - var name = this.props.customDisplayName || this.props.member.name; - var initial = name[0]; - if (initial === '@' && name[1]) { - initial = name[1]; - } - return initial.toUpperCase(); - }, - - refreshUrl: function() { - var newUrl = this._getMemberImageUrl(); - if (newUrl != this.currentUrl) { - this.currentUrl = newUrl; - this.setState({imageUrl: newUrl}); + _getState: function(props) { + return { + name: props.member.name, + title: props.member.userId, + imageUrl: Avatar.avatarUrlForMember(props.member, + props.width, + props.height, + props.resizeMethod) } }, render: function() { - var name = this.props.customDisplayName || this.props.member.name; - - if (this.state.imageUrl === this.state.defaultImageUrl) { - var initialLetter = this._getInitialLetter(); - return ( - - - - - ); - } + var BaseAvatar = sdk.getComponent("avatars.BaseAvatar"); return ( - + ); } }); From d8d79722ac2d5901a392cdd9d4a9fdd8647b9771 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Fri, 15 Jan 2016 16:13:25 +0000 Subject: [PATCH 2/3] Make RoomAvatar use BaseAvatar --- src/components/views/avatars/BaseAvatar.js | 14 +-- src/components/views/avatars/RoomAvatar.js | 119 ++++-------------- src/components/views/rooms/MemberTile.js | 16 ++- src/components/views/rooms/RoomHeader.js | 2 +- src/components/views/rooms/RoomTile.js | 2 +- src/components/views/settings/ChangeAvatar.js | 2 +- 6 files changed, 46 insertions(+), 109 deletions(-) diff --git a/src/components/views/avatars/BaseAvatar.js b/src/components/views/avatars/BaseAvatar.js index 0472b1c651..7291d42bdc 100644 --- a/src/components/views/avatars/BaseAvatar.js +++ b/src/components/views/avatars/BaseAvatar.js @@ -23,9 +23,9 @@ module.exports = React.createClass({ displayName: 'BaseAvatar', propTypes: { - name: React.PropTypes.string.isRequired, + name: React.PropTypes.string.isRequired, // The name (first initial used as default) idName: React.PropTypes.string, // ID for generating hash colours - title: React.PropTypes.string, + title: React.PropTypes.string, // onHover title text url: React.PropTypes.string, // highest priority of them all urls: React.PropTypes.array, // [highest_priority, ... , lowest_priority] width: React.PropTypes.number, @@ -95,7 +95,7 @@ module.exports = React.createClass({ _getInitialLetter: function() { var name = this.props.name; var initial = name[0]; - if (initial === '@' && name[1]) { + if ((initial === '@' || initial === '#') && name[1]) { initial = name[1]; } return initial.toUpperCase(); @@ -107,21 +107,21 @@ module.exports = React.createClass({ if (this.state.imageUrl === this.state.defaultImageUrl) { var initialLetter = this._getInitialLetter(); return ( - - ); } return ( - - - - - ); - } - else { - return - } + var BaseAvatar = sdk.getComponent("avatars.BaseAvatar"); + return ( + + ); } }); diff --git a/src/components/views/rooms/MemberTile.js b/src/components/views/rooms/MemberTile.js index 4752c4d539..304d759ea7 100644 --- a/src/components/views/rooms/MemberTile.js +++ b/src/components/views/rooms/MemberTile.js @@ -151,14 +151,26 @@ module.exports = React.createClass({ } var MemberAvatar = sdk.getComponent('avatars.MemberAvatar'); + var BaseAvatar = sdk.getComponent('avatars.BaseAvatar'); + + var av; + if (member) { + av = ( + + ); + } + else { + av = ( + + ); + } return (
- + {av}
{ nameEl }
diff --git a/src/components/views/rooms/RoomHeader.js b/src/components/views/rooms/RoomHeader.js index cdfbc0bfc8..a05c6c30ac 100644 --- a/src/components/views/rooms/RoomHeader.js +++ b/src/components/views/rooms/RoomHeader.js @@ -129,7 +129,7 @@ module.exports = React.createClass({ var roomAvatar = null; if (this.props.room) { roomAvatar = ( - + ); } diff --git a/src/components/views/rooms/RoomTile.js b/src/components/views/rooms/RoomTile.js index 16d798cb87..29b35c040c 100644 --- a/src/components/views/rooms/RoomTile.js +++ b/src/components/views/rooms/RoomTile.js @@ -123,7 +123,7 @@ module.exports = React.createClass({ return connectDragSource(connectDropTarget(
- + { badge }
{ label } diff --git a/src/components/views/settings/ChangeAvatar.js b/src/components/views/settings/ChangeAvatar.js index f5ec6a0467..979b4b3c1b 100644 --- a/src/components/views/settings/ChangeAvatar.js +++ b/src/components/views/settings/ChangeAvatar.js @@ -111,7 +111,7 @@ module.exports = React.createClass({ // Having just set an avatar we just display that since it will take a little // time to propagate through to the RoomAvatar. if (this.props.room && !this.avatarSet) { - avatarImg = ; + avatarImg = ; } else { var style = { maxWidth: 240, From 855bef17faaca477d2ae9e8ac296c6390f419556 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Fri, 15 Jan 2016 17:05:05 +0000 Subject: [PATCH 3/3] Fold BaseAvatar state mainly into state.urls to avoid too many code paths. --- src/components/views/avatars/BaseAvatar.js | 87 ++++++++++++---------- src/components/views/avatars/RoomAvatar.js | 50 ++++++++----- 2 files changed, 78 insertions(+), 59 deletions(-) diff --git a/src/components/views/avatars/BaseAvatar.js b/src/components/views/avatars/BaseAvatar.js index 7291d42bdc..2a7dcc1c01 100644 --- a/src/components/views/avatars/BaseAvatar.js +++ b/src/components/views/avatars/BaseAvatar.js @@ -26,12 +26,12 @@ module.exports = React.createClass({ name: React.PropTypes.string.isRequired, // The name (first initial used as default) idName: React.PropTypes.string, // ID for generating hash colours title: React.PropTypes.string, // onHover title text - url: React.PropTypes.string, // highest priority of them all + url: React.PropTypes.string, // highest priority of them all, shortcut to set in urls[0] urls: React.PropTypes.array, // [highest_priority, ... , lowest_priority] width: React.PropTypes.number, height: React.PropTypes.number, resizeMethod: React.PropTypes.string, - defaultToInitialLetter: React.PropTypes.bool + defaultToInitialLetter: React.PropTypes.bool // true to add default url }, getDefaultProps: function() { @@ -44,51 +44,58 @@ module.exports = React.createClass({ }, getInitialState: function() { + return this._getState(this.props); + }, + + componentWillReceiveProps: function(nextProps) { + // work out if we need to call setState (if the image URLs array has changed) + var newState = this._getState(nextProps); + var newImageUrls = newState.imageUrls; + var oldImageUrls = this.state.imageUrls; + if (newImageUrls.length !== oldImageUrls.length) { + this.setState(newState); // detected a new entry + } + else { + // check each one to see if they are the same + for (var i = 0; i < newImageUrls.length; i++) { + if (oldImageUrls[i] !== newImageUrls[i]) { + this.setState(newState); // detected a diff + break; + } + } + } + }, + + _getState: function(props) { + // work out the full set of urls to try to load. This is formed like so: + // imageUrls: [ props.url, props.urls, default image ] + + var urls = props.urls || []; + if (props.url) { + urls.unshift(props.url); // put in urls[0] + } + var defaultImageUrl = null; - if (this.props.defaultToInitialLetter) { + if (props.defaultToInitialLetter) { defaultImageUrl = AvatarLogic.defaultAvatarUrlForString( - this.props.idName || this.props.name + props.idName || props.name ); + urls.push(defaultImageUrl); // lowest priority } return { - imageUrl: this.props.url || (this.props.urls ? this.props.urls[0] : null), + imageUrls: urls, defaultImageUrl: defaultImageUrl, urlsIndex: 0 }; }, - componentWillReceiveProps: function(nextProps) { - // retry all the urls again, they may have changed. - if (this.props.urls && this.state.urlsIndex > 0) { - this.setState({ - urlsIndex: 0, - imageUrl: this.props.urls[0] - }); - } - }, - onError: function(ev) { - var failedUrl = ev.target.src; - - if (this.props.urls) { - var nextIndex = this.state.urlsIndex + 1; - if (nextIndex < this.props.urls.length) { - // try another - this.setState({ - urlsIndex: nextIndex, - imageUrl: this.props.urls[nextIndex] - }); - return; - } - } - - // either no urls array or we've reached the end of it, we may have a default - // we can use... - if (this.props.defaultToInitialLetter) { - if (failedUrl === this.state.defaultImageUrl) { - return; // don't tightloop if the browser can't load the default URL - } - this.setState({ imageUrl: this.state.defaultImageUrl }) + var nextIndex = this.state.urlsIndex + 1; + if (nextIndex < this.state.imageUrls.length) { + // try the next one + this.setState({ + urlsIndex: nextIndex + }); } }, @@ -104,7 +111,9 @@ module.exports = React.createClass({ render: function() { var name = this.props.name; - if (this.state.imageUrl === this.state.defaultImageUrl) { + var imageUrl = this.state.imageUrls[this.state.urlsIndex]; + + if (imageUrl === this.state.defaultImageUrl) { var initialLetter = this._getInitialLetter(); return ( @@ -114,14 +123,14 @@ module.exports = React.createClass({ lineHeight: this.props.height + "px" }}> { initialLetter } - ); } return ( -