SES-4753 - Invite members #1
Conversation
| inviteDialogVisible = manageGroupMembersViewModel.uiState.collectAsState().value.isInviteMemberDialogVisible | ||
| ) | ||
|
|
||
| if (uiState.showUrlDialog) { |
There was a problem hiding this comment.
You can make this reusable by having showUrlDialog as a nullable string. If it is not null you show the dialog, and the url itself is the content of showUrlDialog. This way it can be used for more than one thing, and you don't need to have helpUrl in the state as it's not really a state element.
There was a problem hiding this comment.
Also there might be more dialogs in the future, so as rule we tend to encompass the content where it belongs. So maybe this dialog should exist inside InviteAccountIdScreen?
There was a problem hiding this comment.
Updated. I did move the dialog inside InviteAccountIdScreen since this screen has its own share of dialogs.
| } | ||
|
|
||
| @Composable | ||
| fun ShowInviteContactsDialog( |
There was a problem hiding this comment.
I don't think Composables should have the word 'Show' in their names since they are components.
Isn't this dialog the same as in the invite contact screen? Couldn't it be shared between them?
| onHelp = { newMessageViewModel.onCommand(NewMessageViewModel.Commands.ShowUrlDialog) }, | ||
| onSendInvite = {shareHistory -> | ||
| manageGroupMembersViewModel.onCommand( | ||
| ManageGroupMembersViewModel.Commands.SendInvites( |
There was a problem hiding this comment.
Why is SendInvites part of this viewmodel?
Shouldn't
val manageGroupMembersViewModel: ManageGroupMembersViewModel = hiltViewModel(parentEntry)
be removed here? And SendInvites be moved to the appropriate viewmodel?
Or is SendInvites used somewhere as well? I think the relationship between all those viewmodels need to be really clear and solid
There was a problem hiding this comment.
I did it similar to how InviteContactsScreen sends the invite since it is the same with only a difference in how to get the Address. InviteContactsScreen does not use onCommand though, maybe I can update this one to make it uniform, or should I change it completely and add the same functions into InviteMembersViewModel?
| ) { | ||
| InviteAccountId( | ||
| state = state, | ||
| inviteState = viewModel.uiState.collectAsState().value.inviteContactsDialog, |
There was a problem hiding this comment.
Hou shouldn't collectAsState in the declaration itself.
You should have something like:
val uiState by viewModel.uiState.collectAsState()
above, and then
inviteState = uiState.inviteContactsDialog,
to avoid unneeded calls to collectAsState
Part 3 of SES-4753
This PR updates how we can invite members to groups
NewMessagecomposable to handle invite mode.