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

feat: persist region filter in URL in and across pages #493

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

mateusfpleite
Copy link
Collaborator

@mateusfpleite mateusfpleite commented Jan 11, 2025

Closes #466.

This PR:

  • Add filter param to URL when one of the regions is selected in Events, Partners, or Index page;
  • Persist the region in the URL when switching from one of these pages to the others.

I’m not entirely sure this is the best way to implement this feature, but I couldn’t find any other approach. In Elm Pages, there doesn’t appear to be a straightforward way to manage or track URL changes directly (using anchor elements with preventDefault to change the URL resulted in strange scrolling behavior).

Since this is my first time using Elm Pages, I may be missing a simpler approach. I’d be happy to hear any suggestions on how to do this in a cleaner way, ideally without using JavaScript/ports and direct history manipulation.

Regarding testing, ideally we should test the behavior of storing and retrieving the URL filter, but I haven’t had much success with that either. It seems to me that this is a case where it would be much simpler to test the entire behavior using a framework like the one suggested by @kimadactyl in #492

Copy link

cloudflare-workers-and-pages bot commented Jan 11, 2025

Deploying the-trans-dimension with  Cloudflare Pages  Cloudflare Pages

Latest commit: e95fcc1
Status: ✅  Deploy successful!
Preview URL: https://c6ee61e9.the-trans-dimension.pages.dev
Branch Preview URL: https://feat-persist-region-url.the-trans-dimension.pages.dev

View logs

Copy link

Connected to Huly®: TD-494

@kimadactyl
Copy link
Member

This works great on the preview branch, great job!

My only comment is to make the filter param region instead of filter so we can add more later.

Otherwise way outside my ability to review, can you have a look please @katjam?

@katjam
Copy link
Member

katjam commented Jan 15, 2025

I'll make time to look at this tomorrow. I think you can track url changes in elm-pages. We access the query values when they occur in the URL so I am assuming we can sync that to the model. It might not be documented very well and I may be missing some obvious reason it won't work.

@katjam
Copy link
Member

katjam commented Jan 22, 2025

I am still making time to look at this! Sorry for delay. :)

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.

Allow linking directly to Manchester or London listings
3 participants