Feature - Implement basic search functionality v2 #159

barrymun · 2025-01-03T16:53:19Z

Description

These changes will allow the search bar to work as intended.
Changes in the PR are heavily inspired by Add basic search functionality #34 and the work completed by @GreenMan36 and the other comments/suggestions within that PR.
A demo can be found here.

Type of Change

✨ New snippet
🛠 Improvement to an existing snippet
🐞 Bug fix
📖 Documentation update
🔧 Other (please describe): Feature - implementing basic search functionality

Checklist

I have tested my code and verified it works as expected.
My code follows the style and contribution guidelines of this project.
Comments are added where necessary for clarity.
Documentation has been updated (if applicable).
There are no new warnings or errors from my changes.

Related Issues

Closes #64 #56

Additional Context

Screenshots (Optional)

Click to view screenshots

netlify · 2025-01-03T16:53:36Z

✅ Deploy Preview for quicksnip ready!

Name	Link
🔨 Latest commit	`bfe0b4b`
🔍 Latest deploy log	https://app.netlify.com/sites/quicksnip/deploys/677816022c5c3e0008d11d4b
😎 Deploy Preview	https://deploy-preview-159--quicksnip.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

dostonnabotov · 2025-01-03T17:07:54Z

Hey, @barrymun. Great work. 👍 After seeing the demo, I noticed that there is also All snippets category. Will we have this in the final version as well? 🤔

barrymun · 2025-01-03T17:11:57Z

