From 61c16569cb327be350dbd67a81f95e51fb8c18a5 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Mon, 24 Jul 2017 17:18:29 +0100 Subject: [PATCH 1/4] Get user pill profile remote data Instead of relying on the local avatar/displayname of a user, request the data from the server and update the pill if it shows up. This required a slight refactor which means we're not doing everything in `render` now. Also I noticed unknown rooms weren't being rendered _at all_! So now you get something that looks like a normal link but with the room alias/ID in it. --- src/components/views/elements/Pill.js | 133 ++++++++++++++++++++------ 1 file changed, 102 insertions(+), 31 deletions(-) diff --git a/src/components/views/elements/Pill.js b/src/components/views/elements/Pill.js index 39987228e5..b7d775fff7 100644 --- a/src/components/views/elements/Pill.js +++ b/src/components/views/elements/Pill.js @@ -28,7 +28,7 @@ const REGEX_MATRIXTO = new RegExp(MATRIXTO_URL_PATTERN); // HttpUtils transformTags to relative links const REGEX_LOCAL_MATRIXTO = /^#\/(?:user|room)\/(([\#\!\@\+]).*)$/; -export default React.createClass({ +const Pill = React.createClass({ statics: { isPillUrl: (url) => { return !!REGEX_MATRIXTO.exec(url); @@ -36,6 +36,8 @@ export default React.createClass({ isMessagePillUrl: (url) => { return !!REGEX_LOCAL_MATRIXTO.exec(url); }, + TYPE_USER_MENTION: 'TYPE_USER_MENTION', + TYPE_ROOM_MENTION: 'TYPE_ROOM_MENTION', }, props: { @@ -47,10 +49,21 @@ export default React.createClass({ room: PropTypes.instanceOf(Room), }, - render: function() { - const MemberAvatar = sdk.getComponent('avatars.MemberAvatar'); - const RoomAvatar = sdk.getComponent('avatars.RoomAvatar'); + getInitialState() { + return { + // ID/alias of the room/user + resourceId: null, + // Type of pill + pillType: null, + // The member related to the user pill + member: null, + // The room related to the room pill + room: null, + }; + }, + + componentWillMount() { let regex = REGEX_MATRIXTO; if (this.props.inMessage) { regex = REGEX_LOCAL_MATRIXTO; @@ -60,46 +73,102 @@ export default React.createClass({ // resource and prefix will be undefined instead of throwing const matrixToMatch = regex.exec(this.props.url) || []; - const resource = matrixToMatch[1]; // The room/user ID + const resourceId = matrixToMatch[1]; // The room/user ID const prefix = matrixToMatch[2]; // The first character of prefix - // Default to the room/user ID - let linkText = resource; + const pillType = { + '@': Pill.TYPE_USER_MENTION, + '#': Pill.TYPE_ROOM_MENTION, + '!': Pill.TYPE_ROOM_MENTION, + }[prefix]; - const isUserPill = prefix === '@'; - const isRoomPill = prefix === '#' || prefix === '!'; + let member; + let room; + switch (pillType) { + case Pill.TYPE_USER_MENTION: { + const localMember = this.props.room.getMember(resourceId); + member = localMember; + if (!localMember) { + member = new RoomMember(null, resourceId); + this.doProfileLookup(resourceId, member); + } + } + break; + case Pill.TYPE_ROOM_MENTION: { + const localRoom = resourceId[0] === '#' ? + MatrixClientPeg.get().getRooms().find((r) => { + return r.getAliases().includes(resourceId); + }) : MatrixClientPeg.get().getRoom(resourceId); + room = localRoom; + if (!localRoom) { + // TODO: This would require a new API to resolve a room alias to + // a room avatar and name. + // this.doRoomProfileLookup(resourceId, member); + } + } + break; + } + this.setState({resourceId, pillType, member, room}); + }, + + doProfileLookup: function(userId, member) { + MatrixClientPeg.get().getProfileInfo(userId).done((resp) => { + member.name = resp.displayname; + member.rawDisplayName = resp.displayname; + member.events.member = { + getContent: () => { + return {avatar_url: resp.avatar_url}; + }, + }; + + this.setState({member}); + }, (err) => { + console.error( + 'Could not retrieve profile data for ' + userId + ':', + err, + ); + }); + }, + + render: function() { + const MemberAvatar = sdk.getComponent('avatars.MemberAvatar'); + const RoomAvatar = sdk.getComponent('avatars.RoomAvatar'); + + const resource = this.state.resourceId; let avatar = null; + let linkText = resource; + let pillClass; let userId; - if (isUserPill) { - // If this user is not a member of this room, default to the empty member - // TODO: This could be improved by doing an async profile lookup - const member = this.props.room.getMember(resource) || - new RoomMember(null, resource); - if (member) { - userId = member.userId; - linkText = member.name; - avatar = ; + switch (this.state.pillType) { + case Pill.TYPE_USER_MENTION: { + // If this user is not a member of this room, default to the empty member + // TODO: This could be improved by doing an async profile lookup + const member = this.state.member; + if (member) { + userId = member.userId; + linkText = member.name; + avatar = ; + pillClass = 'mx_UserPill'; + } } - } else if (isRoomPill) { - const room = prefix === '#' ? - MatrixClientPeg.get().getRooms().find((r) => { - return r.getAliases().includes(resource); - }) : MatrixClientPeg.get().getRoom(resource); - - if (room) { - linkText = (room ? getDisplayAliasForRoom(room) : null) || resource; - avatar = ; + break; + case Pill.TYPE_ROOM_MENTION: { + const room = this.state.room; + if (room) { + linkText = (room ? getDisplayAliasForRoom(room) : null) || resource; + avatar = ; + pillClass = 'mx_RoomPill'; + } } + break; } - const classes = classNames({ - "mx_UserPill": isUserPill, - "mx_RoomPill": isRoomPill, + const classes = classNames(pillClass, { "mx_UserPill_me": userId === MatrixClientPeg.get().credentials.userId, }); - if ((isUserPill || isRoomPill) && avatar) { + if (this.state.pillType) { return this.props.inMessage ? {avatar} @@ -115,3 +184,5 @@ export default React.createClass({ } }, }); + +export default Pill; From 5efd5bf9279ff6fa81c3cc7b040f2da5a87932c9 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Tue, 25 Jul 2017 09:20:14 +0100 Subject: [PATCH 2/4] done -> then, style --- src/components/views/elements/Pill.js | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/components/views/elements/Pill.js b/src/components/views/elements/Pill.js index b7d775fff7..b7731182f9 100644 --- a/src/components/views/elements/Pill.js +++ b/src/components/views/elements/Pill.js @@ -112,7 +112,7 @@ const Pill = React.createClass({ }, doProfileLookup: function(userId, member) { - MatrixClientPeg.get().getProfileInfo(userId).done((resp) => { + MatrixClientPeg.get().getProfileInfo(userId).then((resp) => { member.name = resp.displayname; member.rawDisplayName = resp.displayname; member.events.member = { @@ -120,13 +120,9 @@ const Pill = React.createClass({ return {avatar_url: resp.avatar_url}; }, }; - this.setState({member}); - }, (err) => { - console.error( - 'Could not retrieve profile data for ' + userId + ':', - err, - ); + }).catch((err) => { + console.error('Could not retrieve profile data for ' + userId + ':', err); }); }, From 026582bcf7a231d72b2be3abcbeb099ab368c952 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Tue, 25 Jul 2017 09:22:08 +0100 Subject: [PATCH 3/4] Add unmounted guard --- src/components/views/elements/Pill.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/components/views/elements/Pill.js b/src/components/views/elements/Pill.js index b7731182f9..660c5604e5 100644 --- a/src/components/views/elements/Pill.js +++ b/src/components/views/elements/Pill.js @@ -64,6 +64,7 @@ const Pill = React.createClass({ }, componentWillMount() { + this._unmounted = false; let regex = REGEX_MATRIXTO; if (this.props.inMessage) { regex = REGEX_LOCAL_MATRIXTO; @@ -111,8 +112,15 @@ const Pill = React.createClass({ this.setState({resourceId, pillType, member, room}); }, + componentWillUnmount() { + this._unmounted = true; + }, + doProfileLookup: function(userId, member) { MatrixClientPeg.get().getProfileInfo(userId).then((resp) => { + if (this._unmounted) { + return; + } member.name = resp.displayname; member.rawDisplayName = resp.displayname; member.events.member = { From ac597996e009f51adc618cb397843f3d1ce8278c Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Tue, 25 Jul 2017 09:37:18 +0100 Subject: [PATCH 4/4] Remove outdated comment --- src/components/views/elements/Pill.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/views/elements/Pill.js b/src/components/views/elements/Pill.js index 660c5604e5..45c6a03ad4 100644 --- a/src/components/views/elements/Pill.js +++ b/src/components/views/elements/Pill.js @@ -147,7 +147,6 @@ const Pill = React.createClass({ switch (this.state.pillType) { case Pill.TYPE_USER_MENTION: { // If this user is not a member of this room, default to the empty member - // TODO: This could be improved by doing an async profile lookup const member = this.state.member; if (member) { userId = member.userId;