Add search URL params - #282
Conversation
|
Looks good to me. Let’s wait to see what @paweljarosz thinks. Thank you for your contribution! |
|
|
||
| const search = params.toString(); | ||
| const nextUrl = `${window.location.pathname}${search ? `?${search}` : ""}${window.location.hash}`; | ||
| window.history.replaceState(window.history.state, "", nextUrl); |
There was a problem hiding this comment.
My only concern is a frequent call to replaceState at every input, but I'm not sure if this will be much of an issue in production, compared to the benefits of the PR
There was a problem hiding this comment.
It's a valid question. I don’t think this should be an issue. replaceState() is relatively lightweight and, unlike pushState(), doesn’t create a new history entry on every input. It only updates the current URL without triggering navigation or reloading the page.
paweljarosz
left a comment
There was a problem hiding this comment.
Looks good to me (beside a small comment above), thank you so much for the contribution! <3
|
The build looks green✅! P.S. In my next PR I will migrate the webiste to HUGO 😄 20 minutes build is a bit too much! |
Yeah, it has ballooned out of proportion. We chose Jekyll because it was the default for GitHub Pages, but we could migrate to something else, such as Hugo, which is supposed to be a lot faster |
Closes: #281
Changes:
assets_search_url.mp4