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

Rewrite search filter sidebar with React #296

Merged
merged 2 commits into from May 8, 2020

Conversation

cmfcmf
Copy link
Member

@cmfcmf cmfcmf commented May 5, 2020

I rewrote the search filters with React to more easily incorporate the following improvements:

  • An "apply" button appears after changing a filter.
  • Pressing ENTER inside an integer min/max field applies the filter
  • Long filter lists now scroll when extended
  • Other minor styling updates

The rewrite should also make it easier for a future PR to no longer load the huge list of options when the page is initially rendered and instead only fetch the big lists (e.g., list of notable items) in batches via AJAX.

This PR also showcases how modern JS like React can be integrated into AdventureLookup without much hassle.

@Wigginns
Copy link
Collaborator

Wigginns commented May 6, 2020

Love this. React is cool stuff and this is a good step forward for the app imo.

What are your thoughts on approval processes? I usually tend to check out a branch and at least run it locally for a sanity check first but with the testing environment setup and CI/CD working again maybe that's overkill.

@cmfcmf
Copy link
Member Author

cmfcmf commented May 7, 2020

Generally speaking I prefer to checkout PRs and test them locally, just to make sure they really work (especially because the tests don't cover that much). But I also know that it is always a hassle to do that. Maybe I'll find the time to setup Heroku Review Apps for AdventureLookup. That would make it easier to sanity check PRs, because you'd simply have to click a link to see a deployed version of the PR.

Copy link
Collaborator

@Wigginns Wigginns left a comment

Choose a reason for hiding this comment

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

Looks good. works great for me. Feels a it snappier. Small style thing for readability's sake but that's up to you.

app/Resources/views/adventures/index.html.twig Outdated Show resolved Hide resolved
Copy link
Collaborator

@Wigginns Wigginns left a comment

Choose a reason for hiding this comment

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

Approved. I meant to approve and leave a comment last time but ticked the wrong box

@cmfcmf cmfcmf merged commit 6efd9e4 into AdventureLookup:dev May 8, 2020
@cmfcmf cmfcmf deleted the rewrite-filters branch May 8, 2020 17:57
Wigginns added a commit to Wigginns/AdventureLookup that referenced this pull request May 11, 2020
Rewrite search filter sidebar with React (AdventureLookup#296)
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.

None yet

2 participants