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.

Sign up for GitHub

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

Jump to bottom

Blog: Added filter by tags #466

Merged
merged 30 commits into from Sep 1, 2022
Merged

Blog: Added filter by tags #466

merged 30 commits into from Sep 1, 2022

Conversation

NLTN
Copy link
Collaborator

@NLTN NLTN commented May 12, 2022

  • Created TagField Component
    File Location: src/lib/components/utils/acm-tagfield.svelte
    Usage: Allows users to make selections and filter content.

  • Created new file utils.ts to store common functions
    File Location: src/lib/common/utils.ts

  • Added Filter by Tags feature for Blog page.
    Made some changes to these files:

    • In blog/index.svelte: Added TagField component.
    • In blog/_query.ts: Modified fetchNewsletters.
    • In blog/index.json.ts: Modified get function.

This PR is intended to resolve #277

@NLTN NLTN added the enhancement New feature or request label May 12, 2022
@vercel
Copy link

vercel bot commented May 12, 2022 •
edited

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
acm-csuf-site ✅ Ready (Inspect) Visit Preview Sep 1, 2022 at 1:08AM (UTC)

EthanThatOneKid
EthanThatOneKid requested changes May 12, 2022
View reviewed changes
Copy link
Owner

@EthanThatOneKid EthanThatOneKid left a comment •
edited

Choose a reason for hiding this comment

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

This PR represents a very complicated and well thought out feature. I left several suggestions regarding Svelte/TypeScript techniques.

Do not hesitate to ask for clarification for any of the comments I left. Please reply to each comment with something like "Done." so that I can resolve the individual comment if it is ready to be resolved.

src/routes/blog/index.svelte Outdated Show resolved Hide resolved
src/routes/blog/index.xml.ts Outdated Show resolved Hide resolved
src/lib/components/utils/acm-tagfield.svelte Outdated Show resolved Hide resolved
src/routes/blog/_query.ts Outdated Show resolved Hide resolved
src/routes/blog/_query.ts Outdated Show resolved Hide resolved
src/routes/blog/index.svelte Outdated Show resolved Hide resolved
src/routes/blog/index.svelte Outdated Show resolved Hide resolved
src/routes/blog/index.svelte Show resolved Hide resolved
src/routes/blog/index.svelte Outdated Show resolved Hide resolved
src/routes/blog/index.svelte Outdated Show resolved Hide resolved
@NLTN @EthanThatOneKid
Update src/lib/components/utils/acm-tagfield.svelte
98affdb 
Great suggestion. Much cleaner code & avoid bugs.

Co-authored-by: Ethan Davidson <31261035+EthanThatOneKid@users.noreply.github.com>
@vercel vercel bot temporarily deployed to Preview May 12, 2022 03:49 Inactive
@NLTN @EthanThatOneKid
Update src/routes/blog/index.xml.ts
2021d42 
OMG. Deconstructor syntax is so helpful.

Co-authored-by: Ethan Davidson <31261035+EthanThatOneKid@users.noreply.github.com>
@vercel vercel bot temporarily deployed to Preview May 12, 2022 05:42 Inactive
@vercel vercel bot temporarily deployed to Preview May 13, 2022 07:03 Inactive
@vercel vercel bot temporarily deployed to Preview May 13, 2022 07:34 Inactive
@vercel vercel bot temporarily deployed to Preview May 13, 2022 09:00 Inactive
@vercel vercel bot temporarily deployed to Preview May 13, 2022 23:41 Inactive
@vercel vercel bot temporarily deployed to Preview May 15, 2022 06:14 Inactive
@vercel vercel bot temporarily deployed to Preview May 27, 2022 07:14 Inactive
EthanThatOneKid
EthanThatOneKid reviewed May 27, 2022
View reviewed changes
Copy link
Owner

@EthanThatOneKid EthanThatOneKid left a comment

Choose a reason for hiding this comment

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

The code in this PR overall looks amazing! I left a couple new nitpicks related to @jaasonw's #466 (comment) and one simply making a suggestion to space out your SCSS blocks. I cannot wait to see the search bar in production :) Getting this feature to work with JavaScript disabled was no easy task!

src/routes/blog/index.json.ts Outdated Show resolved Hide resolved
src/routes/blog/index.svelte Outdated Show resolved Hide resolved
src/routes/blog/index.xml.ts Outdated Show resolved Hide resolved
src/lib/components/utils/acm-labelfield.svelte Outdated Show resolved Hide resolved
@NLTN @EthanThatOneKid
Update src/lib/components/utils/acm-labelfield.svelte
111206b 
Co-authored-by: Ethan Davidson <31261035+EthanThatOneKid@users.noreply.github.com>
@vercel vercel bot temporarily deployed to Preview May 28, 2022 01:50 Inactive
@NLTN @EthanThatOneKid
Update src/routes/blog/index.svelte
1d52644 
Co-authored-by: Ethan Davidson <31261035+EthanThatOneKid@users.noreply.github.com>
@vercel vercel bot temporarily deployed to Preview June 20, 2022 15:24 Inactive
jaasonw
jaasonw suggested changes Jun 21, 2022
View reviewed changes
Copy link
Contributor

@jaasonw jaasonw left a comment

Choose a reason for hiding this comment

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

It does not appear to be workin

image

src/routes/blog/index.json.ts Outdated Show resolved Hide resolved
src/routes/blog/_query.ts Outdated Show resolved Hide resolved
@NLTN
Copy link
Collaborator Author

NLTN commented Jun 21, 2022

It does not appear to be workin

image

I don’t know what happened. Maybe you’re in DEBUG mode or maybe there are something wrong in “.env” file.
Everything works perfectly on my PC.

@jaasonw
Copy link
Contributor

jaasonw commented Jun 27, 2022

@NLTN Oh that's not my local environment it's the vercel preview linked in #466 (comment), if you want we can try to find some time to work on this maybe this week or a week from now (the week of 7/10)? If not we can push this back to when the semester starts

NLTN
NLTN commented Jun 29, 2022
View reviewed changes
src/routes/blog/index.json.ts Outdated Show resolved Hide resolved
jaasonw
jaasonw suggested changes Jul 28, 2022
View reviewed changes
src/routes/blog/index.svelte Outdated Show resolved Hide resolved
@vercel vercel bot temporarily deployed to Preview August 29, 2022 09:06 Inactive
@EthanThatOneKid
Fixed a mistake made when merging conflicts
c0b4f53 
Somehow, I managed to empty out the contents of `/src/lib/common/utils.ts`. I put them back, merged.
@vercel vercel bot temporarily deployed to Preview August 29, 2022 09:13 Inactive
@vercel vercel bot temporarily deployed to Preview August 29, 2022 09:35 Inactive
@vercel vercel bot temporarily deployed to Preview August 29, 2022 10:21 Inactive
@vercel vercel bot temporarily deployed to Preview August 30, 2022 03:27 Inactive
@vercel vercel bot temporarily deployed to Preview August 30, 2022 05:12 Inactive
@EthanThatOneKid
Merge branch 'main' into fix/277
1afde39
@EthanThatOneKid
Revert "Revert "Refactored caching logic for individual blog posts (#515
c729607 
)" (#524)"

This reverts commit 7db1f3b.
@EthanThatOneKid
Fixed typo in blog page title
5fb3e7d 
[4:48 PM]jasonw: i forgot to put post but
[4:48 PM]jasonw: change blog back to readme
@vercel vercel bot temporarily deployed to Preview September 1, 2022 01:08 Inactive
jaasonw
jaasonw approved these changes Sep 1, 2022
View reviewed changes
Copy link
Contributor

@jaasonw jaasonw left a comment

Choose a reason for hiding this comment

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

👍

@NLTN NLTN merged commit a9efcdb into main Sep 1, 2022
@jaasonw jaasonw deleted the fix/277 branch September 7, 2022 07:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jaasonw jaasonw jaasonw approved these changes

@EthanThatOneKid EthanThatOneKid EthanThatOneKid approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Blog: Create filter-by-tag feature
3 participants
@NLTN @jaasonw @EthanThatOneKid