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

Filter for beginner friendly trips #106

Merged
merged 1 commit into from
Dec 24, 2024
Merged

Filter for beginner friendly trips #106

merged 1 commit into from
Dec 24, 2024

Conversation

Raikhen
Copy link
Contributor

@Raikhen Raikhen commented Dec 22, 2024

This simply allows you filter trips on whether they are beginner-friendly or not (see issue #7). The implementation is not great because I'm still figuring out htmx but by the time I finish with the filters it'll be improved.

@alexpetros
Copy link
Member

alexpetros commented Dec 24, 2024 •
edited
Loading

Hey there 👋 welcome to the project!

This is really nice work. I made a couple cleanups to remove the variables and simplify the templates a bit: mainly I removed some nesting and moved some logic to the JS side.

The htmx stuff is great too. The only thing to keep in mind is that right now the backend doesn't know that the filter buttons are regular links but the checkbox is an htmx link, so it sends the whole page in response to each request. For the htmx-enabled checkbox, that means that the server is re-sending the <head> items (title, script tags, and so on) and the hx-target link is telling that to go to body. This matters basically zero, as far as I can tell—the browser just puts all that stuff into the head anyway. But something to keep in mind if you plan to add more filters there (ping me if you're gonna do that too, I have a couple ideas).

Also, I added hx-push-url so that that URL changes when you change the input, and autocomplete=off which ensures that the server has full control over the "checked" state of the checkbox (this pretty much only affects Firefox users, which I very stubbornly am).

Link to my changes: db531e9

@alexpetros alexpetros merged commit 4e3ee61 into dartmouth-outing-club:main Dec 24, 2024
1 check passed
@Raikhen
Copy link
Contributor Author

Raikhen commented Dec 24, 2024

Thanks for the welcome! I didn't know something like hx-push-url existed, that's great. I was aware that the server was sending the whole page back (although I didn't realize that this implied that the head would be added to the body) instead of just the HTML that changed. I plan to fix it and add the other filters but I'm still familiarizing myself with the stack. Also, I'm waiting to hear back from Rory to see if he wants to hire me or not lol.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexpetros alexpetros alexpetros approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Raikhen @alexpetros