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

fix: collectionId in path of url #6454

Merged
merged 4 commits into from
Jul 24, 2023
Merged

Conversation

Jarsen136
Copy link
Contributor

@Jarsen136 Jarsen136 commented Jul 21, 2023

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
  • Feature
  • Refactoring

Needs QA check

  • @kodadot/qa-guild please review

Context

Did your issue had any of the "$" label on it?

Screenshot 📸

  • My fix has changed UI

Copilot Summary

🤖 Generated by Copilot at 1f57248

This pull request refactors the collection search feature to use a pinia store and a composable function. This simplifies the code and makes the collection search mode accessible to other components. It also adds a feature to disable the collection search mode on mobile devices.

🤖 Generated by Copilot at 1f57248

Search by collection
Refactored with pinia
No more on mobile

@Jarsen136 Jarsen136 requested a review from a team as a code owner July 21, 2023 16:45
@Jarsen136 Jarsen136 requested review from roiLeo and vikiival and removed request for a team July 21, 2023 16:45
@kodabot
Copy link
Collaborator

kodabot commented Jul 21, 2023

SUCCESS @Jarsen136 PR for issue #6278 which is assigned to you. Please wait for review and don't hesitate to grab another issue in the meantime!

@netlify
Copy link

netlify bot commented Jul 21, 2023

Deploy Preview for koda-canary ready!

Name Link
🔨 Latest commit 59cfbe3
🔍 Latest deploy log https://app.netlify.com/sites/koda-canary/deploys/64be3d7f3468cb00080b0c70
😎 Deploy Preview https://deploy-preview-6454--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 Jul 21, 2023

AI-Generated Summary: This pull request addresses the issue of how the collectionId is handled in the path of URLs. Four files have been modified and one new file (stores/search.ts) has been created. Changes include removing unnecessary checks from useSearchParams.ts and modifying the condition for isCollectionSearchMode in SearchBar.vue. The new search store, defined in stores/search.ts, now handles the state for collection search. With these updates, the collection search process is now controlled by the searchStore and adjusted by the actual device width, which ensures the proper display on both desktop and mobile devices.

@reviewpad reviewpad bot added small Pull request is small waiting-for-review labels Jul 21, 2023
@prury
Copy link
Member

prury commented Jul 21, 2023

Ways of triggering the error:
go to a collection page and switch between activity and items
go to a nft and click on the link that leads to its collection

the redundant collection id does not show up on the address bar anymore, but the warning still appears:

image

@codeclimate
Copy link

codeclimate bot commented Jul 22, 2023

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

View more on Code Climate.

@Jarsen136
Copy link
Contributor Author

Ways of triggering the error: go to a collection page and switch between activity and items go to a nft and click on the link that leads to its collection

the redundant collection id does not show up on the address bar anymore, but the warning still appears:

image

✅ Fixed. I have removed the annoying warning. TBH, this warning is related to other issues instead of the collection search.

Copy link
Member

@vikiival vikiival left a comment

Choose a reason for hiding this comment

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

CODE LGTM

@prury please check if it works as intended

@prury
Copy link
Member

prury commented Jul 22, 2023

CODE LGTM

@prury please check if it works as intended

yup, no more warnings and redundancy

@prury prury added the S-works-for-me-✅ qa-guild has tested PR from end user perspective and functionality worked label Jul 22, 2023
@vikiival
Copy link
Member

pay 15 usd

@vikiival vikiival merged commit ac3aee2 into kodadot:main Jul 24, 2023
@yangwao
Copy link
Member

yangwao commented Jul 24, 2023

😍 Perfect, I’ve sent the payout
💵 $15 @ 5.31 USD/DOT ~ 2.825 $DOT
🧗 16SjUbGKSdjCdWTy3NNT3JxbRVGGqD4mwkHpc6BD9U2Rp29Z
🔗 0xc3930d57516ea0bae576a661e23fdd0bf6667cea26ecafa971a637417b36aab2

🪅 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 24, 2023
This was referenced Jul 25, 2023
This was referenced Aug 2, 2023
This was referenced Aug 2, 2023
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 small Pull request is small waiting-for-review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

collectionId in path
5 participants