-
Notifications
You must be signed in to change notification settings - Fork 78
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add remove user groups option to group chat context menu #16360
base: refactor/16286_refactor_profile_context_menu
Are you sure you want to change the base?
Add remove user groups option to group chat context menu #16360
Conversation
refactor ProfileContextMenu to make it a functional component This refactor ProfileContextMenu to make it a functional component by: refactored out direct calls to backend, and passing backend data structures and moved this logic to the callers, also refactored common calls between the callers common types of context menus have been extracted to their sub components which removes a lot of logic too and makes the behaviour very clear user verification workflow (which was already disabled) has been removed refactor: use signals and call singletons on the parent instead remove unused code for now from profile context menu refactor profile context menu into two components; add property to storybook extract blocked profile context menu and self profile context menu use profileType instead of individual bools refactor to pass trustStatus as an argument make contact type a parameter remove unnecessary method from RegularProfileContextMenu add ensVerified property to ProfileContextMenu components add onlineStatus property to ProfileContextMenu components move ProfileContextMenu storybook controls to the right sidebar move contactDetails logic up from the view add local nickname property to ProfileContextMenu components fix issue with missing signal; fix logs in storybook use constant for profileType instead of string refactor common code into a single method refactor getProfileContext remove references to contactDetails which are not longer needed remove unnecessary comments fix bridged constant refactor into a single ProfileContextMenu component refactor into a single ProfileContextMenu component refactor into a single ProfileContextMenu component simplify imports remove unused store field move methods from utils to contacts store remove onClosed signal remove unused param
Jenkins BuildsClick to see older builds (7)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just a suggestion.
Also, the issue has designs for the community options, could you open a new issue for those since the og issue will be closed with this PR. Thanks.
@@ -212,6 +214,10 @@ Item { | |||
const contactDetails = publicKey === "" ? {} : Utils.getContactDetailsAsJson(publicKey, true, true) | |||
Global.blockContactRequested(publicKey, contactDetails) | |||
} | |||
onRemoveFromGroup: (publicKey) => { | |||
const chatId = root.store.chatCommunitySectionModule.activeItem.id | |||
root.store.chatCommunitySectionModule.removeMemberFromGroupChat("", chatId, publicKey) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a suggestion, but we could create a removeMemberFromActiveGroupChat(publicKey)
function in the store that would do the two calls above and hide some of the logic. No need to use chatCommunitySectionModule
in the component that way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's also important rule (affects testing) that context properties should be accessed only from stores and not exposed from them directly. So chatCommunitySectionModule
should not be used (and even accessible) from root.store
directly like root.store.chatCommunitySectionModule
.
@@ -1214,6 +1216,10 @@ Loader { | |||
const contactDetails = publicKey === "" ? {} : Utils.getContactDetailsAsJson(publicKey, true, true) | |||
Global.blockContactRequested(publicKey, contactDetails) | |||
} | |||
onRemoveFromGroup: (publicKey) => { | |||
const chatId = root.rootStore.chatCommunitySectionModule.activeItem.id | |||
root.rootStore.chatCommunitySectionModule.removeMemberFromGroupChat("", chatId, publicKey) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above
93f17b2
to
8e0b619
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I think you just need to rebase on the other branch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to resolve the conflicts (rebase)
closes #16130
requires #16334
What does the PR do
Adds "Remove from group" to context menu in group chats
Affected areas
Should only affect group chats
Screenshot of functionality (including design for comparison)
How to test
OR
expected: user is removed from the group and the message "user has left the group" is displayed for everyone
Risk
Worst case scenario this option appears somewhere it shouldn't (like community member list) and clicking it has unknown behaviour (probably not happens)