Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Look up email addresses in ChatInviteDialog #653

Merged
merged 4 commits into from
Jan 26, 2017
Merged

Conversation

dbkr
Copy link
Member

@dbkr dbkr commented Jan 25, 2017

So email addresses known to the IS get a display name & avatar

Requires element-hq/element-web#3064

So email addresses known to the IS get a display name & avatar
dbkr added a commit to element-hq/element-web that referenced this pull request Jan 25, 2017
@dbkr dbkr requested a review from richvdh January 25, 2017 18:53
this._lookupThreepid(addrType, query).then((res) => {
if (res !== null) {
this.setState({
queryList: [res]
Copy link
Member

Choose a reason for hiding this comment

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

why not do this in _lookupThreepid?

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

@@ -435,6 +445,39 @@ module.exports = React.createClass({
return inviteList;
},

_lookupThreepid(medium, address) {
Copy link
Member

Choose a reason for hiding this comment

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

missing function

Copy link
Member Author

Choose a reason for hiding this comment

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

oops, done

// wait a bit to let the user finish typing
return q.delay(500).then(() => {
// If the query has changed, forget it
if (this.state.queryList[0] && this.state.queryList[0].address !== address) {
Copy link
Member

Choose a reason for hiding this comment

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

I sort-of wonder if it would be better to set a flag when we are changing the addresslist, rather than do this the whole time: basically rolling your own promise cancellation.

something like:

_lookupThreepid: function (medium, address) {
    let cancelled = false;
    this.cancelThreepidLookup = function() {
        cancelled = true;
    }
    return q.delay(500).then(() => {
        if (cancelled) { return null }
        ...
    }) /* ... */ .finally(() => { this.cancelThreepidLookup = null })
}

then call cancelThreepidLookup in onQueryChanged

Copy link
Member

Choose a reason for hiding this comment

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

or, y'know, just use bluebird.

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, but note you can't remove cancelThreepidLookup from this in the finally because you don't know that it's the same one you added.

@dbkr dbkr assigned richvdh and unassigned dbkr Jan 26, 2017
@dbkr
Copy link
Member Author

dbkr commented Jan 26, 2017

ptal

Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

lgtm

@richvdh richvdh assigned dbkr and unassigned richvdh Jan 26, 2017
@dbkr dbkr merged commit e2edecc into develop Jan 26, 2017
Half-Shot pushed a commit to Half-Shot/riot-web that referenced this pull request Feb 9, 2017
@richvdh richvdh deleted the dbkr/invite_look_up_emails branch February 15, 2017 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants