Display the list of owned safes #2495
Conversation
|
CLA Assistant Lite All Contributors have signed the CLA. |
ESLint Summary View Full Report
[warning] @typescript-eslint/explicit-module-boundary-types
Report generated by eslint-plus-action |
|
This is not just an issue when testing, but also for users. There is already a similar ticket addressing this problem, even with some designs: #510 So would be more in favor of coming up with a solution that would also work as a v1 that we can also use in production. Even if it's not the "proper solution" as planned in #510. Maybe also worth looking into parcel.money or multisafe.finance, as they offer to add Safes where the connect wallet is an owner by "scanning for them". |
|
@lukasschor thanks Lucas!
I could make the unsaved saves a collapsible section and add an "Add safe" button on hover.
|
katspaugh
changed the title
katspaugh
force-pushed
the
owned-safes
branch
2 times, most recently
from
8ef7b5b
to
77f03a9
Compare
| import { getTxServiceUrl } from 'src/config' | ||
|
|
||
| export const fetchSafesByOwner = async (ownerAddress: string): Promise<string[]> => { | ||
| const url = `${getTxServiceUrl()}/owners/${ownerAddress}` |
There was a problem hiding this comment.
This endpoint structure is deprecated. It should have /safes after owner address
Check swagger for more info
https://safe-transaction.mainnet.gnosis.io/
|
Really nice! Would love to have this available as a feature for users if the overhead is small enough. Some minor comments
|
|
Fixed, thanks @lukasschor!
|
|
This feature is truly awesome The only thing: it looks like laggy in scenarios with a lot of safes: https://www.loom.com/share/927df3bf33c94e73a847b9a3949f6e7f Maybe virtualizing the list would help |
@mikheevm I've checked react-virtualized docs and it seems it needs to know the height in advance. And we have a flexible height there. The whole container scrolls. I assume regular users won't have more than a dozen safes. Good UX pattern though, will note for future. 👍 |
katspaugh
moved this from Code review and dev test
to Ready for QA
in Gnosis Safe React (Archived)
|
@katspaugh Looks like there's a component for dynamic sizing: https://github.com/bvaughn/react-virtualized/blob/master/docs/AutoSizer.md Anyway, this is a rather extreme case, so it's not required to fix it in this PR. |
francovenica
moved this from Ready for QA
to QA in progress
in Gnosis Safe React (Archived)
|
I like the feature and is functional. A few improvements we can make:
Still, give what this feature was created for, I'd not put much more effort into it. |
Fixed, good catch @francovenica. |
What it solves
When testing the safe interface, oftentimes you clear the localStorage and lose all your safes.
Related to #510.
How this PR fixes it
Your safes can be still retrieved via an endpoint, so I've made a little custom hook that fetches safes by their owner.
How to test it
Connect your wallet and see your unsaved safes in the sidebar. Should not be visible on production.
Screenshots