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

Search items on top of collections #5851

Merged
merged 16 commits into from
May 9, 2023
Merged

Conversation

Jarsen136
Copy link
Contributor

@Jarsen136 Jarsen136 commented May 3, 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

Context

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

Optional

  • I've tested it at </ksm/collection>
  • I've tested PR on mobile
  • I've written unit tests 🧪
  • I've found edge cases

Had issue bounty label?

  • Fill up your KSM address: Payout

Community participation

Screenshot 📸

  • My fix has changed something on UI; a screenshot is best to understand changes for others.

image

Copilot Summary

🤖 Generated by Copilot at c1d6e3b

This pull request adds a new feature to enable searching for collections and within collections on the NFT gallery. It also refactors some utility functions and components, updates import paths, and adds new theme variables and styles for the UI. The main files affected are components/search/Search.vue, components/search/SearchBar.vue, components/search/SearchCollection.vue, components/shared/BreadcrumbsFilter.vue, components/shared/gallery/NeoTag.vue, locales/en.json, styles/abstracts/_theme.scss, styles/components/_search.scss, and components/search/utils/useCollectionSearch.ts. The rest of the files are mostly import path changes.

🤖 Generated by Copilot at c1d6e3b

Oh we're searching for collections on the web
We've moved the exist function to a better place
We've added a blue tag to toggle the mode
And we've styled it with variables for the theme

@Jarsen136 Jarsen136 requested a review from a team as a code owner May 3, 2023 10:39
@Jarsen136 Jarsen136 requested review from preschian and daiagi and removed request for a team May 3, 2023 10:39
@kodabot
Copy link
Collaborator

kodabot commented May 3, 2023

SUCCESS @Jarsen136 PR for issue #5333 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 May 3, 2023

Deploy Preview for koda-canary ready!

Name Link
🔨 Latest commit cecf4e2
🔍 Latest deploy log https://app.netlify.com/sites/koda-canary/deploys/645a5f8b73ecf90008c21fe9
😎 Deploy Preview https://deploy-preview-5851--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 settings.

@reviewpad
Copy link
Contributor

reviewpad bot commented May 3, 2023

AI-Generated Summary: This pull request contains several changes in the codebase, summarized as follows:

  1. Removal of unused components SearchCollection.vue and TypeTagInput.vue.
  2. Refactoring of exist function import in various components.
  3. Addition of search functionality in the collection page.
  4. Addition of blue theme tag to search bar.
  5. Update in the search process with collection ID.
  6. Disabling of the collection search feature on mobile devices.

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

daiagi commented May 3, 2023

@exezbcz
let's say i hit backspace, and then i want to resume collection search
right now i need to refresh the page

is it something we want to support?

Copy link
Contributor

@daiagi daiagi left a comment

Choose a reason for hiding this comment

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

it doesn't respond to screen width change
image

got here by starting on a wide screen and then shrinking

components/shared/BreadcrumbsFilter.vue Outdated Show resolved Hide resolved
styles/components/_search.scss Outdated Show resolved Hide resolved
styles/abstracts/_theme.scss Outdated Show resolved Hide resolved
Jarsen136 and others added 2 commits May 3, 2023 23:56
Co-authored-by: Luke Fishman <daiagi@gmail.com>
Co-authored-by: Luke Fishman <daiagi@gmail.com>
@exezbcz
Copy link
Member

exezbcz commented May 3, 2023

@daiagi yes, Thank you for pointing that out. It's a first version, so there will be changes, and the search recommendations will need to be changed as well - I believe that once we finish this first version, we can move on to adding and refining it. So, right now there is not an option to resume collection search.

@Jarsen136
Copy link
Contributor Author

it doesn't respond to screen width change
got here by starting on a wide screen and then shrinking

✅ Fixed.

@Jarsen136 Jarsen136 requested a review from daiagi May 3, 2023 16:13
@exezbcz
Copy link
Member

exezbcz commented May 3, 2023

Amazing!
some feedback:

image

  • can you please remove the hover on text
    image

let's say i hit backspace, and then i want to resume collection search right now i need to refresh the page

quick hack for this:

  • if user deletes the tag but then does not search but click out of the search bar - the tag is added again, so that means the tag is always there unless you leave the collection

@Jarsen136
Copy link
Contributor Author

Amazing! some feedback:

image

  • can you please remove the hover on text
    image

let's say i hit backspace, and then i want to resume collection search right now i need to refresh the page

quick hack for this:

  • if user deletes the tag but then does not search but click out of the search bar - the tag is added again, so that means the tag is always there unless you leave the collection

✅Updated

Copy link
Contributor

@daiagi daiagi 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 ✔️

@daiagi daiagi added S-code-lgtm-✅ code review guild has reviewed this PR and it's code is approved S-works-for-me-✅ qa-guild has tested PR from end user perspective and functionality worked labels May 4, 2023
@yangwao
Copy link
Member

yangwao commented May 5, 2023

@exezbcz let's drop visual-ok 👀

@yangwao
Copy link
Member

yangwao commented May 5, 2023

I would like to see some suggestions on writing as query on top of collection is much easier and less heavy to search.
So similar we are having on classic search showing items, but on top

Plus for next design can include wrapped name of collection for like 40 chars

We can do this as followup

@Jarsen136
Copy link
Contributor Author

I would like to see some suggestions on writing as query on top of collection is much easier and less heavy to search. So similar we are having on classic search showing items, but on top

Plus for next design can include wrapped name of collection for like 40 chars

We can do this as followup

I would take a try after this PR merged

@codeclimate
Copy link

codeclimate bot commented May 9, 2023

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

View more on Code Climate.

@vikiival
Copy link
Member

vikiival commented May 9, 2023

pay 80 usd

@vikiival vikiival merged commit 2a950ae into kodadot:main May 9, 2023
18 of 19 checks passed
@yangwao
Copy link
Member

yangwao commented May 9, 2023

😍 Perfect, I’ve sent the payout
💵 $80 @ 25.81 USD/KSM ~ 3.1 $KSM
🧗 Caiv9TbPz68q5dC8EcHu5xKYPRnremimGzqmEejDFNpWWLG
🔗 0xa4fdca0fa6cfdc04bb2d893fa90acbfcb68509d43febb5b22e5af0b56f9f3b71

🪅 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 May 9, 2023
@yangwao yangwao mentioned this pull request May 10, 2023
@daiagi daiagi mentioned this pull request Jul 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
medium Pull request is medium paid pull-request has been paid S-code-lgtm-✅ code review guild has reviewed this PR and it's code is approved S-visual-ok-✅ S-works-for-me-✅ qa-guild has tested PR from end user perspective and functionality worked waiting-for-review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Search on top of collections v0.9
7 participants