@dostonnabotov This feature was in v1 (#34) and I thought it was to be implemented here too?

Mathys-Gasnier

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 as q=, 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

Mathys-Gasnier · 2025-01-03T17:23:49Z

src/router.tsx

This is the old way of handing routers in react router dom, please see: https://reactrouter.com/start/library/routing#routing

I will see what I can do with regards the lang and category selection in the routes. I will also update the code so we use the new routing method.

I've made some changes that should get this working. I haven't fully tested it, so it's not ready for merge, but I will confirm everything asap. The netlify build currently points to an older commit and I cannot force it to point to the latest commit, so this will need to be done if you would like to view the changes.

Demo so far (there are some things that I don't like that will be fixed later): https://jam.dev/c/acdebbf9-df85-4a98-945e-48392067d9d8

dostonnabotov · 2025-01-03T18:02:31Z

@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.

barrymun · 2025-01-03T18:25:07Z

@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).

… structure

barrymun · 2025-01-04T08:17:47Z

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.

…l, search and snippet set as query params

Mathys-Gasnier

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

barrymun · 2025-01-10T12:02:54Z

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.

Mathys-Gasnier · 2025-01-10T12:16:38Z

Lets wait on another review and we are good to go

barrymun · 2025-01-10T12:18:16Z

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.

Mathys-Gasnier · 2025-01-10T16:21:36Z

We disabled auto previews because we were running out of build time with our current plan, they will need to clone and test localy

saminjay · 2025-01-10T17:16:18Z

It works great but there are few minor issues,

When you first open the website, it will show JavaScript -> Array Manipulation but will not update the url. I think you should redirect the default path / to javascript/array-manipulation or something else.
When we are on the root path / and open the snippet it works correctly, it is even sharable. But does not add javascript/array-manipulation to the url bar. Its just the first issue.
The search is not working live. You have to press enter to search, I think it will be better to show the results while typing and only change the url on enter.
Also I think search should also update on blur.

Mathys-Gasnier · 2025-01-10T17:51:29Z

I'm not a big fan of the site redirecting / to javascript/array-manipulation... I think it clutters the URL when you open the website...

One option would be to, in the futur have a real homepage, with just the selection of languages

saminjay · 2025-01-10T18:29:16Z

I'm not a big fan of the site redirecting / to javascript/array-manipulation... I think it clutters the URL when you open the website...
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 / and all the other on their respective routes. Redirecting or replacing feels like a good alternative until we add a dashboard or home or something.

barrymun · 2025-01-10T18:29:16Z

@saminjay

I can make the URL redirect to javascript/array-manipulation when starting at /, but it seems @Mathys-Gasnier is not a fan of this.
My understanding (based on the previous PR), was to only search when "Enter" was pressed. I can of course change this to onKeyup and onBlur but I need the other reviewers to confirm this behaviour first.

I'll keep on eye on the thread and see that everyone can agree on the functionality here. Thanks for reviewing

Mathys-Gasnier · 2025-01-10T18:52:14Z

For the search yeah it shouldnt require you to press enter. And it should update the url on blur and on enter

… search will be performed as the user types, some refactoring

barrymun · 2025-01-10T22:11:54Z

I've made the following changes to the search functionality:

search will now be performed as the user types.
removed keyup listener for Enter key press, and removed onBlur functionality, as these are simply not required given that the search is now performed as the user types.
as before, the search is cleared if it is focused and the user presses the Escape key.
as before, the search can be cleared by clicking the x button that appears if there is text in the search input.
as before, the search element can be focused by pressing the / key.

Let me know if any other changes are required.

barrymun added 2 commits January 3, 2025 15:36

working search feature

8bb107c

Merge branch 'main' into feature/search

bfe0b4b

barrymun requested review from psychlone77 and saminjay as code owners January 3, 2025 16:53

barrymun mentioned this pull request Jan 3, 2025

Add basic search functionality #34

Closed

Mathys-Gasnier requested changes Jan 3, 2025

View reviewed changes

actions-user and others added 3 commits January 3, 2025 21:19

Update consolidated snippets

b78c224

Merge branch 'main' into feature/search

cb4e762

upgrading react-router-dom package to latest and utilising new routes…

558dff5

… structure

majvax added the enhancement New feature or request label Jan 4, 2025

barrymun added 4 commits January 9, 2025 22:19

navigation concept (wip)

965cce5

Merge branch 'main' into feature/search

5aa4619

working search functionality with language and category set in the ur…

43bcf18

…l, search and snippet set as query params

create an enum for query params, change search to q

1b4e679

Mathys-Gasnier approved these changes Jan 10, 2025

View reviewed changes

Mathys-Gasnier mentioned this pull request Jan 10, 2025

[Feature] - Add a page to see every snippets for one language #224

Open

4 tasks

barrymun added 2 commits January 10, 2025 21:47

Merge branch 'main' into feature/search

70e716c

remove keyup listener for Enter button as this is no longer required,…

9904aec

… search will be performed as the user types, some refactoring

Mathys-Gasnier approved these changes Jan 11, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature - Implement basic search functionality v2 #159

Feature - Implement basic search functionality v2 #159

barrymun commented Jan 3, 2025 •

edited

Loading

netlify bot commented Jan 3, 2025 •

edited

Loading

dostonnabotov commented Jan 3, 2025

barrymun commented Jan 3, 2025

Mathys-Gasnier left a comment

Mathys-Gasnier Jan 3, 2025

barrymun Jan 3, 2025

barrymun Jan 10, 2025

barrymun Jan 10, 2025

dostonnabotov commented Jan 3, 2025

barrymun commented Jan 3, 2025

barrymun commented Jan 4, 2025

Mathys-Gasnier left a comment

barrymun commented Jan 10, 2025 •

edited

Loading

Mathys-Gasnier commented Jan 10, 2025

barrymun commented Jan 10, 2025

Mathys-Gasnier commented Jan 10, 2025

saminjay commented Jan 10, 2025

Mathys-Gasnier commented Jan 10, 2025

saminjay commented Jan 10, 2025

barrymun commented Jan 10, 2025

Mathys-Gasnier commented Jan 10, 2025

barrymun commented Jan 10, 2025 •

edited

Loading

Feature - Implement basic search functionality v2 #159

Are you sure you want to change the base?

Feature - Implement basic search functionality v2 #159

Conversation

barrymun commented Jan 3, 2025 • edited Loading

Description

Type of Change

Checklist

Related Issues

Additional Context

Screenshots (Optional)

netlify bot commented Jan 3, 2025 • edited Loading

✅ Deploy Preview for quicksnip ready!

dostonnabotov commented Jan 3, 2025

barrymun commented Jan 3, 2025

Mathys-Gasnier left a comment

Choose a reason for hiding this comment

Mathys-Gasnier Jan 3, 2025

Choose a reason for hiding this comment

barrymun Jan 3, 2025

Choose a reason for hiding this comment

barrymun Jan 10, 2025

Choose a reason for hiding this comment

barrymun Jan 10, 2025

Choose a reason for hiding this comment

dostonnabotov commented Jan 3, 2025

barrymun commented Jan 3, 2025

barrymun commented Jan 4, 2025

Mathys-Gasnier left a comment

Choose a reason for hiding this comment

barrymun commented Jan 10, 2025 • edited Loading

Mathys-Gasnier commented Jan 10, 2025

barrymun commented Jan 10, 2025

Mathys-Gasnier commented Jan 10, 2025

saminjay commented Jan 10, 2025

Mathys-Gasnier commented Jan 10, 2025

saminjay commented Jan 10, 2025

barrymun commented Jan 10, 2025

Mathys-Gasnier commented Jan 10, 2025

barrymun commented Jan 10, 2025 • edited Loading

barrymun commented Jan 3, 2025 •

edited

Loading

netlify bot commented Jan 3, 2025 •

edited

Loading

barrymun commented Jan 10, 2025 •

edited

Loading

barrymun commented Jan 10, 2025 •

edited

Loading