Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Better support for inviting multiple people #403

Merged
merged 11 commits into from
Aug 11, 2016
Merged

Better support for inviting multiple people #403

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
54b3638
Better support for inviting multiple people
dbkr Aug 10, 2016
4a6a9bd
Remove redundant setState
dbkr Aug 10, 2016
020e4e1
Factor out single-invite
dbkr Aug 10, 2016
b9f0b72
Missing proptypes
dbkr Aug 10, 2016
6b3b2e3
Don't reset completionStates
dbkr Aug 10, 2016
fa498eb
Comment typo
dbkr Aug 10, 2016
d6f7358
Make completionStates an object
dbkr Aug 10, 2016
cab95f4
Show cancel button whilst inviting is in progress
dbkr Aug 10, 2016
a58a627
Just use _canceled
dbkr Aug 11, 2016
806477d
s/then/done/
dbkr Aug 11, 2016
f42d619
Missing comma
dbkr Aug 11, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 27 additions & 8 deletions src/components/views/dialogs/MultiInviteDialog.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,12 @@ export default class MultiInviteDialog extends React.Component {

this._onCancel = this._onCancel.bind(this);
this._startInviting = this._startInviting.bind(this);
this._canceled = false;

this.state = {
busy: false,
completionStates: [], // State of each address (invited or error)
errorTexts: [], // Textual error per address
completionStates: {}, // State of each address (invited or error)
errorTexts: {}, // Textual error per address
done: false,
};
for (let i = 0; i < this.props.inputs.length; ++i) {
Expand All @@ -41,20 +42,28 @@ export default class MultiInviteDialog extends React.Component {
}
}

componentWillUnmount() {
this._unmounted = true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we also be setting _canceled here? What happens if the user closes the dialog without clicking 'cancel'? I'm a bit confused about the difference between _unmounted and _canceled, and why we handle them differently in _inviteMore. Perhaps some comments would help?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Point - it should definitely cancel if you dismiss the dialog so I've just made them the same variable.

}

_onCancel() {
this._canceled = true;
this.props.onFinished(false);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this attempt to cancel the invite process?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise the invite process could go on for hours in the background? And react will get very sad about the attempts to update the state.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would, although this button isn't visible once you start inviting - it probably should be though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't the close button visible?

On 10 August 2016 18:38:55 BST, David Baker notifications@github.com wrote:

  •        busy: false,

  •        completionStates: [], // State of each address (invited
    or error)
  •        errorTexts: [], // Textual error per address

  •        done: false,

  •    };

  •    for (let i = 0; i < this.props.inputs.length; ++i) {

  •        const input = this.props.inputs[i];

  •        if (getAddressType(input) === null) {

  •            this.state.completionStates[i] = 'error';

  •            this.state.errorTexts[i] = 'Unrecognised address';

  •        }

  •    }
  • }
  • _onCancel() {
  •    this.props.onFinished(false);

It would, although this button isn't visible once you start inviting -
it probably should be though.

You are receiving this because you were assigned.
Reply to this email directly or view it on GitHub:
https://github.com/matrix-org/matrix-react-sdk/pull/403/files/54b3638a85abbfff0b8fb119cfc17a6429b0a9b1#r74292572

}

_startInviting() {
this.setState({
completionStates: [],
busy: true,
done: false,
});
this._inviteMore(0);
}

