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

🔧 bsx gallery sort & buy now #3442

Merged
merged 11 commits into from
Jul 18, 2022

Conversation

roiLeo
Copy link
Contributor

@roiLeo roiLeo commented Jul 15, 2022

⚠️ wait for #3441 before merging this

PR type

  • Bugfix

What's new?

Before submitting Pull Request, please make sure:

  • My contribution builds clean without any errors or warnings
  • I've merged recent default branch -- main and I've no conflicts
  • I've tried to respect high code quality standards
  • I've didn't break any original functionality
  • I've posted a screenshot of demonstrated change in this PR

Had issue bounty label?

  • Fill up your KSM address: Payout

Screenshot

Capture d’écran 2022-07-18 à 1 57 37 PM

@roiLeo roiLeo requested review from a team as code owners July 15, 2022 08:56
@roiLeo roiLeo requested review from prachi00 and yangwao and removed request for a team July 15, 2022 08:56
@netlify
Copy link

netlify bot commented Jul 15, 2022

Deploy Preview for koda-nuxt ready!

Name Link
🔨 Latest commit 73c4ccb
🔍 Latest deploy log https://app.netlify.com/sites/koda-nuxt/deploys/62d553271bf3470008990eb1
😎 Deploy Preview https://deploy-preview-3442--koda-nuxt.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 settings.

@yangwao yangwao requested a review from petersopko July 18, 2022 07:39
@petersopko
Copy link
Contributor

couple things I've noticed:

  • buy now toggle seems to be turned off by default, as opposed to RMRK, i.e. we're showing unlisted nft as default
  • when buy now toggle is off (showing unlisted) and I sort NFTs by price high to low, I can still see listed NFTs in the search?
  • range slider and setting priceMin + priceMax seems to work, but it need adjustment for bsx
  • replace range slider with input fields #3461

Copy link
Contributor

@petersopko petersopko left a comment

Choose a reason for hiding this comment

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

left comment

@petersopko petersopko added the S-changes-requested-🤞 PR is almost good to go, just some fine tunning label Jul 18, 2022
@roiLeo roiLeo marked this pull request as draft July 18, 2022 11:41
@roiLeo
Copy link
Contributor Author

roiLeo commented Jul 18, 2022

buy now toggle seems to be turned off by default, as opposed to RMRK, i.e. we're showing unlisted nft as default

Done on purpose, no need to hide unlisted item by default (same stuff on all chain).
Buy now toggle off should show all items.
Filters design might be changed after #2947

when buy now toggle is off (showing unlisted) and I sort NFTs by price high to low, I can still see listed NFTs in the search?

I got really weird sorting on bsx
Capture d’écran 2022-07-18 à 1 47 35 PM

I may have break something

edit:
nvm it's fixed
Capture d’écran 2022-07-18 à 1 57 37 PM

@roiLeo roiLeo marked this pull request as ready for review July 18, 2022 11:58
@petersopko
Copy link
Contributor

looks like rmrk/explore/gallery gets wrong query now?
image

{
   "errors":[
      {
         "message":"Variable \"$orderBy\" got invalid value null at \"orderBy[0]\"; Expected non-nullable type \"NftEntitiesOrderBy!\" not to be null.",
         "extensions":{
            "code":"BAD_USER_INPUT",
            "exception":{
               "stacktrace":[
                  "GraphQLError: Variable \"$orderBy\" got invalid value null at \"orderBy[0]\"; Expected non-nullable type \"NftEntitiesOrderBy!\" not to be null.",
                  "    at /usr/local/lib/node_modules/@subql/query/node_modules/graphql/execution/values.js:116:15",
                  "    at coerceInputValueImpl (/usr/local/lib/node_modules/@subql/query/node_modules/graphql/utilities/coerceInputValue.js:57:5)",
                  "    at /usr/local/lib/node_modules/@subql/query/node_modules/graphql/utilities/coerceInputValue.js:70:14",
                  "    at Array.map (<anonymous>)",
                  "    at safeArrayFrom (/usr/local/lib/node_modules/@subql/query/node_modules/graphql/jsutils/safeArrayFrom.js:36:23)",
                  "    at coerceInputValueImpl (/usr/local/lib/node_modules/@subql/query/node_modules/graphql/utilities/coerceInputValue.js:68:50)",
                  "    at coerceInputValue (/usr/local/lib/node_modules/@subql/query/node_modules/graphql/utilities/coerceInputValue.js:37:10)",
                  "    at _loop (/usr/local/lib/node_modules/@subql/query/node_modules/graphql/execution/values.js:109:69)",
                  "    at coerceVariableValues (/usr/local/lib/node_modules/@subql/query/node_modules/graphql/execution/values.js:121:16)",
                  "    at getVariableValues (/usr/local/lib/node_modules/@subql/query/node_modules/graphql/execution/values.js:50:19)"
               ]
            }
         }
      }
   ]
}

@roiLeo
Copy link
Contributor Author

roiLeo commented Jul 18, 2022

@petersopko should be good now

@petersopko petersopko added S-works-for-me-✅ qa-guild has tested PR from end user perspective and functionality worked and removed S-changes-requested-🤞 PR is almost good to go, just some fine tunning labels Jul 18, 2022
@petersopko petersopko merged commit de4fea4 into kodadot:main Jul 18, 2022
@petersopko
Copy link
Contributor

pay 100 usd

@yangwao
Copy link
Member

yangwao commented Jul 18, 2022

😍 Perfect, I’ve sent the payout
💵 $100 @ 60.37 USD/KSM ~ 1.656 $KSM
🧗 DVYy1qnocE8t6ZvUfPx3rEjG829khNRXx3YrCGVHHj19Lcb
🔗 0xb2d781bb33779dbe7feef064897caa8be09f15b6d65a55e7036244348d682596

🪅 Let’s grab another issue and get rewarded!
🪄 github.com/kodadot/nft-gallery/issues

@yangwao yangwao added the paid pull-request has been paid label Jul 18, 2022
@petersopko petersopko mentioned this pull request Jul 18, 2022
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
paid pull-request has been paid S-works-for-me-✅ qa-guild has tested PR from end user perspective and functionality worked
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sort in gallery broken
3 participants