From f105ec279451cf8866fc1a202a546458fb5cde50 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 18 Jan 2017 17:51:39 +0000 Subject: [PATCH 1/6] Attempt to sanitize ChatInviteDialog a bit * Use binds rather than onFoo functions which aren't actually handler functions themselves but return them * Rename onKeyUp to moveSelectionDown etc,, reserving onKeyUp for "a key has been released" rather than, "the up arrow key has been pressed" --- .../views/dialogs/ChatInviteDialog.js | 6 ++--- .../views/elements/AddressSelector.js | 24 +++++++------------ 2 files changed, 12 insertions(+), 18 deletions(-) diff --git a/src/components/views/dialogs/ChatInviteDialog.js b/src/components/views/dialogs/ChatInviteDialog.js index e9a041357f..9ca3ff635d 100644 --- a/src/components/views/dialogs/ChatInviteDialog.js +++ b/src/components/views/dialogs/ChatInviteDialog.js @@ -119,15 +119,15 @@ module.exports = React.createClass({ } else if (e.keyCode === 38) { // up arrow e.stopPropagation(); e.preventDefault(); - this.addressSelector.onKeyUp(); + this.addressSelector.moveSelectionUp(); } else if (e.keyCode === 40) { // down arrow e.stopPropagation(); e.preventDefault(); - this.addressSelector.onKeyDown(); + this.addressSelector.moveSelectionDown(); } else if (this.state.queryList.length > 0 && (e.keyCode === 188, e.keyCode === 13 || e.keyCode === 9)) { // comma or enter or tab e.stopPropagation(); e.preventDefault(); - this.addressSelector.onKeySelect(); + this.addressSelector.chooseSelection(); } else if (this.refs.textinput.value.length === 0 && this.state.inviteList.length && e.keyCode === 8) { // backspace e.stopPropagation(); e.preventDefault(); diff --git a/src/components/views/elements/AddressSelector.js b/src/components/views/elements/AddressSelector.js index 2c2d7e2d61..c477b8e7eb 100644 --- a/src/components/views/elements/AddressSelector.js +++ b/src/components/views/elements/AddressSelector.js @@ -55,7 +55,7 @@ module.exports = React.createClass({ } }, - onKeyUp: function() { + moveSelectionUp: function() { if (this.state.selected > 0) { this.setState({ selected: this.state.selected - 1, @@ -64,7 +64,7 @@ module.exports = React.createClass({ } }, - onKeyDown: function() { + moveSelectionDown: function() { if (this.state.selected < this._maxSelected(this.props.addressList)) { this.setState({ selected: this.state.selected + 1, @@ -73,25 +73,19 @@ module.exports = React.createClass({ } }, - onKeySelect: function() { + chooseSelection: function() { this.selectAddress(this.state.selected); }, onClick: function(index) { - var self = this; - return function() { - self.selectAddress(index); - }; + this.selectAddress(index); }, onMouseEnter: function(index) { - var self = this; - return function() { - self.setState({ - selected: index, - hover: true, - }); - }; + this.setState({ + selected: index, + hover: true, + }); }, onMouseLeave: function() { @@ -124,7 +118,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 7b7728c93abec435c44ae66f218fa23ee23e35ef Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 18 Jan 2017 18:32:38 +0000 Subject: [PATCH 2/6] Make behaviour of ChatInviteDialog more consistent * Pressing enter now always adds whatever was in the input box to the invite list, if it's a valid address (previously it added it to the list of it was a search result but submitted the form straight away if there were no results). * Remove isValidAddress as it was only used in the context of testing whether its return value was true or null (where null meant "unsure") so just use getAddressType instead. --- src/Invite.js | 22 ----------------- .../views/dialogs/ChatInviteDialog.js | 24 +++++++++++++++---- 2 files changed, 19 insertions(+), 27 deletions(-) diff --git a/src/Invite.js b/src/Invite.js index 6422812734..6bfc646977 100644 --- a/src/Invite.js +++ b/src/Invite.js @@ -59,25 +59,3 @@ export function inviteMultipleToRoom(roomId, addrs) { return this.inviter.invite(addrs); } -/** - * Checks is the supplied address is valid - * - * @param {addr} The mx userId or email address to check - * @returns true, false, or null for unsure - */ -export function isValidAddress(addr) { - // Check if the addr is a valid type - var addrType = this.getAddressType(addr); - if (addrType === "mx") { - let user = MatrixClientPeg.get().getUser(addr); - if (user) { - return true; - } else { - return null; - } - } else if (addrType === "email") { - return true; - } else { - return false; - } -} diff --git a/src/components/views/dialogs/ChatInviteDialog.js b/src/components/views/dialogs/ChatInviteDialog.js index 9ca3ff635d..767620d93f 100644 --- a/src/components/views/dialogs/ChatInviteDialog.js +++ b/src/components/views/dialogs/ChatInviteDialog.js @@ -74,8 +74,8 @@ module.exports = React.createClass({ var inviteList = this.state.inviteList.slice(); // Check the text input field to see if user has an unconverted address // If there is and it's valid add it to the local inviteList - var check = Invite.isValidAddress(this.refs.textinput.value); - if (check === true || check === null) { + const addrType = Invite.getAddressType(this.refs.textinput.value); + if (addrType !== null) { inviteList.push(this.refs.textinput.value); } else if (this.refs.textinput.value.length > 0) { this.setState({ error: true }); @@ -135,12 +135,26 @@ module.exports = React.createClass({ } else if (e.keyCode === 13) { // enter e.stopPropagation(); e.preventDefault(); - this.onButtonClick(); + if (this.state.queryList.length > 0) { + this.addressSelector.chooseSelection(); + } else { + 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: [], + }); + } else { + this.setState({ error: true }); + } + } } else if (e.keyCode === 188 || e.keyCode === 9) { // comma or tab e.stopPropagation(); e.preventDefault(); - var check = Invite.isValidAddress(this.refs.textinput.value); - if (check === true || check === null) { + const addrType = Invite.getAddressType(this.refs.textinput.value); + if (addrType !== null) { var inviteList = this.state.inviteList.slice(); inviteList.push(this.refs.textinput.value.trim()); this.setState({ From 2a08abaa955a9e60a06584ef7203e07743d70928 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 19 Jan 2017 16:35:40 +0000 Subject: [PATCH 3/6] Keep old behaviour of submitting on enter if input is empty --- src/components/views/dialogs/ChatInviteDialog.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/components/views/dialogs/ChatInviteDialog.js b/src/components/views/dialogs/ChatInviteDialog.js index 767620d93f..c117944482 100644 --- a/src/components/views/dialogs/ChatInviteDialog.js +++ b/src/components/views/dialogs/ChatInviteDialog.js @@ -135,7 +135,10 @@ module.exports = React.createClass({ } else if (e.keyCode === 13) { // enter e.stopPropagation(); e.preventDefault(); - if (this.state.queryList.length > 0) { + if (this.refs.textinput.value == '') { + // if there's nothing in the input box, submit the form + this.onButtonClick(); + } else if (this.state.queryList.length > 0) { this.addressSelector.chooseSelection(); } else { const addrType = Invite.getAddressType(this.refs.textinput.value); From ee1f6c772e164d24e0896c88be200f75d8e371aa Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 19 Jan 2017 16:50:09 +0000 Subject: [PATCH 4/6] Remove duplicate case handled above And fix typo where it was handled --- src/components/views/dialogs/ChatInviteDialog.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/components/views/dialogs/ChatInviteDialog.js b/src/components/views/dialogs/ChatInviteDialog.js index c117944482..5cbb0d4503 100644 --- a/src/components/views/dialogs/ChatInviteDialog.js +++ b/src/components/views/dialogs/ChatInviteDialog.js @@ -124,7 +124,7 @@ module.exports = React.createClass({ e.stopPropagation(); e.preventDefault(); this.addressSelector.moveSelectionDown(); - } else if (this.state.queryList.length > 0 && (e.keyCode === 188, e.keyCode === 13 || e.keyCode === 9)) { // comma or enter or tab + } else if (this.state.queryList.length > 0 && (e.keyCode === 188 || e.keyCode === 13 || e.keyCode === 9)) { // comma or enter or tab e.stopPropagation(); e.preventDefault(); this.addressSelector.chooseSelection(); @@ -138,8 +138,6 @@ module.exports = React.createClass({ if (this.refs.textinput.value == '') { // if there's nothing in the input box, submit the form this.onButtonClick(); - } else if (this.state.queryList.length > 0) { - this.addressSelector.chooseSelection(); } else { const addrType = Invite.getAddressType(this.refs.textinput.value); if (addrType !== null) { From a2ff1cd8e6578d237720fb0ac77885677c4e4775 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 19 Jan 2017 17:03:16 +0000 Subject: [PATCH 5/6] Factor out adding the input field to the list --- .../views/dialogs/ChatInviteDialog.js | 51 ++++++++----------- 1 file changed, 22 insertions(+), 29 deletions(-) diff --git a/src/components/views/dialogs/ChatInviteDialog.js b/src/components/views/dialogs/ChatInviteDialog.js index 5cbb0d4503..9928030d7f 100644 --- a/src/components/views/dialogs/ChatInviteDialog.js +++ b/src/components/views/dialogs/ChatInviteDialog.js @@ -71,15 +71,12 @@ module.exports = React.createClass({ }, onButtonClick: function() { - var inviteList = this.state.inviteList.slice(); + let inviteList = this.state.inviteList.slice(); // Check the text input field to see if user has an unconverted address // If there is and it's valid add it to the local inviteList - const addrType = Invite.getAddressType(this.refs.textinput.value); - if (addrType !== null) { - inviteList.push(this.refs.textinput.value); - } else if (this.refs.textinput.value.length > 0) { - this.setState({ error: true }); - return; + if (this.refs.textinput.value !== '') { + inviteList = this._addInputToList(); + if (inviteList === false) return; } if (inviteList.length > 0) { @@ -139,32 +136,12 @@ module.exports = React.createClass({ // if there's nothing in the input box, submit the form this.onButtonClick(); } else { - 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: [], - }); - } else { - this.setState({ error: true }); - } + this._addInputToList(); } } else if (e.keyCode === 188 || e.keyCode === 9) { // comma or tab e.stopPropagation(); e.preventDefault(); - const addrType = Invite.getAddressType(this.refs.textinput.value); - if (addrType !== null) { - var inviteList = this.state.inviteList.slice(); - inviteList.push(this.refs.textinput.value.trim()); - this.setState({ - inviteList: inviteList, - queryList: [], - }); - } else { - this.setState({ error: true }); - } + this._addInputToList(); } }, @@ -376,6 +353,22 @@ module.exports = React.createClass({ return addrs; }, + _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 false; + } + }, + render: function() { var TintableSvg = sdk.getComponent("elements.TintableSvg"); var AddressSelector = sdk.getComponent("elements.AddressSelector"); From afa384c4f39e466c22dc6b0552ad9552a80c28bd Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 19 Jan 2017 18:13:27 +0000 Subject: [PATCH 6/6] Use null instead of false --- src/components/views/dialogs/ChatInviteDialog.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/views/dialogs/ChatInviteDialog.js b/src/components/views/dialogs/ChatInviteDialog.js index 9928030d7f..18f4ce36ba 100644 --- a/src/components/views/dialogs/ChatInviteDialog.js +++ b/src/components/views/dialogs/ChatInviteDialog.js @@ -76,7 +76,7 @@ module.exports = React.createClass({ // If there is and it's valid add it to the local inviteList if (this.refs.textinput.value !== '') { inviteList = this._addInputToList(); - if (inviteList === false) return; + if (inviteList === null) return; } if (inviteList.length > 0) { @@ -365,7 +365,7 @@ module.exports = React.createClass({ return inviteList; } else { this.setState({ error: true }); - return false; + return null; } },