_inviteMore(nextIndex) {
if (this._canceled) {
return;
}

if (nextIndex == this.props.inputs.length) {
this.setState({
busy: false,
Expand All @@ -67,7 +76,7 @@ export default class MultiInviteDialog extends React.Component {

// don't try to invite it if it's an invalid address
// (it will already be marked as an error though,
// so no need top do so again
// so no need to do so again)
if (getAddressType(input) === null) {
this._inviteMore(nextIndex + 1);
return;
Expand All @@ -81,12 +90,16 @@ export default class MultiInviteDialog extends React.Component {
}

inviteToRoom(this.props.roomId, input).then(() => {
if (this._unmounted) { return; }

this.setState((s) => {
s.completionStates[nextIndex] = 'invited'
return s;
});
this._inviteMore(nextIndex + 1);
}, (err) => {
if (this._unmounted) { return; }

let errorText;
let fatal = false;
if (err.errcode == 'M_FORBIDDEN') {
Expand Down Expand Up @@ -115,16 +128,19 @@ export default class MultiInviteDialog extends React.Component {
}

_getProgressIndicator() {
const numErrors = this.state.completionStates.filter((s) => {
return s == 'error';
}).length;
let numErrors = 0;
for (const k of Object.keys(this.state.completionStates)) {
if (this.state.completionStates[k] == 'error') {
++numErrors;
}
}
let errorText;
if (numErrors > 0) {
const plural = numErrors > 1 ? 's' : '';
errorText = <span className="error">({numErrors} error{plural})</span>
}
return <span>
{this.state.completionStates.length} / {this.props.inputs.length} {errorText}
{Object.keys(this.state.completionStates).length} / {this.props.inputs.length} {errorText}
</span>;
}

Expand Down Expand Up @@ -153,6 +169,7 @@ export default class MultiInviteDialog extends React.Component {
let controls = [];
if (this.state.busy) {
controls.push(<Spinner key="spinner" />);
controls.push(<button key="cancel" onClick={this._onCancel}>Cancel</button>);
controls.push(<span key="progr">{this._getProgressIndicator()}</span>);
} else if (this.state.done) {
controls.push(
Expand Down Expand Up @@ -196,4 +213,6 @@ export default class MultiInviteDialog extends React.Component {

MultiInviteDialog.propTypes = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing inputs, roomId.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Done)

onFinished: React.PropTypes.func.isRequired,
inputs: React.PropTypes.array.isRequired,
roomId: React.PropTypes.string.isRequired,
};
67 changes: 34 additions & 33 deletions src/components/views/rooms/MemberList.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,39 @@ module.exports = React.createClass({
});
},

_doInvite(address) {
Invite.inviteToRoom(self.props.roomId, address).catch((err) => {
if (err !== null) {
console.error("Failed to invite: %s", JSON.stringify(err));
if (err.errcode == 'M_FORBIDDEN') {
Modal.createDialog(ErrorDialog, {
title: "Unable to Invite",
description: "You do not have permission to invite people to this room."
});
} else {
Modal.createDialog(ErrorDialog, {
title: "Server error whilst inviting",
description: err.message
});
}
}
}).finally(() => {
self.setState({
inviting: false
});
// XXX: hacky focus on the invite box
setTimeout(function() {
var inviteBox = document.getElementById("mx_SearchableEntityList_query");
if (inviteBox) {
inviteBox.focus();
}
}, 0);
}).done();
self.setState({
inviting: true
});
}

onInvite: function(inputText) {
var ErrorDialog = sdk.getComponent("dialogs.ErrorDialog");
var NeedToRegisterDialog = sdk.getComponent("dialogs.NeedToRegisterDialog");
Expand Down Expand Up @@ -247,39 +280,7 @@ module.exports = React.createClass({
if (inputs.length == 1) {
// for a single address, we just send the invite
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about factoring this out to another method?

promise.then(() => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/then/done/

return Invite.inviteToRoom(self.props.roomId, inputs[0]);
}).catch((err) => {
if (err !== null) {
console.error("Failed to invite: %s", JSON.stringify(err));
if (err.errcode == 'M_FORBIDDEN') {
Modal.createDialog(ErrorDialog, {
title: "Unable to Invite",
description: "You do not have permission to invite people to this room."
});
} else {
Modal.createDialog(ErrorDialog, {
title: "Server error whilst inviting",
description: err.message
});
}
}
self.setState({
inviting: false
});
}).finally(() => {
self.setState({
inviting: false
});
// XXX: hacky focus on the invite box
setTimeout(function() {
var inviteBox = document.getElementById("mx_SearchableEntityList_query");
if (inviteBox) {
inviteBox.focus();
}
}, 0);
}).done();
self.setState({
inviting: true
this.doInvite(inputs[0]);
});
} else {
// if there are several, display the confirmation/progress dialog
Expand Down