GroupView: Add a User #1402
GroupView: Add a User #1402
Conversation
lukebarnard1
commented
Sep 20, 2017
•
lukebarnard1
commented
- Add "Add a User/Room" buttons and always display default lists
- Implement UserPickerDialog for adding users
- Add users to group summary using new API
- Implement avatar, displayname for featured users
- Disable "Add a Room" button for when we have a room picker
Also, use AccessibleButtons.
Profile data has been added to the API response for users in the group summary
| if (!success) return; | ||
| addrs.map((addr) => { | ||
| return MatrixClientPeg.get() | ||
| .addUserToGroupSummary(this.props.groupId, addr.address); |
There was a problem hiding this comment.
You're using map here and returning the promise, but then discarding the result - if you're not interested in the result, forEach may be better suited. That said, we should probably display an error if any of them failed, so we should probably not be ignoring the results (https://github.com/matrix-org/matrix-react-sdk/blob/develop/src/GroupInvite.js#L43 is where I did this fwiw)
| // Add avatar once we get profile info inline in the summary response | ||
| //const BaseAvatar = sdk.getComponent("avatars.BaseAvatar"); | ||
| const BaseAvatar = sdk.getComponent("avatars.BaseAvatar"); | ||
| const name = this.props.summaryInfo.displayname || this.props.summaryInfo.user_id.slice(1); |
There was a problem hiding this comment.
Ah. I hadn't noticed that _getInitialLetter ignores the "@".
| this.setState({ | ||
| busy: false, | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Shouldn't this be fetching the group user list once and then doing a client-side filter when the query changes rather than waiting until you type something and then displaying the whole group user list?
There was a problem hiding this comment.
I would argue that it's best to get the group list frequently and avoid early optimisation (so that the user doesn't have to close and open the dialog once someone has accepted the invite to a group, for example). But yes. I totally forgot to actually implement the filtering. 🤦♂️
