Introduce component displaying group of users on the list in AddMembersModal - e.g. "Users in parent thread:", or "Not in parent thread"
Details
Details
Component can be tested after introducing "AddMembersModal"
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| web/modals/threads/members/add-members-group.react.js | ||
|---|---|---|
| 14 ↗ | (On Diff #11510) | We should use set to avoid O(n^2) complexity |
| 22 ↗ | (On Diff #11510) | Sorting here is the most expensive operation - maybe it makes sense to create a separate memo with just this? It will depend on just userInfos |
| web/modals/threads/members/members-modal.css | ||
| 58–60 ↗ | (On Diff #11510) | It might be a good idea, but can't we just use header with correct capitalization? |
Comment Actions
Looks ok, but would like to clarify header capitalization before accepting.
| web/modals/threads/members/add-members-group.react.js | ||
|---|---|---|
| 14 ↗ | (On Diff #11777) | |
| 18–19 ↗ | (On Diff #11777) | Is it possible to receive a header that doesn't need to be modified? (prop that is capitalized by parent component) |
| 40 ↗ | (On Diff #11777) | What do you think about template literal here? |
| web/modals/threads/members/add-members-group.react.js | ||
|---|---|---|
| 40 ↗ | (On Diff #11777) | By template literal do you mean something like: <div className={css.addMemberItemsGroupHeader}>{`${capitalizedHeader}:`}</div>? I'm not sure if it's more readable as we need more braces. |
| web/modals/threads/members/add-members-group.react.js | ||
|---|---|---|
| 40 ↗ | (On Diff #11777) | Ok, let's keep it |