Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ swap page landing #11163

Open
wants to merge 52 commits into
base: main
Choose a base branch
from
Open

Conversation

roiLeo
Copy link
Contributor

@roiLeo roiLeo commented Oct 23, 2024

Screenshot 2024-10-23 at 11-52-32 Swap NFTs One Stop Shop for Polkadot NFTs

@roiLeo roiLeo requested a review from a team as a code owner October 23, 2024 10:05
@roiLeo roiLeo requested review from vikiival and hassnian and removed request for a team October 23, 2024 10:05
Copy link

netlify bot commented Oct 23, 2024

Deploy Preview for koda-canary ready!

Name Link
🔨 Latest commit 55a6c88
🔍 Latest deploy log https://app.netlify.com/sites/koda-canary/deploys/674d21568717180008194a37
😎 Deploy Preview https://deploy-preview-11163--koda-canary.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

@hassnian hassnian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice!

@roiLeo I see that the pr does not close the issue, do you want to finish it ? otherwise I can take over.

can you

  • use translation keys for all words
  • limit this page to only assethub inside redirect.global using getPermissionRouteCondition -> permission.config.ts
  • if you are not logged this should be a button (I guess like the transfer page)
    CleanShot 2024-10-24 at 08 57 12@2x

CleanShot 2024-10-24 at 08 57 57@2x

@roiLeo roiLeo marked this pull request as draft October 24, 2024 06:27
@roiLeo
Copy link
Contributor Author

roiLeo commented Oct 24, 2024

I see that the pr does not close the issue, do you want to finish it ? otherwise I can take over.

Yea it's WIP, feel free to take it if you need to ship it faster

use translation keys for all words

yes, that would be ideal

limit this page to only assethub inside redirect.global using getPermissionRouteCondition -> permission.config.ts
if you are not logged this should be a button (I guess like the transfer page)

noted! ty

@roiLeo
Copy link
Contributor Author

roiLeo commented Oct 24, 2024

@hassnian any way to split the work? so we don't erase each other stuff
like someone handle the logic, any other step or maybe you want full control?

@hassnian
Copy link
Contributor

@hassnian any way to split the work? so we don't erase each other stuff like someone handle the logic, any other step or maybe you want full control?

yeah it's gonna be hard since its a lot of new code, I'll let you finish it. I can work on something else in the meantime

@vikiival
Copy link
Member

Well how swap works:

  • you take your NFT
  • you select nft you want
  • call same call as offer :P

So my recommendation is that one can start with item detail:

  • create button, or add swap for another in the more select

The Swap page (cc @exezbcz):
On one side is your nft
on the other is either desired nft or considered collection

It means if desired on the indexer is null you want nft from particular collection

Future
You can additionally add surcharge which https://github.com/kodadot/stick/blob/39ac052e60cc0b9c03ca1d3e23cb6c3f869e5822/schema.graphql#L244

You can either send or receive

@vikiival
Copy link
Member

However I like also your idea to invite a "conter-party" so you can easily look for "his/hers" nfts

@hassnian hassnian marked this pull request as ready for review November 14, 2024 10:10
@hassnian
Copy link
Contributor

@roiLeo a08d281 I think we should not use bulma anymore , new code should only use tailwind since we have

wdyt?

@roiLeo
Copy link
Contributor Author

roiLeo commented Nov 14, 2024

@roiLeo a08d281 I think we should not use bulma anymore , new code should only use tailwind since we have

* [Bulma conflicting with Tailwind classes  #9973](https://github.com/kodadot/nft-gallery/issues/9973)

* [remove bulma css #9013](https://github.com/kodadot/nft-gallery/issues/9013)

wdyt?

sure i'll replace is-hidden-touch with tailwind equivalent, same goes for colums with grid/flex layout.
I'm not too worried about Bulma, I'm more concerned with oruga UI.

@vikiival
Copy link
Member

https://github.com/paritytech/polkadot-sdk/blob/aff3a0796176ff3c0ee1b89c2f1d811a858f17a8/substrate/frame/nfts/src/features/atomic_swap.rs#L49

Yes

offered_collection_id: T::CollectionId,
offered_item_id: T::ItemId,
desired_collection_id: T::CollectionId,
maybe_desired_item_id: Option<T::ItemId>,

as I have mentioned in #11163 (comment)

You need to provided NFT you want to swap (offered_collection_id, offered_item_id)
and the thing you want to get:

  • any NFT from particular collection (desired_collection_id, null)
  • that exact NFT from that collection (desired_collection_id, maybe_desired_item_id)

and yes It can be anything

e.g I have nft (ahp: 263-4231008173)
(that is from collection 263)

and I want this NFT (ahp: 152-2406426862) which is from collecton (152)

Hope it makes sense now

components/items/ItemsGrid/ItemsGridImage.vue Outdated Show resolved Hide resolved
components/swap/landing.vue Outdated Show resolved Hide resolved
Copy link

sonarcloud bot commented Dec 2, 2024

@hassnian hassnian requested review from preschian and removed request for hassnian December 2, 2024 09:27
@hassnian
Copy link
Contributor

hassnian commented Dec 2, 2024

I'm removing myslef and asking @preschian for a review since there's a lot of my code on this pr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create Swap Page
4 participants