UI for Wechat invites

[lucyhe]

Tested manually & ran grunt test

This requires freedomjs/freedom-social-wechat#11 to be checked in and published
Combined with the freedom pull request, fixes: #1573 and #1977

[jpevarnek]

Aah, I'm so sorry on the delay here! Few questions below, hope you don't mind.

---

Reviewed 4 of 8 files at r1, 10 of 10 files at r3.
Review status: all files reviewed at latest revision, 6 unresolved discussions, some commit checks failed.

---

src/generic_ui/polymer/network-invite-user.html, line 112 [r3] (raw file):
Remote TODO?

---

src/generic_ui/polymer/network-invite-user.ts, line 27 [r3] (raw file):
I think here it would look a bit better to use the .then(success, failure) syntax (since in the rare chance that logging a message fails (not actually possible...), you don't want that to trigger the failure message).

Also, second log seems to me as if it should be at least warn.

---

src/generic_ui/polymer/network-invite-user.ts, line 49 [r3] (raw file):
I'm a bit confused, why do we need to fetch these from the backend when we already have the list of users?

---

src/generic_ui/polymer/network-invite-user.ts, line 55 [r3] (raw file):
What is this return value for?

---

src/generic_ui/polymer/network-invite-user.ts, line 98 [r3] (raw file):
It seems to me from the other blocks that this should open a panel for the user to interact with, I don't see that happening, is that done elsewhere?

---

src/generic_ui/scripts/core_connector.ts, line 218 [r1] (raw file):
I'm pretty sure this comment is not relevant to the function you added.

---

Comments from the review on Reviewable.io

[masq]

Review status: 5 of 9 files reviewed at latest revision, 6 unresolved discussions, some commit checks failed.

---

src/generic_ui/polymer/network-invite-user.html, line 112 [r3] (raw file):
Assuming you meant, "remove", this is done.

---

src/generic_ui/polymer/network-invite-user.ts, line 27 [r3] (raw file):
done.

---

src/generic_ui/polymer/network-invite-user.ts, line 49 [r3] (raw file):
Well we're getting all of the wechat possible friends here; friends we would potentially want to invite, but haven't yet. uProxy knows of only our current friends.

---

src/generic_ui/polymer/network-invite-user.ts, line 55 [r3] (raw file):
Not sure, as such I removed it.

---

src/generic_ui/polymer/network-invite-user.ts, line 98 [r3] (raw file):
haha, that would explain why the UI wasn't showing up... Thanks for pointing this out! Fixed this.

---

src/generic_ui/scripts/core_connector.ts, line 218 [r1] (raw file):
removed this comment.

---

Comments from the review on Reviewable.io

[jpevarnek]

Sorry about the delay, I missed this last week.

---

Reviewed 1 of 2 files at r4, 5 of 5 files at r5.
Review status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

---

src/generic_ui/polymer/network-invite-user.html, line 73 [r5] (raw file):
Can you make the class descriptive of what element this is being applied to instead of what the CSS rule is? I've found in the past that makes it a bit less of a headache to change things around later on. Thanks!

It might also be good to move the inline margin-top: 20px; rule to its own actual class as well.

---

src/generic_ui/polymer/network-invite-user.ts, line 49 [r3] (raw file):
Ah, okay. Any chance of renaming the method to make it a bit more clear of what it is doing (i.e. grabbing non-friend non-profiles)? Thanks!

It would also be great if we could move from the any type to something explicit.

---

Comments from Reviewable

[trevj]

This has been open for a long time - is it still active, or can we close?

[masq]

The issue is still "active" in that it hasn't been resolved/fixed. IIRC, I was last looking into making sure that friends are populated in the invite list as they are loaded/resolved.

I've not been active on working on it as I have been focusing on other projects (work), but others are free to look into it. If I find some free time this summer, I can look into some of these issues though!

[trevj]

I'm happy to keep the branch around but I prefer to keep our list of pull requests short and up to date, this one will require some serious merging to get back up to date :-)