Closed amenconi closed 4 months ago
Latest commit: |
8056d88
|
Status: | ✅ Deploy successful! |
Preview URL: | https://fb6287cd.app-cqo.pages.dev |
Branch Preview URL: | https://amenconi-nft-bulk-transfer.app-cqo.pages.dev |
Nice work!
I've just transffered you one of my creamies: https://amenconi-nft-bulk-transfer.app-cqo.pages.dev/nft/0x428a56244a46a1181a56c9e318b26d148d49ac1d
Few notes by quickly playing with it:
Nice work!
I've just transffered you one of my creamies: https://amenconi-nft-bulk-transfer.app-cqo.pages.dev/nft/0x428a56244a46a1181a56c9e318b26d148d49ac1d
Few notes by quickly playing with it:
- I think the card should not be clickable once one tickbox is selected
- "clear" button is invisible to me
- bottom toolbar could actually have both transfer/withdraw buttons
- not sure what the status is for and how it's relevant
- there should be select all option
- I would select all NFTs upon first opening
- once I complete transfer it ends up in inconsistent state. Maybe that's were "status" comes in? You intended to update it with success message per NFT?
Thanks!
Great point on making card not clickable once selected, definitely a no brainer and will add that ASAP.
Will also fix the font color styling for light theme on the "Clear" button.
For the "Withdraw" button on the toolbar, are you thinking that would bring up a separate modal for withdrawals? I know you can withdraw when transferring but will have to more into how the withdraw mechanism works. I think it just withdraws to the verified address for the user right? So maybe in this case clicking withdraw might not need any further interface and just perform a bulk withdrawal for any selected NFTs?
Status field is populated with the response for each NFT transfer attempt. I've included a video for how it works at least in debug.
Good idea for select all, when opening the transfer modal interface I'll pre-select all for transfer and have a select all toggle for transfer and withdraw checkboxes. I'd assumed that transfers could only happen one a single network at a time but haven't set up purchasing NFTs in debug for other networks yet, though as I think about it, if the transfer is internal then I suppose network shouldn't matter and I can remove the logic I put in to only allow selection of NFTs on a single network at a time.
Also when trying to test on this link: https://amenconi-nft-bulk-transfer.app-cqo.pages.dev/ I'm getting a lot of errors and weird behavior such as it taking a very long time to open selected NFT toolbar or the transfer interface, have you seen the same?
https://github.com/soonaverse/app/assets/7841580/20499e44-5d93-40cf-873e-7c9706763f85
In latest push I've changed:
Currently you can select to withdraw when transferring, would it make sense to add withdraw on the toolbar or add a button per NFT in the transfer modal that allows just withdrawing as a separate action to transfer/withdrawing?
Thanks.
@amenconi the footer is not visible when I'm on my Profile NFTs page. It is visible if I'm on a collection page and select my NFT there.
I would expect, as a final user, to see the footer on Profile > NFTs page.
Also, should selected NFTs be stored in local storage? Because if I refresh the page, footer is gone and selected NFT is no longer selected.
Same thing for multiple browser tabs opened.
@amenconi the footer is not visible when I'm on my Profile NFTs page. It is visible if I'm on a collection page and select my NFT there.
I would expect, as a final user, to see the footer on Profile > NFTs page.
Also, should selected NFTs be stored in local storage? Because if I refresh the page, footer is gone and selected NFT is no longer selected.
Same thing for multiple browser tabs opened.
Footer should definitely show no matter the page you are on, messaged you on Discord to discuss more in-depth.
My initial design was to only track selected NFTs in memory rather than persist it with local storage. My thoughts were that selecting NFTs and then doing something with that selection (such as transferring) would likely be an immediate process rather than something that would make sense to span browser sessions as opposed to the shopping cart where you might put a few items in the cart and then want to come back later and pick up your shopping process before pulling the trigger on a purchase. I may be thinking of this wrong or missing something though and definitely open to saving selected NFTs in local storage if you think that would be preferable.
Thanks.
I think you have a point regarding that the selection of NFTs for a bulk transfer would be a immediate process, so I think it makes sense not to span the process across multiple browser tabs. I'm not sure though about a process where someone is setting up the bulk process and accidentally reloads the page or goes elsewhere and then comes back to the process and find it was reset. Though it's not a big deal (only if the user had set up like hundreds of nfts). Either way, I think this can be left handled as is, let the users use it and give feedback, then adjust to a more complex feature if needed.
This latest push includes:
Hopefully this makes it more clear to user what happened so they can make changes and attempt transfer again if appropriate.
I'm also looking into sending a message to user that transferred and user that received NFT during each transfer. On the back end these are named "Notification" but are different than the nzNotifications from the ng-zorro-antd library and show up when the Bell icon is clicked on the header. Initially I see that the Notification being referenced from Build.5 interface library is deprecated so it may take a bit more research before implementing this. However I feel this may be a more nice to have than anything critical and I think we can release this feature without it if it's not added in a timely manner.
Thank you.
Thank you @amenconi for these last improvements!
A few things I found today:
Thank you!
Great feedback as always, thank you.
Thank you.
Ok this push should fix the following:
Will continue to do testing on Firefox and let you know once I can reproduce the issues and hopefully solve.
Thank you.
Pulled in latest develop branch state and resolved merge conflicts.
I also believe I've fixed the Firefox issue. Oddly persistent problem that took a few iterations to solve but this version was working well on Chrome and Firefox in debug at least.
Also did some light UI styling changes to better match the buttons on shopping cart modal, nft selection toolbar and nft transfer modal.
Thank you.
I confirm the bug is fixed on Firefox. Everything seems to be working properly. And UI looks better now with those adjustments.
I only have a few comments that would smooth user experience, but not really a blocker for this feature to go live:
Maybe it would be best to have the "specific member" selection set by default, instead of the wallet address.
When the transfer succeeded and modal is closed, the page does a full reload. If it simply removes the NFTs from view, instead of reloading the page, would be a better user experience.
If the recipient user had their profile NFT tab opened, and NFTs arrive, they won't notice until a manual page refresh is made or they navigate to another page and navigate back to their NFT profile tab.
@amenconi maybe we can move this feature live, given that it's functional and my comments were only ux improvements.
What do you guys think @adamunchained @SoonaverseTF ?
It seems that this version of the app is no longer working, when I try to connect to the staging URL I get the error: "Failed to initialize wallet, try to reload page.".
It seems that this version of the app is no longer working, when I try to connect to the staging URL I get the error: "Failed to initialize wallet, try to reload page.".
Test account has been shutdown as it has nearly no use. Did you try to point to prod environment instead?
Bulk NFT transfer feature contains 3 main parts:
1) Checkboxes on eligible NFT cards to allow selection of 1 or more NFTs. Checkbox is available if 2 conditions are met, the logged in user uid matches the nft owner property (you own the NFT) and the nft “locked” property is false. This should allow selection of these NFTs anywhere in Soonaverse that shows the NFT cards (user profile NFTs, Collection NFTs page etc).
2) NFT selection toolbar. If one or more NFTs are selected a toolbar component opens at the bottom of the screen displaying how many NFTs are selected with a button to clear all selected NFTs and a button to initiate transfer of selected NFTs.
3) NFT transfer modal. When Transfer button is clicked on the toolbar it opens the transfer modal which lists each selected NFT showing: NFT image. NFT name. NFT network. Checkbox to indicate NFT should be transferred. Checkbox to indicate transferred NFT should also be withdrawn. There is also a text input which will allow input of transfer destination (member or wallet address) and then a Close and Transfer button. Field to contain response from Build.5 API once NFT transfer is processed.
Checkbox to withdraw NFT when transferred will be disabled unless the transfer checkbox for that NFT is checked. Transfer button will be disabled until at least one transfer checkbox is checked and a destination is input. Once the first transfer checkbox is checked, all transfer checkboxes for NFTs on an alternate network will be disabled until all transfer checkboxes are unchecked (will remove this if transfers for multiple NFTs on multiple networks can be transferred in a single API call).
You can toggle the recipient selection from wallet address input to selecting Soonaverse member. Selecting Soonaverse member will let you search by Soonaverse account name.
Once transfer selections are made and a recipient name or address is input you can initiate the NFT transfer. Response/error codes returned for each NFT transfer are updated in the transfer modal NFT list. Multiple transfers can be done in the modal iteratively.
When on the profile NFTs page with the NFT transfer modal up, had some issues reactively updating the list of NFTs on profile page in cases where an transfer of one or more NFTs was successful and the transfer modal was closed. For now, in these cases the page will reload to reflect the currently owned NFT for the user. A bit of a hack and leaving the transferred NFTs on the profile NFT page shouldn't open any risks as the API restricts transfers to NFT owned by the logged in user, however it could be confusing. I'll look for better ways to update the list reactively but current PR should have code that works and wanted to open up the feature for core functionality testing.