agileurbanite / ui.multisafe

5 stars 8 forks source link

Feat: Loading Existing Multisafe UX #125

Open agileurbanite opened 2 years ago

agileurbanite commented 2 years ago

Loading existing multisafe is not obvious UX that you need to enter the full account name of a multisafe to use said multisafe....

Almost a BUG imo. 2022-06-24 14 11 05

agileurbanite commented 2 years ago

If the user has visited multisafe before, when disconnecting account we should just persist that in local storage so that when the user re-authenticates the safe can be added back in. It is strange though to get visibility on a safe that you have no permissions over.

marcinbodnar commented 2 years ago

If the user has visited multisafe before, when disconnecting account we should just persist that in local storage so that when the user re-authenticates the safe can be added back in.

@agileurbanite That's a great idea.

There is one issue. At this moment Multisafe is supporting only one account at a time, so it's storing (in local storage) state for one account only.

There might be a situation:

  1. User is connected (account number 1.), has a few safes added and he Disconnects - we are keeping the state so he will be able to connect again and don't need to add safes.
  2. User is connecting to a different account (account number 2.) - in this case, we need to clear the state, because the stored state belongs to a different account.
  3. User is disconnecting and connecting again to account number 1. - again we need to clear the state and the user would have to add safes again.

Of course, we can leave it this way, but I think it would be better to handle it and make it work properly. We can solve it by:

  1. Adding additional reducer and store only basic data (accountId and safeId) for all accounts there
  2. Or refactor redux state to store all data for all accounts, but it wouldn't be much of an upgrade, because the project is not using redux-persist properly and all data is still loaded every time you enter Multisafe.

An additional advantage would be that by introducing this functionality we will be able to introduce functionality to support multiple accounts much easier if we would decide to do that - it will be basically ready.

Additional thing is that if we would like to keep the state in local storage, I think we should do two additional things:

marcinbodnar commented 2 years ago

It is strange though to get visibility on a safe that you have no permissions over.

@agileurbanite So if we would like to support showing a safe data to user that isn't a member, we can handle it much better. IMO we should separate it from loading safe flow, and the existing safes list. So the user will not be loading safe that he's not a member, but he will be able to see safe data. For example on the landing page we can have a button Show safe data, and the user can enter safeId (without a name) and see all (or partial) data.

With this, it would be also possible to share the link to the safe, without the need to load the safe. For example, if someone would like to share the safe data with someone who is not a member.

Also, we need to handle situations, when a user was a member of the safe, so he has this safe on the safes list, but he was removed from the safe members.