feat(desktop@communities): change kicked/banned member behavior #13706

mprakhov · 2024-02-23T13:58:31Z

What does the PR do

when a user is banned or kicked, the community should disappear from the community nav bar and the next community down should be selected (unless there are no other communities in the nav bar in which case the Find community page is selected).
if a user is banned from the community, they should not be able to see or search for the community in the Find Community page
if the user navigates to the community while they are banned (e.g. via a share link), they will see a banned state for that community

Closes: #13454

status-go PR: status-im/status-go#4806

Affected areas

Ban/unban
Curated communities list

Screenshot of functionality (including design for comparison)

Screen.Recording.2024-02-23.at.14.36.33.mov

status-im-auto · 2024-02-23T14:05:47Z

Jenkins Builds

Click to see older builds (6)

❔	Commit	#️⃣	Finished (UTC)	Duration	Platform	Result
✔️	`17f14bf`	#2	2024-02-23 14:05:47	~5 min	`macos/aarch64`	🍎`dmg`
✔️	`17f14bf`	#2	2024-02-23 14:06:10	~6 min	`tests/nim`	📄`log`
✔️	`17f14bf`	#2	2024-02-23 14:10:35	~10 min	`tests/ui`	📄`log`
✔️	`17f14bf`	#2	2024-02-23 14:11:26	~11 min	`macos/x86_64`	🍎`dmg`
✔️	`17f14bf`	#2	2024-02-23 14:15:54	~15 min	`linux/x86_64`	📦`tgz`
✔️	`17f14bf`	#2	2024-02-23 14:24:04	~23 min	`windows/x86_64`	💿`exe`

❔	Commit	#️⃣	Finished (UTC)	Duration	Platform	Result
✔️	`34b9645`	#5	2024-02-26 13:49:20	~4 min	`macos/aarch64`	🍎`dmg`
✔️	`34b9645`	#5	2024-02-26 13:50:06	~5 min	`tests/nim`	📄`log`
✔️	`34b9645`	#5	2024-02-26 13:52:53	~7 min	`macos/x86_64`	🍎`dmg`
✔️	`34b9645`	#5	2024-02-26 13:53:58	~9 min	`tests/ui`	📄`log`
✔️	`34b9645`	#5	2024-02-26 13:59:55	~15 min	`linux/x86_64`	📦`tgz`
✔️	`34b9645`	#5	2024-02-26 14:06:49	~21 min	`windows/x86_64`	💿`exe`

✔️	`1ec7df4`	#6	2024-02-27 09:41:45	~4 min	`macos/aarch64`	🍎`dmg`
✔️	`1ec7df4`	#6	2024-02-27 09:43:29	~6 min	`tests/nim`	📄`log`
✔️	`1ec7df4`	#6	2024-02-27 09:44:50	~7 min	`macos/x86_64`	🍎`dmg`
✔️	`1ec7df4`	#6	2024-02-27 09:46:25	~9 min	`tests/ui`	📄`log`
✔️	`1ec7df4`	#6	2024-02-27 09:54:17	~16 min	`linux/x86_64`	📦`tgz`
✔️	`1ec7df4`	#6	2024-02-27 09:58:07	~20 min	`windows/x86_64`	💿`exe`

caybro

Just some minor UI remarks 👮‍♂️ :)

ui/app/AppLayouts/Chat/ChatLayout.qml

ui/app/AppLayouts/Communities/CommunitiesPortalLayout.qml

ui/app/AppLayouts/Communities/panels/CommunityBannedMemberCenterPanel.qml

ui/StatusQ/src/StatusQ/Controls/StatusNavBarTabButton.qml

MishkaRogachev

LGTM

MishkaRogachev · 2024-02-26T15:29:48Z

src/app/modules/main/communities/models/curated_community_item.nim

