This component will be used on friend and block add lists.
Details
Details
- Reviewers
• jacek atul • benschac - Commits
- rCOMMee84b8052c24: [web] Introduce add user item
Render the component on a modal and check if it looks correctly and reacts to user click action
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| web/settings/relationship/add-users-list-item.react.js | ||
|---|---|---|
| 16 ↗ | (On Diff #13505) | Maybe something like onAddUserButtonClicked/onAddUserClicked/etc? Probably doesn't make a difference now since the component is pretty simple, but maybe in the future there are multiple interactive "clickable" elements? |
| 23 ↗ | (On Diff #13505) | Not sure whether we want to capitalize "add" here? I think we capitalize user actions in other places (eg Add Friend, Block User, etc) so maybe we should keep that consistent? As an aside, this looks like a place where we could benefit from some design work? Maybe instead of "add" we could include an icon like the following:  ? None of these block the diff, just something we might want to consider. |
| web/settings/relationship/add-users-list-item.react.js | ||
|---|---|---|
| 23 ↗ | (On Diff #13505) | (this was the icon I intended to attach:  ) |
| web/settings/relationship/add-users-list-item.react.js | ||
|---|---|---|
| 16 ↗ | (On Diff #13505) | I don't think it is a good idea to complicate this - it is unlikely that this row would have a couple of clickable elements, and even if we decided to add one, we could then rename the callback. On the other hand, it would be beneficial to have less abstract name, describing what this action does instead of saying when it is used, so addUser might be a good idea. |
| 23 ↗ | (On Diff #13505) | Yes, we should use capital A here. Regarding using icons, I'm not really sure - I created a task https://linear.app/comm/issue/ENG-1278/consider-using-icons-instead-of-text-in-member-user-add-modals where we can decide if this is a good idea. |