From ebc199353005e6f5cabc1cbfa34d8e7badc1c574 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Wed, 30 Aug 2017 10:55:25 +0100 Subject: [PATCH 01/10] Implement Flair Add 14x14 group avatars next to sender profiles. --- src/components/views/elements/Flair.js | 179 ++++++++++++++++++ .../views/messages/SenderProfile.js | 8 +- src/components/views/rooms/EventTile.js | 4 +- 3 files changed, 187 insertions(+), 4 deletions(-) create mode 100644 src/components/views/elements/Flair.js diff --git a/src/components/views/elements/Flair.js b/src/components/views/elements/Flair.js new file mode 100644 index 0000000000..6ec9f36424 --- /dev/null +++ b/src/components/views/elements/Flair.js @@ -0,0 +1,179 @@ +/* + Copyright 2017 New Vector 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'; + +import React from 'react'; +import PropTypes from 'prop-types'; +import {MatrixClient} from 'matrix-js-sdk'; + +const BULK_REQUEST_DEBOUNCE_MS = 200; + +// TODO: Cache-busting based on time. (The server won't inform us of membership changes.) +// This applies to userGroups and groupProfiles. We can provide a slightly better UX by +// cache-busting when the current user joins/leaves a group. +const userGroups = { + // $userId: ['+group1:domain', '+group2:domain', ...] +}; + +const groupProfiles = { +// $groupId: { +// avatar_url: 'mxc://...' +// } +}; + +// Represents all unsettled promises to retrieve the groups for each userId. When a promise +// is settled, it is deleted from this object. +const usersPending = { +// $userId: { +// prom: Promise +// resolve: () => {} +// reject: () => {} +// } +}; + +let debounceTimeoutID; +function getPublicGroupsCached(matrixClient, userId) { + if (userGroups[userId]) { + return Promise.resolve(userGroups[userId]); + } + + // Bulk lookup ongoing, return promise to resolve/reject + if (usersPending[userId]) { + return usersPending[userId].prom; + } + + usersPending[userId] = {}; + usersPending[userId].prom = new Promise((resolve, reject) => { + usersPending[userId].resolve = resolve; + usersPending[userId].reject = reject; + }).then((groups) => { + userGroups[userId] = groups; + // TODO: Reset cache at this point + return userGroups[userId]; + }).catch((err) => { + throw err; + }).finally(() => { + delete usersPending[userId]; + }); + + if (debounceTimeoutID) clearTimeout(debounceTimeoutID); + debounceTimeoutID = setTimeout(() => { + batchedGetPublicGroups(matrixClient); + }, BULK_REQUEST_DEBOUNCE_MS); + + return usersPending[userId].prom; +} + +async function batchedGetPublicGroups(matrixClient) { + // Take the userIds from the keys of usersPending + const usersInFlight = Object.keys(usersPending); + let resp = { + users: [], + }; + try { + resp = await matrixClient.getPublicGroups(usersInFlight); + } catch (err) { + // Propagate the same error to all usersInFlight + usersInFlight.forEach((userId) => { + usersPending[userId].prom.reject(err); + }); + return; + } + const updatedUserGroups = resp.users; + usersInFlight.forEach((userId) => { + usersPending[userId].resolve(updatedUserGroups[userId] || []); + }); +} + +async function getGroupProfileCached(matrixClient, groupId) { + if (groupProfiles[groupId]) { + return groupProfiles[groupId]; + } + + groupProfiles[groupId] = await matrixClient.getGroupProfile(groupId); + return groupProfiles[groupId]; +} + +export default class Flair extends React.Component { + constructor() { + super(); + this.state = { + avatarUrls: [], + }; + } + + componentWillUnmount() { + this._unmounted = true; + } + + async componentWillMount() { + this._unmounted = false; + this._generateAvatars(); + } + + async _getAvatarUrls(groups) { + const profiles = []; + for (const groupId of groups) { + let groupProfile = null; + try { + groupProfile = await getGroupProfileCached(this.context.matrixClient, groupId); + } catch (err) { + console.error('Could not get profile for group', groupId, err); + } + profiles.push(groupProfile); + } + + const avatarUrls = profiles.filter((p) => p !== null).map((p) => p.avatar_url); + return avatarUrls; + } + + async _generateAvatars() { + let groups; + try { + groups = await getPublicGroupsCached(this.context.matrixClient, this.props.userId); + } catch (err) { + console.error('Could not get groups for user', this.props.userId, err); + } + const avatarUrls = await this._getAvatarUrls(groups); + if (!this.unmounted) { + this.setState({avatarUrls}); + } + } + + render() { + if (this.state.avatarUrls.length === 0) { + return
; + } + const avatars = this.state.avatarUrls.map((avatarUrl, index) => { + const httpUrl = this.context.matrixClient.mxcUrlToHttp(avatarUrl, 14, 14, 'scale', false); + return ; + }); + return ( + + {avatars} + + ); + } +} + +Flair.propTypes = { + userId: PropTypes.string, +}; + +Flair.contextTypes = { + matrixClient: React.PropTypes.instanceOf(MatrixClient).isRequired, +}; diff --git a/src/components/views/messages/SenderProfile.js b/src/components/views/messages/SenderProfile.js index e224714a27..e9d2fe62ff 100644 --- a/src/components/views/messages/SenderProfile.js +++ b/src/components/views/messages/SenderProfile.js @@ -18,6 +18,7 @@ import React from 'react'; import sdk from '../../../index'; +import Flair from '../elements/Flair.js'; export default function SenderProfile(props) { const EmojiText = sdk.getComponent('elements.EmojiText'); @@ -30,8 +31,11 @@ export default function SenderProfile(props) { } return ( - {`${name || ''} ${props.aux || ''}`} +
+ {name || ''} + {props.enableFlair ? : null} + {props.aux ? props.aux : null} +
); } diff --git a/src/components/views/rooms/EventTile.js b/src/components/views/rooms/EventTile.js index a6f8ed5542..7328cfe0b6 100644 --- a/src/components/views/rooms/EventTile.js +++ b/src/components/views/rooms/EventTile.js @@ -508,10 +508,10 @@ module.exports = withMatrixClient(React.createClass({ if (msgtype === 'm.image') aux = _t('sent an image'); else if (msgtype === 'm.video') aux = _t('sent a video'); else if (msgtype === 'm.file') aux = _t('uploaded a file'); - sender = ; + sender = ; } else { - sender = ; + sender = ; } } From 86e8a4c7e24a571a638933872d91d348c9c6c813 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Thu, 31 Aug 2017 16:44:14 +0100 Subject: [PATCH 02/10] Make componentWillMount not async This was left over from a previous refactor --- src/components/views/elements/Flair.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/elements/Flair.js b/src/components/views/elements/Flair.js index 6ec9f36424..cfabd4841e 100644 --- a/src/components/views/elements/Flair.js +++ b/src/components/views/elements/Flair.js @@ -120,7 +120,7 @@ export default class Flair extends React.Component { this._unmounted = true; } - async componentWillMount() { + componentWillMount() { this._unmounted = false; this._generateAvatars(); } From e89d52ccbf2e3dfc9a9fc1dd8b817669f7d077bf Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Thu, 31 Aug 2017 16:46:39 +0100 Subject: [PATCH 03/10] Do not get avatars when no groups were/could be retrieved --- src/components/views/elements/Flair.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/components/views/elements/Flair.js b/src/components/views/elements/Flair.js index cfabd4841e..9b0534e9b0 100644 --- a/src/components/views/elements/Flair.js +++ b/src/components/views/elements/Flair.js @@ -148,6 +148,9 @@ export default class Flair extends React.Component { } catch (err) { console.error('Could not get groups for user', this.props.userId, err); } + if (!groups || groups.length === 0) { + return; + } const avatarUrls = await this._getAvatarUrls(groups); if (!this.unmounted) { this.setState({avatarUrls}); From d84190f58d2bdc56a1c9c88e9e2cdc718979b0f6 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Thu, 31 Aug 2017 17:49:19 +0100 Subject: [PATCH 04/10] Explain Flair debounce --- src/components/views/elements/Flair.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/components/views/elements/Flair.js b/src/components/views/elements/Flair.js index 9b0534e9b0..cd0acd92f8 100644 --- a/src/components/views/elements/Flair.js +++ b/src/components/views/elements/Flair.js @@ -70,6 +70,16 @@ function getPublicGroupsCached(matrixClient, userId) { delete usersPending[userId]; }); + // This debounce will allow consecutive requests for the public groups of users that + // are sent in intervals of < BULK_REQUEST_DEBOUNCE_MS to be batched and only requested + // when no more requests are received within the next BULK_REQUEST_DEBOUNCE_MS. The naive + // implementation would do a request that only requested the groups for `userId`, leading + // to a worst and best case of 1 user per request. This implementation's worst is still + // 1 user per request but only if the requests are > BULK_REQUEST_DEBOUNCE_MS apart and the + // best case is N users per request. + // + // This is to reduce the number of requests made whilst trading off latency when viewing + // a Flair component. if (debounceTimeoutID) clearTimeout(debounceTimeoutID); debounceTimeoutID = setTimeout(() => { batchedGetPublicGroups(matrixClient); From 0ded4acba02d5b8d915241c3c63f75b0370f50cf Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Thu, 31 Aug 2017 17:52:53 +0100 Subject: [PATCH 05/10] Mark TODO for https://github.com/vector-im/riot-web/issues/4951 --- src/components/views/elements/Flair.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/components/views/elements/Flair.js b/src/components/views/elements/Flair.js index cd0acd92f8..9abea0d202 100644 --- a/src/components/views/elements/Flair.js +++ b/src/components/views/elements/Flair.js @@ -187,6 +187,9 @@ Flair.propTypes = { userId: PropTypes.string, }; +// TODO: We've decided that all components should follow this pattern, which means removing withMatrixClient and using +// this.context.matrixClient everywhere instead of this.props.matrixClient. +// See https://github.com/vector-im/riot-web/issues/4951. Flair.contextTypes = { matrixClient: React.PropTypes.instanceOf(MatrixClient).isRequired, }; From 548e5f516c7dcaff2c4608d2209263e5f44c82f2 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Mon, 18 Sep 2017 14:38:41 +0100 Subject: [PATCH 06/10] Put flair into labs --- src/UserSettingsStore.js | 6 ++++++ src/components/views/elements/Flair.js | 5 ++++- src/i18n/strings/en_EN.json | 3 ++- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/UserSettingsStore.js b/src/UserSettingsStore.js index 68a1ba229f..1d1924cd23 100644 --- a/src/UserSettingsStore.js +++ b/src/UserSettingsStore.js @@ -33,11 +33,17 @@ export default { // XXX: Always use default, ignore localStorage and remove from labs override: true, }, + { + name: "-", + id: 'feature_flair', + default: false, + }, ], // horrible but it works. The locality makes this somewhat more palatable. doTranslations: function() { this.LABS_FEATURES[0].name = _t("Matrix Apps"); + this.LABS_FEATURES[1].name = _t("Flair"); }, loadProfileInfo: function() { diff --git a/src/components/views/elements/Flair.js b/src/components/views/elements/Flair.js index 9abea0d202..52eff46f58 100644 --- a/src/components/views/elements/Flair.js +++ b/src/components/views/elements/Flair.js @@ -19,6 +19,7 @@ import React from 'react'; import PropTypes from 'prop-types'; import {MatrixClient} from 'matrix-js-sdk'; +import UserSettingsStore from '../../../UserSettingsStore'; const BULK_REQUEST_DEBOUNCE_MS = 200; @@ -132,7 +133,9 @@ export default class Flair extends React.Component { componentWillMount() { this._unmounted = false; - this._generateAvatars(); + if (UserSettingsStore.isFeatureEnabled('feature_flair')) { + this._generateAvatars(); + } } async _getAvatarUrls(groups) { diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 63e86aff16..fc346fcdea 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -872,5 +872,6 @@ "Leave Group": "Leave Group", "Leave %(groupName)s?": "Leave %(groupName)s?", "%(widgetName)s widget modified by %(senderName)s": "%(widgetName)s widget modified by %(senderName)s", - "Robot check is currently unavailable on desktop - please use a web browser": "Robot check is currently unavailable on desktop - please use a web browser" + "Robot check is currently unavailable on desktop - please use a web browser": "Robot check is currently unavailable on desktop - please use a web browser", + "Flair" } From 3476cfca791a2b22c98914ac8854af5dea331dc9 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Mon, 18 Sep 2017 14:44:35 +0100 Subject: [PATCH 07/10] getPublicGroups->getPublicisedGroups To match dbkrs js-sdk wrappers for flair: matrix-org/matrix-js-sdk/pull#542 --- src/components/views/elements/Flair.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/views/elements/Flair.js b/src/components/views/elements/Flair.js index 52eff46f58..19b4f28811 100644 --- a/src/components/views/elements/Flair.js +++ b/src/components/views/elements/Flair.js @@ -47,7 +47,7 @@ const usersPending = { }; let debounceTimeoutID; -function getPublicGroupsCached(matrixClient, userId) { +function getPublicisedGroupsCached(matrixClient, userId) { if (userGroups[userId]) { return Promise.resolve(userGroups[userId]); } @@ -96,7 +96,7 @@ async function batchedGetPublicGroups(matrixClient) { users: [], }; try { - resp = await matrixClient.getPublicGroups(usersInFlight); + resp = await matrixClient.getPublicisedGroups(usersInFlight); } catch (err) { // Propagate the same error to all usersInFlight usersInFlight.forEach((userId) => { @@ -157,7 +157,7 @@ export default class Flair extends React.Component { async _generateAvatars() { let groups; try { - groups = await getPublicGroupsCached(this.context.matrixClient, this.props.userId); + groups = await getPublicisedGroupsCached(this.context.matrixClient, this.props.userId); } catch (err) { console.error('Could not get groups for user', this.props.userId, err); } From 6add06db44a0052ec89d06b83a04aba9d5415ae3 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Mon, 18 Sep 2017 15:11:49 +0100 Subject: [PATCH 08/10] Fix big with rejecting promises upon error --- src/components/views/elements/Flair.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/elements/Flair.js b/src/components/views/elements/Flair.js index 19b4f28811..6d86651560 100644 --- a/src/components/views/elements/Flair.js +++ b/src/components/views/elements/Flair.js @@ -100,7 +100,7 @@ async function batchedGetPublicGroups(matrixClient) { } catch (err) { // Propagate the same error to all usersInFlight usersInFlight.forEach((userId) => { - usersPending[userId].prom.reject(err); + usersPending[userId].reject(err); }); return; } From 241d442284d8f67a9e5e9028ba569dbfc76f85d2 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Mon, 18 Sep 2017 15:12:38 +0100 Subject: [PATCH 09/10] Fail gracefully for non-supporting servers --- src/components/views/elements/Flair.js | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/components/views/elements/Flair.js b/src/components/views/elements/Flair.js index 6d86651560..6c32933faf 100644 --- a/src/components/views/elements/Flair.js +++ b/src/components/views/elements/Flair.js @@ -23,6 +23,10 @@ import UserSettingsStore from '../../../UserSettingsStore'; const BULK_REQUEST_DEBOUNCE_MS = 200; +// Does the server support groups? Assume yes until we receive M_UNRECOGNIZED. +// If true, flair can function and we should keep sending requests for groups and avatars. +let groupSupport = true; + // TODO: Cache-busting based on time. (The server won't inform us of membership changes.) // This applies to userGroups and groupProfiles. We can provide a slightly better UX by // cache-busting when the current user joins/leaves a group. @@ -133,7 +137,7 @@ export default class Flair extends React.Component { componentWillMount() { this._unmounted = false; - if (UserSettingsStore.isFeatureEnabled('feature_flair')) { + if (UserSettingsStore.isFeatureEnabled('feature_flair') && groupSupport) { this._generateAvatars(); } } @@ -159,6 +163,13 @@ export default class Flair extends React.Component { try { groups = await getPublicisedGroupsCached(this.context.matrixClient, this.props.userId); } catch (err) { + // Indicate whether the homeserver supports groups + if (err.errcode === 'M_UNRECOGNIZED') { + console.warn('Cannot display flair, server does not support groups'); + groupSupport = false; + // Return silently to avoid spamming for non-supporting servers + return; + } console.error('Could not get groups for user', this.props.userId, err); } if (!groups || groups.length === 0) { From 925cb2dccf746befb470ac06941a2dc6f6b50e4d Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Mon, 18 Sep 2017 17:44:13 +0100 Subject: [PATCH 10/10] Do translation correctly --- src/i18n/strings/en_EN.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 90841960c3..d0df48886c 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -884,5 +884,5 @@ "Leave %(groupName)s?": "Leave %(groupName)s?", "%(widgetName)s widget modified by %(senderName)s": "%(widgetName)s widget modified by %(senderName)s", "Robot check is currently unavailable on desktop - please use a web browser": "Robot check is currently unavailable on desktop - please use a web browser", - "Flair" + "Flair": "Flair" }