From de621902fc3acc6075d48b57e8123753ac26422a Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 18 Jan 2017 15:21:50 +0000 Subject: [PATCH 1/8] Better feedback in invite dialog Show feedback if you enter a valid but unknown email address or mxid Fixes https://github.com/vector-im/riot-web/issues/2933 --- src/Invite.js | 7 +++++-- .../views/dialogs/ChatInviteDialog.js | 18 +++++++++++++++--- .../views/elements/AddressSelector.js | 4 +++- src/components/views/elements/AddressTile.js | 4 +++- 4 files changed, 26 insertions(+), 7 deletions(-) diff --git a/src/Invite.js b/src/Invite.js index 6422812734..6cb04b1b19 100644 --- a/src/Invite.js +++ b/src/Invite.js @@ -19,9 +19,12 @@ import MultiInviter from './utils/MultiInviter'; const emailRegex = /^\S+@\S+\.\S+$/; +// We allow localhost for mxids to avoid confusion +const mxidRegex = /^@\S+:(?:\S+\.\S+|localhost)$/ + export function getAddressType(inputText) { - const isEmailAddress = /^\S+@\S+\.\S+$/.test(inputText); - const isMatrixId = inputText[0] === '@' && inputText.indexOf(":") > 0; + const isEmailAddress = emailRegex.test(inputText); + const isMatrixId = mxidRegex.test(inputText); // sanity check the input for user IDs if (isEmailAddress) { diff --git a/src/components/views/dialogs/ChatInviteDialog.js b/src/components/views/dialogs/ChatInviteDialog.js index e9a041357f..f6d7c17898 100644 --- a/src/components/views/dialogs/ChatInviteDialog.js +++ b/src/components/views/dialogs/ChatInviteDialog.js @@ -154,14 +154,26 @@ module.exports = React.createClass({ }, onQueryChanged: function(ev) { - var query = ev.target.value; - var queryList = []; + const query = ev.target.value; + let queryList = []; // Only do search if there is something to search - if (query.length > 0) { + if (query.length > 0 && query != '@') { + // filter the known users list queryList = this._userList.filter((user) => { return this._matches(query, user); + }).map((user) => { + return user.userId; }); + + // If the query isn't a user we know about, but is a + // valid address, add an entry for that + if (queryList.length == 0) { + const addrType = Invite.getAddressType(query); + if (addrType !== null) { + queryList.push(query); + } + } } this.setState({ diff --git a/src/components/views/elements/AddressSelector.js b/src/components/views/elements/AddressSelector.js index 2c2d7e2d61..853b8db144 100644 --- a/src/components/views/elements/AddressSelector.js +++ b/src/components/views/elements/AddressSelector.js @@ -25,6 +25,8 @@ module.exports = React.createClass({ propTypes: { onSelected: React.PropTypes.func.isRequired, + + // List of strings: the addresses to display addressList: React.PropTypes.array.isRequired, truncateAt: React.PropTypes.number.isRequired, selected: React.PropTypes.number, @@ -125,7 +127,7 @@ module.exports = React.createClass({ // method, how far to scroll when using the arrow keys addressList.push(
{ this.addressListElement = ref; }} > - +
); } diff --git a/src/components/views/elements/AddressTile.js b/src/components/views/elements/AddressTile.js index 2799f10a41..b49a84cedd 100644 --- a/src/components/views/elements/AddressTile.js +++ b/src/components/views/elements/AddressTile.js @@ -71,6 +71,8 @@ module.exports = React.createClass({ imgUrl = "img/avatar-error.svg"; } + // Removing networks for now as they're not really supported + /* var network; if (this.props.networkUrl !== "") { network = ( @@ -79,6 +81,7 @@ module.exports = React.createClass({ ); } + */ var info; var error = false; @@ -145,7 +148,6 @@ module.exports = React.createClass({ return (
- { network }
From 5f24fc3e5da798b945adf245a21ae22df0d8b0ed Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 24 Jan 2017 13:56:22 +0000 Subject: [PATCH 2/8] Fix merge fail --- src/components/views/elements/AddressSelector.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/elements/AddressSelector.js b/src/components/views/elements/AddressSelector.js index f9d1c2e28d..a14a8ffdff 100644 --- a/src/components/views/elements/AddressSelector.js +++ b/src/components/views/elements/AddressSelector.js @@ -123,7 +123,7 @@ module.exports = React.createClass({ // Saving the addressListElement so we can use it to work out, in the componentDidUpdate // method, how far to scroll when using the arrow keys addressList.push( -
{ this.addressListElement = ref; }} > +
{ this.addressListElement = ref; }} >
); From 5091bab657f8c043b96634fb7a236b795a06894a Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 24 Jan 2017 13:59:02 +0000 Subject: [PATCH 3/8] Fix failed merge #2 --- src/components/views/elements/AddressSelector.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/elements/AddressSelector.js b/src/components/views/elements/AddressSelector.js index a14a8ffdff..361705c6d5 100644 --- a/src/components/views/elements/AddressSelector.js +++ b/src/components/views/elements/AddressSelector.js @@ -124,7 +124,7 @@ module.exports = React.createClass({ // method, how far to scroll when using the arrow keys addressList.push(
{ this.addressListElement = ref; }} > - +
); } From 6c263c1c894daed6e8d3f5dc8a04f4484706112d Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 24 Jan 2017 18:23:34 +0000 Subject: [PATCH 4/8] Change what AddressTile takes to be Objects Rather than just passing in a list of strings. This paves the way for passing in display names & avatars of looked-up threepids. --- .../views/dialogs/ChatInviteDialog.js | 74 ++++++++++----- .../views/elements/AddressSelector.js | 13 +-- src/components/views/elements/AddressTile.js | 91 +++++++++++-------- 3 files changed, 109 insertions(+), 69 deletions(-) diff --git a/src/components/views/dialogs/ChatInviteDialog.js b/src/components/views/dialogs/ChatInviteDialog.js index f8d93a3080..60aa6b0c5b 100644 --- a/src/components/views/dialogs/ChatInviteDialog.js +++ b/src/components/views/dialogs/ChatInviteDialog.js @@ -164,7 +164,13 @@ module.exports = React.createClass({ queryList = this._userList.filter((user) => { return this._matches(query, user); }).map((user) => { - return user.userId; + return { + addressType: 'mx', + address: user.userId, + displayName: user.displayName, + avatarMxc: user.avatarUrl, + isKnown: true, + } }); // If the query isn't a user we know about, but is a @@ -172,7 +178,11 @@ module.exports = React.createClass({ if (queryList.length == 0) { const addrType = Invite.getAddressType(query); if (addrType !== null) { - queryList.push(query); + queryList.push({ + addressType: addrType, + address: query, + isKnown: false, + }); } } } @@ -204,7 +214,7 @@ module.exports = React.createClass({ onSelected: function(index) { var inviteList = this.state.inviteList.slice(); - inviteList.push(this.state.queryList[index].userId); + inviteList.push(this.state.queryList[index]); this.setState({ inviteList: inviteList, queryList: [], @@ -239,10 +249,14 @@ module.exports = React.createClass({ return; } + const addrTexts = addrs.map((addr) => { + return addr.address; + }); + if (this.props.roomId) { // Invite new user to a room var self = this; - Invite.inviteMultipleToRoom(this.props.roomId, addrs) + Invite.inviteMultipleToRoom(this.props.roomId, addrTexts) .then(function(addrs) { var room = MatrixClientPeg.get().getRoom(self.props.roomId); return self._showAnyInviteErrors(addrs, room); @@ -257,9 +271,9 @@ module.exports = React.createClass({ return null; }) .done(); - } else if (this._isDmChat(addrs)) { + } else if (this._isDmChat(addrTexts)) { // Start the DM chat - createRoom({dmUserId: addrs[0]}) + createRoom({dmUserId: addrTexts[0]}) .catch(function(err) { console.error(err.stack); var ErrorDialog = sdk.getComponent("dialogs.ErrorDialog"); @@ -276,7 +290,7 @@ module.exports = React.createClass({ var room; createRoom().then(function(roomId) { room = MatrixClientPeg.get().getRoom(roomId); - return Invite.inviteMultipleToRoom(roomId, addrs); + return Invite.inviteMultipleToRoom(roomId, addrTexts); }) .then(function(addrs) { return self._showAnyInviteErrors(addrs, room); @@ -294,7 +308,7 @@ module.exports = React.createClass({ } // Close - this will happen before the above, as that is async - this.props.onFinished(true, addrs); + this.props.onFinished(true, addrTexts); }, _updateUserList: new rate_limited_func(function() { @@ -345,7 +359,10 @@ module.exports = React.createClass({ _isOnInviteList: function(uid) { for (let i = 0; i < this.state.inviteList.length; i++) { - if (this.state.inviteList[i].toLowerCase() === uid) { + if ( + this.state.inviteList[i].addressType == 'mx' && + this.state.inviteList[i].address.toLowerCase() === uid + ) { return true; } } @@ -380,24 +397,35 @@ module.exports = React.createClass({ }, _addInputToList: function() { - const addrType = Invite.getAddressType(this.refs.textinput.value); - if (addrType !== null) { - const inviteList = this.state.inviteList.slice(); - inviteList.push(this.refs.textinput.value.trim()); - this.setState({ - inviteList: inviteList, - queryList: [], - }); - return inviteList; - } else { - this.setState({ error: true }); - return null; + const addressText = this.refs.textinput.value.trim(); + const addrType = Invite.getAddressType(addressText); + const addrObj = { + addressType: addrType, + address: addressText, + isKnown: false, + }; + if (addrType == null) { + } else if (addrType == 'mx') { + const user = MatrixClientPeg.get().getUser(addrObj.address); + if (user) { + addrObj.displayName = user.displayName; + addrObj.avatarMxc = user.avatarUrl; + addrObj.isKnown = true; + } } + + const inviteList = this.state.inviteList.slice(); + inviteList.push(addrObj); + this.setState({ + inviteList: inviteList, + queryList: [], + }); + return inviteList; }, render: function() { - var TintableSvg = sdk.getComponent("elements.TintableSvg"); - var AddressSelector = sdk.getComponent("elements.AddressSelector"); + const TintableSvg = sdk.getComponent("elements.TintableSvg"); + const AddressSelector = sdk.getComponent("elements.AddressSelector"); this.scrollElement = null; var query = []; diff --git a/src/components/views/elements/AddressSelector.js b/src/components/views/elements/AddressSelector.js index 361705c6d5..fe6346b36a 100644 --- a/src/components/views/elements/AddressSelector.js +++ b/src/components/views/elements/AddressSelector.js @@ -16,18 +16,19 @@ limitations under the License. 'use strict'; -var React = require("react"); -var sdk = require("../../../index"); -var classNames = require('classnames'); +const React = require("react"); +const sdk = require("../../../index"); +const classNames = require('classnames'); +const InviteAddressType = require("./AddressTile"); -module.exports = React.createClass({ +export default React.createClass({ displayName: 'AddressSelector', propTypes: { onSelected: React.PropTypes.func.isRequired, - // List of strings: the addresses to display - addressList: React.PropTypes.array.isRequired, + // List of the addresses to display + addressList: React.PropTypes.arrayOf(InviteAddressType).isRequired, truncateAt: React.PropTypes.number.isRequired, selected: React.PropTypes.number, diff --git a/src/components/views/elements/AddressTile.js b/src/components/views/elements/AddressTile.js index 5f282afe93..01c1ed3255 100644 --- a/src/components/views/elements/AddressTile.js +++ b/src/components/views/elements/AddressTile.js @@ -23,16 +23,33 @@ var Invite = require("../../../Invite"); var MatrixClientPeg = require("../../../MatrixClientPeg"); var Avatar = require('../../../Avatar'); -module.exports = React.createClass({ +// React PropType definition for an object describing +// an address that can be invited to a room (which +// could be a third party identifier or a matrix ID) +// along with some additional information about the +// address / target. +export const InviteAddressType = React.PropTypes.shape({ + addressType: React.PropTypes.oneOf([ + 'mx', 'email' + ]).isRequired, + address: React.PropTypes.string.isRequired, + displayName: React.PropTypes.string, + avatarMxc: React.PropTypes.string, + // true if the address is known to be a valid address (eg. is a real + // user we've seen) or false otherwise (eg. is just an address the + // user has entered) + isKnown: React.PropTypes.bool, +}); + + +export default React.createClass({ displayName: 'AddressTile', propTypes: { - address: React.PropTypes.string.isRequired, + address: InviteAddressType.isRequired, canDismiss: React.PropTypes.bool, onDismissed: React.PropTypes.func, justified: React.PropTypes.bool, - networkName: React.PropTypes.string, - networkUrl: React.PropTypes.string, }, getDefaultProps: function() { @@ -40,35 +57,26 @@ module.exports = React.createClass({ canDismiss: false, onDismissed: function() {}, // NOP justified: false, - networkName: "", - networkUrl: "", }; }, render: function() { - var userId, name, imgUrl, email; - var BaseAvatar = sdk.getComponent('avatars.BaseAvatar'); - var TintableSvg = sdk.getComponent("elements.TintableSvg"); + const address = this.props.address; + const name = address.displayName || address.address; - // Check if the addr is a valid type - var addrType = Invite.getAddressType(this.props.address); - if (addrType === "mx") { - let user = MatrixClientPeg.get().getUser(this.props.address); - if (user) { - userId = user.userId; - name = user.rawDisplayName || userId; - imgUrl = Avatar.avatarUrlForUser(user, 25, 25, "crop"); - } else { - name=this.props.address; - imgUrl = "img/icon-mx-user.svg"; - } - } else if (addrType === "email") { - email = this.props.address; - name="email"; - imgUrl = "img/icon-email-user.svg"; + let imgUrl; + if (address.avatarMxc) { + imgUrl = MatrixClientPeg.get().mxcUrlToHttp( + address.avatarMxc, 25, 25, 'crop' + ); + } + + if (address.addressType === "mx") { + if (!imgUrl) imgUrl = 'img/icon-mx-user.svg'; + } else if (address.addressType === 'email') { + if (!imgUrl) imgUrl = 'img/icon-email-user.svg'; } else { - name="Unknown"; - imgUrl = "img/avatar-error.svg"; + if (!imgUrl) imgUrl = "img/avatar-error.svg"; } // Removing networks for now as they're not really supported @@ -83,15 +91,18 @@ module.exports = React.createClass({ } */ - var info; - var error = false; - if (addrType === "mx" && userId) { - var nameClasses = classNames({ + const BaseAvatar = sdk.getComponent('avatars.BaseAvatar'); + const TintableSvg = sdk.getComponent("elements.TintableSvg"); + + let info; + let error = false; + if (address.addressType === "mx" && address.isKnown) { + const nameClasses = classNames({ "mx_AddressTile_name": true, "mx_AddressTile_justified": this.props.justified, }); - var idClasses = classNames({ + const idClasses = classNames({ "mx_AddressTile_id": true, "mx_AddressTile_justified": this.props.justified, }); @@ -99,26 +110,26 @@ module.exports = React.createClass({ info = (
{ name }
-
{ userId }
+
{ address.address }
); - } else if (addrType === "mx") { - var unknownMxClasses = classNames({ + } else if (address.addressType === "mx") { + const unknownMxClasses = classNames({ "mx_AddressTile_unknownMx": true, "mx_AddressTile_justified": this.props.justified, }); info = ( -
{ this.props.address }
+
{ this.props.address.address }
); - } else if (email) { + } else if (address.addressType === "email") { var emailClasses = classNames({ "mx_AddressTile_email": true, "mx_AddressTile_justified": this.props.justified, }); info = ( -
{ email }
+
{ address.address }
); } else { error = true; @@ -132,12 +143,12 @@ module.exports = React.createClass({ ); } - var classes = classNames({ + const classes = classNames({ "mx_AddressTile": true, "mx_AddressTile_error": error, }); - var dismiss; + let dismiss; if (this.props.canDismiss) { dismiss = (
From 9020a7515cce880896d5933de0165d5c923d9553 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 25 Jan 2017 14:50:00 +0000 Subject: [PATCH 5/8] Correctly bail out on unknown address --- src/components/views/dialogs/ChatInviteDialog.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/components/views/dialogs/ChatInviteDialog.js b/src/components/views/dialogs/ChatInviteDialog.js index 60aa6b0c5b..babdf6d9ba 100644 --- a/src/components/views/dialogs/ChatInviteDialog.js +++ b/src/components/views/dialogs/ChatInviteDialog.js @@ -405,6 +405,8 @@ module.exports = React.createClass({ isKnown: false, }; if (addrType == null) { + this.setState({ error: true }); + return null; } else if (addrType == 'mx') { const user = MatrixClientPeg.get().getUser(addrObj.address); if (user) { From fd8d5af63a19c62abd08e3222d74ae69e38d5c61 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 25 Jan 2017 14:54:21 +0000 Subject: [PATCH 6/8] Fix import of InviteAddressType and rewrite to import while we're at it --- src/components/views/elements/AddressSelector.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/components/views/elements/AddressSelector.js b/src/components/views/elements/AddressSelector.js index fe6346b36a..9f37fa90ff 100644 --- a/src/components/views/elements/AddressSelector.js +++ b/src/components/views/elements/AddressSelector.js @@ -16,10 +16,10 @@ limitations under the License. 'use strict'; -const React = require("react"); -const sdk = require("../../../index"); -const classNames = require('classnames'); -const InviteAddressType = require("./AddressTile"); +import React from 'react'; +import sdk from '../../../index'; +import classNames from 'classnames'; +import { InviteAddressType } from './AddressTile'; export default React.createClass({ displayName: 'AddressSelector', From e9804086ca41c95dc31c1f139970a456e0d82fd9 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 25 Jan 2017 15:03:01 +0000 Subject: [PATCH 7/8] Point to InviteAddressType --- src/components/views/dialogs/ChatInviteDialog.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/components/views/dialogs/ChatInviteDialog.js b/src/components/views/dialogs/ChatInviteDialog.js index babdf6d9ba..c35f247ad7 100644 --- a/src/components/views/dialogs/ChatInviteDialog.js +++ b/src/components/views/dialogs/ChatInviteDialog.js @@ -164,6 +164,8 @@ module.exports = React.createClass({ queryList = this._userList.filter((user) => { return this._matches(query, user); }).map((user) => { + // Return objects, structure of which is defined + // by InviteAddressType return { addressType: 'mx', address: user.userId, From ced1c45a349a70567392baec0892081959f3f251 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 25 Jan 2017 15:23:50 +0000 Subject: [PATCH 8/8] Doc state in getinitialstate --- src/components/views/dialogs/ChatInviteDialog.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/components/views/dialogs/ChatInviteDialog.js b/src/components/views/dialogs/ChatInviteDialog.js index c35f247ad7..2326eda852 100644 --- a/src/components/views/dialogs/ChatInviteDialog.js +++ b/src/components/views/dialogs/ChatInviteDialog.js @@ -66,7 +66,14 @@ module.exports = React.createClass({ getInitialState: function() { return { error: false, + + // List of AddressTile.InviteAddressType objects represeting + // the list of addresses we're going to invite inviteList: [], + + // List of AddressTile.InviteAddressType objects represeting + // the set of autocompletion results for the current search + // query. queryList: [], }; },