CirclesUBI / circles-groups-safe-app

[DEPRECATED] Mint Circles group tokens using this Safe app
https://groups.circlesubi.dev/
1 stars 1 forks source link

[Refactor] Manage Group Member search, add, remove members behavior #196

Closed LaimeJesus closed 1 year ago

LaimeJesus commented 1 year ago

related #190

Description

This PR does a full refactor to the manageGroupMembers component with the following changes:

This was necessary to fix many issues related to the group members sync and hard to understand code:

How to test

As this was a full refactor, we should test all of the cases that are related to this component (Manage Group page), principally the following cases:

Search

As a group owner:

In both cases, the user can switch between groups using the selector and the filter should still apply

Remove Member

As a group owner:

Add Member

As a group owner:

Some Examples

Remove a group member

pre-remove-group-member-1 after-remove-group-member-1

Add an user

pre-add-group-member-1

after-add-group-member-1

vercel[bot] commented 1 year ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
circles-groups-safe-app ✅ Ready (Inspect) Visit Preview Aug 16, 2022 at 2:19PM (UTC)
LaimeJesus commented 1 year ago

Wow, how much work.

Adding users and searching members work really well. I've added a member, the list updated nicely. But when I removed a user the list didn't update. Gnosis.Safe.-.Google.Chrome.2022-08-12.15-38-09.mp4

One suggestion :

* In group information the Member list doesn't have the search input. What do you think about adding it?

Ok, will investigate that issue, seems to be a weird behavior but It shouldn't happen so Ill re-test everything trying to fix it

LaimeJesus commented 1 year ago

Wow, how much work.

Adding users and searching members work really well. I've added a member, the list updated nicely. But when I removed a user the list didn't update. Gnosis.Safe.-.Google.Chrome.2022-08-12.15-38-09.mp4

One suggestion :

* In group information the Member list doesn't have the search input. What do you think about adding it?

I found the issue, I was using the users array instead of the members array to filter the removed member :facepalm: , so there were some cases where the user was not found so the remove callback was not being called. Now it is using the members array to find the removing user, and seems to be working

https://user-images.githubusercontent.com/13955827/184436641-198c1428-18b1-4d1f-840d-41e4dd72ffcb.mp4

LaimeJesus commented 1 year ago

new changes:

Found some issues with no result texts, already fixed (when there are no users to add, when there are no members in a group in the members modal and remove member components)

Also, found an issue in staging related to the search logic when you open the members modal using the link:

https://user-images.githubusercontent.com/13955827/184903330-06c026dc-e43e-45b2-bdd6-55096622ca19.mp4

I've fixed that issue in the PR not allowing the user to search (we might want to add that but let do that in a different PR)

https://user-images.githubusercontent.com/13955827/184903479-8e6e98c1-5e98-4ecd-b8ce-e5fbe5811ab8.mp4

@tloffler @marcelorubini