Details
- Reviewers
• jacek atul - Commits
- rCOMM806b52e852ee: [web] Add cancel and confirm buttons
Render the component and check if the buttons look ok.
Confirm button should be disabled while there are no users selected.
Cancel and confirm button should call appropriate functions.
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| web/settings/relationship/add-users-list.react.js | ||
|---|---|---|
| 189–196 ↗ | (On Diff #13625) | Haven't tested the error scenario - it will be handled later |
| web/settings/relationship/add-users-list.react.js | ||
|---|---|---|
| 189–196 ↗ | (On Diff #13625) | Thanks for noting that here! |
| 193 ↗ | (On Diff #13625) | Wonder if Array.from(...) would make it clearer that we're converting pendingUserIDs: Set<> to userIDs: Array<> here? This might be a common pattern, but I personally haven't really seen spreads used to convert between types? |
| 204 ↗ | (On Diff #13625) | Wonder if we should include a LoadingIndicator to visually indicate the status? Probably in a separate diff sequenced later in the stack. Something like what @yayabosh did here: https://github.com/CommE2E/comm/commit/98cd7aeceb61fd354701125d70fb896ee506c080 |
| web/settings/relationship/add-users-list.css | ||
|---|---|---|
| 52–53 ↗ | (On Diff #13765) | We want the button to keep its width - that's why we need to still render the text. The difference between visibility: hidden and display: none is that with visibility the element still takes space. |