Introduce a set to which we add users after clicking their button. Filter out the selected users so that they can be added only once.
Depends on D4307
Details
Details
- Reviewers
• jacek atul - Commits
- rCOMM071e53347dbc: [web] Add option to select pending users
Render the component and add some users - every user should disappear from the list after being added.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| web/settings/relationship/add-users-list.react.js | ||
|---|---|---|
| 108 ↗ | (On Diff #13622) | This will cause the insertion to take O(nlogn) time - which shouldn't be an issue because the number of users that would be added is small. The best possible complexity here is O(logn) if we use a tree, but that would require modifying it - which won't work with the state. We could store the tree outside of the state and somehow force a render, but this is too complicated. If we want to store pending users in state, we can use e.g. insertion from insertion sort, and that would be O(n), but it is significantly more complicated than the current solution and the difference between O(n) and O(nlogn) isn't significant for us (only a handful of elements). |
| web/settings/relationship/add-users-list.react.js | ||
|---|---|---|
| 96–100 ↗ | (On Diff #13622) | Maybe something like this to reduce level of indentation? It's less verbose, but not sure that it's easier to read? if (pendingUsers.some(({id}) => id === userID) {
return pendingUsers;
} |
| 108 ↗ | (On Diff #13622) | Insertion from insertion sort makes sense since we can (probably) assume that the previous pendingUsers is in sorted order. But like you said, it probably doesn't make much of a difference in practice... so maybe not worth it if it's too involved? |
| web/settings/relationship/add-users-list.react.js | ||
|---|---|---|
| 108 ↗ | (On Diff #13622) | I tried using sortedIndexBy from lodash and it almost works. The problem is that in lodash we specify a prop by which items are sorted - in this case a username. Lodash then compare the props and decide the index based on it. This approach is inconsistent with our sorting procedure where we use localeCompare. Using lodash would result in differences between ordering in tags and list. |
| web/settings/relationship/add-users-list.react.js | ||
|---|---|---|
| 108 ↗ | (On Diff #13622) | Decided to simply iterate through array to find a target position |