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.

Sign up for GitHub

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

Jump to bottom

revert back to unstacked nft cards #7482

Merged
merged 2 commits into from
Oct 5, 2023
Merged

revert back to unstacked nft cards #7482

merged 2 commits into from
Oct 5, 2023

Conversation

daiagi
Copy link
Contributor

@daiagi daiagi commented Oct 3, 2023 •
edited by ghost
Loading

Thank you for your contribution to the KodaDot - One Stop Shop for Polkadot NFTs.

👇 __ Let's make a quick check before the contribution.

PR Type

  • Bugfix

Needs QA check

  • @kodadot/qa-guild please review

Context

Screenshot 📸

  • My fix has changed UI

Copilot Summary

🤖 Generated by Copilot at 2676612

This pull request removes the concept of stacks from the NFT gallery UI and backend. It simplifies the data structure and query for fetching NFTs with metadata, and updates the components that display and interact with NFTs accordingly. It also deletes or disables some unused or redundant code related to stacks.

🤖 Generated by Copilot at 2676612

The stacks of NFTs are no more
They were removed from the useNft store
The NeoNftCard and ItemsGridImage too
Were updated to use the new data view
And useItemsGrid got a refactor and more

@daiagi daiagi requested a review from a team as a code owner October 3, 2023 06:45
@daiagi daiagi requested review from preschian and floyd-li and removed request for a team October 3, 2023 06:45
@netlify
Copy link

netlify bot commented Oct 3, 2023 •
edited
Loading

✅ Deploy Preview for koda-canary ready!

Name Link
🔨 Latest commit 4492810
🔍 Latest deploy log https://app.netlify.com/sites/koda-canary/deploys/651bba2f7185d700089da979
😎 Deploy Preview https://deploy-preview-7482--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.

@reviewpad
Copy link
Contributor

reviewpad bot commented Oct 3, 2023

AI-Generated Summary: This pull request contains several changes mainly related to reverting back to unstacked NFT cards. Changes can be observed within several Vue and TypeScript files.

In ItemsGridImage.vue, modifications were made to simplify buying and listing logic by removing the distinction between stacked and single NFTs. The variable nft: ItemsGridEntity was changed to nft: NFTWithMetadata, indicating the decision to work directly with NFT metadata instead of using an intermediary entity.

In useItemsGrid.ts, some related changes took place as well: the ref nfts now holds an array of NFTWithMetadata rather than ItemsGridEntity. Some unnecessary computations and functions were removed, simplifying the code.

In useNft.ts, the Stack type was removed as it became redundant.

In NeoNftCard.vue, the component was adjusted to account for the removal of stacked NFTs from the display logic.

Lastly, the file nftListWithSearch.graphql was deleted because its functionality became obsolete after previous modifications.

Overall, the contributor primarily focused on efforts to simplify the code, making it easier to maintain and comprehend by eliminating unnecessary abstractions and complex logic related to stacked NFTs.

@reviewpad reviewpad bot added medium Pull request is medium waiting-for-review labels Oct 3, 2023
@reviewpad
Copy link
Contributor

reviewpad bot commented Oct 3, 2023

Reviewpad Report

⚠️ Warnings

  • Please link an issue to the pull request

@daiagi daiagi requested a review from roiLeo October 3, 2023 06:46
roiLeo
roiLeo approved these changes Oct 3, 2023
View reviewed changes
components/items/ItemsGrid/useItemsGrid.ts Outdated Show resolved Hide resolved
@daiagi @roiLeo
Update components/items/ItemsGrid/useItemsGrid.ts
4492810 
Co-authored-by: roiLeo <medina.leo42@gmail.com>
@codeclimate
Copy link

codeclimate bot commented Oct 3, 2023

Code Climate has analyzed commit 4492810 and detected 0 issues on this pull request.

View more on Code Climate.

@sonarcloud
Copy link

sonarcloud bot commented Oct 3, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

preschian
preschian approved these changes Oct 3, 2023
View reviewed changes
Copy link
Member

@preschian preschian left a comment

Choose a reason for hiding this comment

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

I hope we can see this stacks again~

@daiagi
Copy link
Contributor Author

daiagi commented Oct 3, 2023 •
edited
Loading

I hope we can see this stacks again~

we will 😉

@vikiival
Copy link
Member

vikiival commented Oct 3, 2023

Does not make sense imo.
Squids run on full power anyway.

Screenshot 2023-10-03 at 10 45 50

@daiagi
Copy link
Contributor Author

daiagi commented Oct 3, 2023

@vikiival

thought you said yes to revert.
np. close it

@yangwao yangwao mentioned this pull request Oct 3, 2023
Closed
@vikiival
Copy link
Member

vikiival commented Oct 3, 2023

thought you said yes to revert. np. close it

Yup, but loooks like @yangwao is in favour of revert

@prury prury added the S-works-for-me-✅ qa-guild has tested PR from end user perspective and functionality worked label Oct 3, 2023
@exezbcz exezbcz mentioned this pull request Oct 4, 2023
Closed
@yangwao
Copy link
Member

yangwao commented Oct 5, 2023

Yup, but loooks like @yangwao is in favour of revert

as I got it, it currently slows down squids?

@yangwao yangwao merged commit 74f08a9 into main Oct 5, 2023
14 checks passed
@yangwao yangwao deleted the revert-stacked-cards branch October 5, 2023 10:31
@daiagi daiagi mentioned this pull request Oct 6, 2023
Closed
This was referenced Oct 11, 2023
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vikiival vikiival vikiival left review comments

@preschian preschian preschian approved these changes

@roiLeo roiLeo roiLeo approved these changes

@floyd-li floyd-li Awaiting requested review from floyd-li floyd-li is a code owner automatically assigned from kodadot/code-review-guild

Assignees
No one assigned
Labels
medium Pull request is medium S-works-for-me-✅ qa-guild has tested PR from end user perspective and functionality worked waiting-for-review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@daiagi @vikiival @yangwao @preschian @roiLeo @prury