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

feat: add NFTs tab #1762

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

feat: add NFTs tab #1762

wants to merge 12 commits into from

Conversation

helciofranco
Copy link
Member

@helciofranco helciofranco commented Jan 9, 2025


📷 Demo
Screen.Recording.2025-01-09.at.17.08.04.mov

@helciofranco helciofranco self-assigned this Jan 9, 2025
@helciofranco helciofranco linked an issue Jan 9, 2025 that may be closed by this pull request
@helciofranco helciofranco marked this pull request as ready for review January 9, 2025 09:14
@helciofranco helciofranco enabled auto-merge (squash) January 9, 2025 09:15
Copy link
Contributor

@LuizAsFight LuizAsFight left a comment

Choose a reason for hiding this comment

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

can we start with all collections expanded? except by the "Others" , that one can be collapsed

Comment on lines +64 to +68
.sort((a, b) => {
if (a.name === UNKNOWN_COLLECTION_TITLE) return 1;
if (b.name === UNKNOWN_COLLECTION_TITLE) return -1;
return 0;
});
Copy link
Contributor

Choose a reason for hiding this comment

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

can we sort by name also?

  • A
  • B
  • Z
  • Others

Copy link
Member Author

Choose a reason for hiding this comment

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

Are you talking about collection names or NFT names?

If it's about NFT names, it's already sorting by name:
Screenshot 2025-01-11 at 20 18 46

If you're talking about collection names, I'd like to apply but I didn't because in the explorer it's also not sorting.

If you compare our listing with explorer right now, they're identical.

Thoughts?

Copy link
Contributor

@LuizAsFight LuizAsFight left a comment

Choose a reason for hiding this comment

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

  • in the black theme I can't see any separator between the collections
  • I think the collection name can be a little bit bigger (in your video it looks bigger than when I run locally), did you change it after recording?

@helciofranco
Copy link
Member Author

@LuizAsFight

  • in the black theme I can't see any separator between the collections
  • I think the collection name can be a little bit bigger (in your video it looks bigger than when I run locally), did you change it after recording?
    • Nope. The video you've seen was the version you reviewed.. I think it's because it has a natural "scale" in the preview. I'll increase a little bit (from $sm to $base). 76ba41b

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

Successfully merging this pull request may close these issues.

Create NFT tab for balances in Home Screen
2 participants