@@ -44,6 +46,7 @@ proc initCuratedCommunityItem*(
  result.permissionModel = newTokenPermissionsModel()
  if tokenPermissionsItems.len > 0:


just curious if the worth of checking permissions for banned user

better to leave it... in case of any regression this is another "gate" not to show the user community content. Also impact is unknown

noeliaSD

I didn't test the PR, only inspected the qml code.

I've left some comments, hope they are useful!

noeliaSD · 2024-02-26T15:24:47Z

ui/StatusQ/src/StatusQ/Controls/StatusNavBarTabButton.qml


 StatusIconTabButton {
    id: statusNavBarTabButton 
    property alias badge: statusBadge
    property alias tooltip: statusTooltip
    property Component popupMenu
+    property bool isMemberBanned: false


I'm thinking that StatusNavBarTabButton is kind of generic component and I don't think we should add "logical / specific" behaviour in here.

WDYT if instead of adding this isMemberBanned property in here you add something more generic?

Rename the property with a generic name (topIconVisible or stateIconVisible).

Expose the asset property for this icon so the consumer can decide different icons.

Expose background and border colors to be more flexible.

ui/app/AppLayouts/Chat/ChatLayout.qml

ui/imports/Themes/Theme.qml

ui/app/mainui/AppMain.qml

noeliaSD · 2024-02-26T15:51:44Z

ui/app/AppLayouts/Communities/views/BannedMemberCommunityView.qml

Could you reuse the JoinCommunityView instead by adding a state or something?
You can use storybook to see your changes here:

JoinCommunityView contains a huge amount of non-needed fields (components) that are not used in my component, so it is 2 different components

I don't see them as different component since there's a lot of repeated code in here and also in ui/app/AppLayouts/Communities/panels/CommunityBannedMemberCenterPanel.qml. The only difference from what we have now and the banned design is just the code starting from line 98 in CommunityBannedMemberCenterPanel.qml. I'll create a task for the UI team to refactor that but we all together as a team should try to avoid this kind of code duplications.

cc: @alexandraB99

Created task: #13742

noeliaSD · 2024-02-26T15:56:32Z

ui/app/AppLayouts/Communities/panels/CommunityBannedMemberCenterPanel.qml

Same comment than here: ui/app/AppLayouts/Communities/views/BannedMemberCommunityView.qml

I think we can reuse the existing components and add a new state to control the desired behaviour.

IMHO, if we want to create one component, that will be configurable via states or any other parameters - I can create a separate ticket for the UI team because such a change will be a breaking change and regression must be tested. Also I don't have UI team vision how it must looks like

ui/app/AppLayouts/Communities/panels/CommunityBannedMemberCenterPanel.qml

caybro

LGTM

mprakhov requested review from jrainville, a team and kounkou and removed request for a team February 23, 2024 13:58

mprakhov force-pushed the mp/issue-13454 branch from bc8aa50 to 17f14bf Compare February 23, 2024 13:59

mprakhov requested review from noeliaSD and MishkaRogachev February 23, 2024 14:00

mprakhov added the waiting-for-status-go-merge label Feb 23, 2024

mprakhov mentioned this pull request Feb 23, 2024

feat: kicked/banned member should not have spectated mode after the kick/ban status-im/status-go#4806

Merged

mprakhov changed the title ~~feat: change kicked/banned member behavior~~ feat(desktop@communities): change kicked/banned member behavior Feb 23, 2024

caybro reviewed Feb 23, 2024

View reviewed changes

mprakhov force-pushed the mp/issue-13454 branch 2 times, most recently from 09d377d to 5d4dbd1 Compare February 26, 2024 13:42

mprakhov removed the waiting-for-status-go-merge label Feb 26, 2024

feat: change kicked/banned member behavior

34b9645

mprakhov force-pushed the mp/issue-13454 branch from 5d4dbd1 to 34b9645 Compare February 26, 2024 13:44

MishkaRogachev approved these changes Feb 26, 2024

View reviewed changes

noeliaSD reviewed Feb 26, 2024

View reviewed changes

caybro reviewed Feb 26, 2024

View reviewed changes

ui/app/AppLayouts/Communities/panels/CommunityBannedMemberCenterPanel.qml Outdated Show resolved Hide resolved

chore: changes after code review

1ec7df4

caybro approved these changes Feb 27, 2024

View reviewed changes

mprakhov merged commit 996199b into master Feb 27, 2024
8 checks passed

mprakhov deleted the mp/issue-13454 branch February 27, 2024 11:55

noeliaSD mentioned this pull request Feb 27, 2024

Modify JoinCommunityView in order to accept different central panel components #13742

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(desktop@communities): change kicked/banned member behavior #13706

feat(desktop@communities): change kicked/banned member behavior #13706

mprakhov commented Feb 23, 2024 •

edited

status-im-auto commented Feb 23, 2024 •

edited

caybro left a comment

MishkaRogachev left a comment

MishkaRogachev Feb 26, 2024

mprakhov Feb 27, 2024 •

edited

noeliaSD left a comment •

edited

noeliaSD Feb 26, 2024

mprakhov Feb 27, 2024

noeliaSD Feb 26, 2024

mprakhov Feb 27, 2024 •

edited

noeliaSD Feb 27, 2024

noeliaSD Feb 27, 2024

noeliaSD Feb 26, 2024

mprakhov Feb 27, 2024

caybro left a comment

		@@ -44,6 +46,7 @@ proc initCuratedCommunityItem*(
		result.permissionModel = newTokenPermissionsModel()
		if tokenPermissionsItems.len > 0:

feat(desktop@communities): change kicked/banned member behavior #13706

feat(desktop@communities): change kicked/banned member behavior #13706

Conversation

mprakhov commented Feb 23, 2024 • edited

What does the PR do

Affected areas

Screenshot of functionality (including design for comparison)

status-im-auto commented Feb 23, 2024 • edited

Jenkins Builds

caybro left a comment

Choose a reason for hiding this comment

MishkaRogachev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mprakhov Feb 27, 2024 • edited

Choose a reason for hiding this comment

noeliaSD left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mprakhov Feb 27, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

caybro left a comment

Choose a reason for hiding this comment

mprakhov commented Feb 23, 2024 •

edited

status-im-auto commented Feb 23, 2024 •

edited

mprakhov Feb 27, 2024 •

edited

noeliaSD left a comment •

edited

mprakhov Feb 27, 2024 •

edited