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

Removed query-string, use goto svelte for url state changes #97

Merged
merged 12 commits into from
Aug 27, 2022
Merged

Removed query-string, use goto svelte for url state changes #97

merged 12 commits into from
Aug 27, 2022

Conversation

ak4zh
Copy link
Collaborator

@ak4zh ak4zh commented Aug 22, 2022

  • Added new environment variable POST_CATEGORIES
  • Added post filters using post categories
  • Removed the use of query-string lib as the same task can be achieved with svelte '$page.url.searchParams`
  • Removed all urlState functions and changed to use svelte goto instead.

Comment on lines -49 to -75
onMount(() => {
if (location.search.length < 1) return; // early terminate if no search
let givenstate = queryString.parse(location.search);
if (!Array.isArray(givenstate.show)) givenstate.show = [givenstate.show];
// if (!givenstate.show.includes('Essays')) essays = false;
// if (!givenstate.show.includes('Talks')) talks = false;
// if (!givenstate.show.includes('Podcasts')) podcasts = false;
// if (!givenstate.show.includes('Tutorials')) tutorials = false;
// if (!givenstate.show.includes('Snippets')) snippets = false;
// if (!givenstate.show.includes('Notes')) notes = false;
if (givenstate.filter) search = givenstate.filter;
urlState = { ...defaultURLState, ...givenstate };
});
function saveURLState() {
setTimeout(() => {
setURLState({
filter: search,
show: [
// essays && 'Essays',
// talks && 'Talks',
// podcasts && 'Podcasts',
// snippets && 'Snippets',
// tutorials && 'Tutorials',
// notes && 'Notes'
].filter(Boolean)
});
}, 100);
Copy link
Owner

Choose a reason for hiding this comment

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

this code is required for URLs to be copy pastable

eg when i search something on my site it shows up in the URL, and can be copy pasted

image

but on this PR it doesnt

image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

14691a7
Fixed the URLs to be updated with search filter.

src/lib/siteConfig.js Outdated Show resolved Hide resolved
@swyxio
Copy link
Owner

swyxio commented Aug 22, 2022

appreciate the PR! left some thoughts before merging, curious what you think. let me know if you are interested in being a maintainer as well - i just ask that you send every change in as a PR so that we can discuss first and I can keep track

@ak4zh
Copy link
Collaborator Author

ak4zh commented Aug 22, 2022

I think URL parameter name should be changed as below for a more clear naming:

  • show to categories or tags (I prefer tags as it's shorter)
  • filter to search or keyword (I prefer keyword here as search gives a feeling of path rather that a param)

Let me know if that looks ok.

@swyxio
Copy link
Owner

swyxio commented Aug 25, 2022

almost! when you go to here: https://deploy-preview-97--swyxkit.netlify.app/blog?filter=test&show=Blog

the "show" code is dropped somehow

image

@swyxio
Copy link
Owner

swyxio commented Aug 25, 2022

i tried fixing it but couldnt find the source of the bug - i can get back to it in future, thank you for the contribution, it is very valued.

@ak4zh
Copy link
Collaborator Author

ak4zh commented Aug 26, 2022

i tried fixing it but couldnt find the source of the bug - i can get back to it in future, thank you for the contribution, it is very valued.

Fixed.
It seems it was happening because both getting the param get value and set were reactive and running at same time.

README.md Outdated Show resolved Hide resolved
@@ -49,6 +49,7 @@ See https://swyxkit.netlify.app/ (see [Deploy Logs](https://app.netlify.com/site
- **Minor design/UX touches**
- Top level blog URLs (`/myblog` instead of `/blog/myblog` - [why](https://www.swyx.io/namespacing-sites/))
- Blog index truncates at 20 posts to make sure to render quickly
- Blog search/facets serialize to URLs for easy copy paste ([thanks @Ak4zh](https://github.com/sw-yx/swyxkit/pull/97))
Copy link
Owner

Choose a reason for hiding this comment

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

added attribution to you @ak4zh :)

@swyxio
Copy link
Owner

swyxio commented Aug 27, 2022

thank you so much!

@swyxio swyxio merged commit f6eba73 into swyxio:main Aug 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants