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

Add routing and split out home from feed #17

Merged
merged 5 commits into from
Sep 23, 2024
Merged

Add routing and split out home from feed #17

merged 5 commits into from
Sep 23, 2024

Conversation

bryophyta
Copy link
Contributor

@bryophyta bryophyta commented Sep 18, 2024

What does this change?

Preliminary Refactors

Main feature

How to test

  • visiting / shows the new landing page
  • visiting /feed shows all wire results
  • visiting /feed?q=ipsum loads filtered wire results
  • searching from landing page (press enter to search) navigates to /feed?q=[your search terms]
  • search from the Feed page nav bar is debounced, and search terms are written to the URL search params

How can we measure success?

Have we considered potential risks?

Images

Landing page

image

Search terms are bound to the URL

image

Accessibility

@bryophyta bryophyta marked this pull request as ready for review September 18, 2024 11:15
@bryophyta bryophyta requested a review from a team as a code owner September 18, 2024 11:15
Copy link
Member

@andrew-nowak andrew-nowak left a comment

Choose a reason for hiding this comment

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

looks to be a good incremental step forward! 👏

Copy link
Member

Choose a reason for hiding this comment

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

nice to see some familiar code! did we consider using a router library? I imagine it'll be easier to build one in from the start, than to migrate later

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nice to see some familiar code!

👍👍

did we consider using a router library? I imagine it'll be easier to build one in from the start, than to migrate later

we had a couple of chats about it, but no super concrete outcome so I thought it made sense to move forward with this for now as it's fairly narrowly scoped. fwiw judging from the spike I did with tanstack-router for crosswordv2 I don't think that migrating to a library solution later should be too painful.

but happy to use one here if we're feeling confident that we'll want one at some point

@bryophyta bryophyta merged commit 12d361a into main Sep 23, 2024
3 checks passed
@bryophyta bryophyta deleted the pf/url-state branch September 23, 2024 11:29
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