Introduce callbacks in menu in ThreadMember component - the same that are used in mobile app.
Details
Details
The actions can be tested after introducing memebrs modal
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Branch
- jacek/members-actions-callbacks
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
Comment Actions
Accepting, but adding @atul as a blocking reviewer just to clarify language questions
| web/modals/threads/members/member.react.js | ||
|---|---|---|
| 71 | It might be a good idea to get some input from a native speaker (maybe @atul). I usually prefer using toggle to indicate that we're alternating between two states and switch if there are more possible states. Is that interpretation correct and we should prefer toggle, or is switch also clear in this case? I think that onSwitchAdmin could mean that we have an admin and now we want to make some other user an admin. | |
| 136 | Another language question: is onRemoveUser misleading in this context? For me onRemoveUser means that somethings happens when we remove a user. But in this case, this happens on click, so either we should have onClick={removeUser} or onClick={onClickRemoveUser}. Does that make sense? | |
Comment Actions
Resigning since I responded to the language question but didn't do a full review
| web/modals/threads/members/member.react.js | ||
|---|---|---|
| 71 | I agree that toggle would make more sense than switch since
Maybe something like onMemberAdminRoleToggled? That might be too long? Can't think of anything else at the moment | |