-
-
Notifications
You must be signed in to change notification settings - Fork 136
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
Feature - Implement basic search functionality v2 #159
Conversation
✅ Deploy Preview for quicksnip ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Hey, @barrymun. Great work. 👍 After seeing the demo, I noticed that there is also |
@dostonnabotov This feature was in v1 (#34) and I thought it was to be implemented here too? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seeing as you changed a lot of things already, could you maybe add the language + category path to the url ? And make snippets link shareable to fix #56
IMO we should to it like so:
/
Default lang + default category/javascript
Lang + default category/javascript/category
Lang + Category*?q=search
Searching in,*
being the lang, category or lack of, in which you want the search to happen*?snippet=snippet-name
Opens the snippet preview,*
has the same behavior asq=
, both can be found at the same time to preserve search too
And we should be able to reconstruct the page based on that, for example:
quicksnip.dev/javascript/basics?q=hello&snippet=hello-world
Should open quicksnip on the javascript language, in the basics category, with a search of hello
and the hello-world
snippet opened
@barrymun I see that you've been actively participating on improving QuickSnip. It would be awesome if you could join our Discord server. I will give you access to our maintainers channel, in which we discuss ideas and plans for QuickSnip. You have been really helpful for our project. |
@dostonnabotov Thank you, I'm already a member, and my username should be the same as it is here (barrymun). |
I will be away until next week but I think I have a decent solution for this, and will hopefully have the code ready shortly after I return. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
…L query param and not the search text, remove the searchVal state and just use the AppContext searchText state instead as it is cleaner, ensure that the search term is preserved when switching categories should one exist
Made some additional changes to fix some small bugs and refactored the code a little so that it is cleaner, but it should be ready now. |
Lets wait on another review and we are good to go |
Do you know if it's possible to point the netlify deploy to the latest commit for this branch? It still seems to be referencing the commit from last week and it would make it easier for others to test if it were updated. |
We disabled auto previews because we were running out of build time with our current plan, they will need to clone and test localy |
It works great but there are few minor issues,
|
I'm not a big fan of the site redirecting One option would be to, in the futur have a real homepage, with just the selection of languages |
We need something though, It doesn't make any sense to have one category on |
Do you think it would be better to revert the latest change, add this "default" category which would be "All Snippets" (or equivalent) and this would show all snippets for the selected language? |
I see your point. searching in a specific language would be easier. I was also thinking like search should apply globally. For example, when I land on a website and I need to find "flatting a list in Python", I wouldn't need to first go to Python itself to be able to search it. I feel like in terms of UX, user should be able to find any snippet with search, or at least, that's what he expects for the first time.
With above approach, I think filters would be more useful in the future as we could use them to filter languages as well. Let me know your thoughts on this approach. It's always good to discuss various perspectives. |
…ages, categories and snippets" This reverts commit 31e622c.
…his is the default category for any langauge and will show all the snippets for that language.
@dostonnabotov @Mathys-Gasnier I've reverted the most recent commit which added global search and reworked the search feature again to include a default category (for all languages) which is labelled |
IMO adding a popup/search page is the thing that would complexify the UI, and doing a search capable of understanding things like Searching globally serves no point, like if I want to find python snippets, why should i bother writting "in Python" to not have all the clutter around it when there is already something to do exactly that in the UI |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good.
Some improvements for future could be, adding a NotFound page, and improving SEO capabilities to this site.
I was also thinking using external library to do the heavy lifting. Algolia for Open-Source has very generous offers for open source projects. Since it takes care of search stuff, we can focus on our other upcoming features. Let me know your thoughts. |
Algolia (even though it would be a nice addition) might be overkill at this stage? |
I think an external lib for a project like quicksnip is way overkill |
As discussed in #34, we could use https://lucaong.github.io/minisearch/ if needed in the future, for more advanced search results. |
Since many are agreeing on not using external lib, we can continue with our own search functionality. And, regarding the current implementation, only searching within a specific language doesn't appeal to me. It would be awesome to implement the global search. Perhaps, we might have to consolidate all files in a single file. For example, in MDN (https://developer.mozilla.org/en-US/) website, you don't first select the language, then search things within it. You just search and it shows everything in the documentation. If it's a lot of headache to implement, we could at least use Minisearch, https://lucaong.github.io/minisearch/ to handle it. I would like to hear your thoughts on this. |
This commit generates an “all.json” file which allows the user to search through every language and category uploaded using a global search. Stylistically it is closer to GitHub as opposed to Mozilla. No minisearch required as of yet. On local make this commit the head and see if you prefer this implementation. |
I know that things like modzilla do it globally, but they are a documentation... |
I like this implementation. It looks closely to what we were discussing in Discord. I noticed routing system is kinda interesting:
I believe snippet sharing and searching are mixing up together. Perhaps, we could remove the searching query from the url and make the snippet sharing on its own? While searching could be managed with state or context api. @Mathys-Gasnier and @barrymun, let me know your thoughts.
|
IMO we should keep it as is, it allows for sharing snippets, but also searchs, for me, it happens often that i want to share something with the search, especially with beginners, to show them how to search for something, this would allow to share searchs to multiple snippets |
@Mathys-Gasnier how do you want to handle the new |
honestly, don't search too far, just add it to the url |
Sharing has now been reworked to handle the new sub language selection. |
Hey, there. Awesome work xD. Will review it. |
Description
Type of Change
Checklist
Related Issues
Closes #64 #56 #224
Additional Context
Screenshots (Optional)
Click to view screenshots