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

filter trade area by country #176

Merged
merged 6 commits into from
Dec 8, 2023

Conversation

JonPurvis
Copy link
Contributor

@JonPurvis JonPurvis commented Jul 30, 2023

A lot of the time when browsing the trade area, I'm interested in seeing Elephpants for trade in the UK, currently the only way to do that is go through each page and carefully look for users that are marked as being in the UK.

This PR adds the ability to filter the trade area by country, so a user can easily filter trades by their country or a specific country. I've also added a handy button to select your own country, instead of finding it in the dropdown.

image

I've also amended the pagination so that it keeps the country in the URL when you switch pages.

@cookieguru
Copy link
Contributor

Just looking at the screenshot, it's not clear that clicking "My Country" is going to change the value of the dropdown: a) the verbiage isn't a call to action and b) it's using the success color and not the color of something that would make the UI perform an action

What if instead the user's country was always the first one in the list?

Alternatively, is there a use case for filtering on a country other than the user's own?

@JonPurvis
Copy link
Contributor Author

Great points @cookieguru

I've swapped the button to use the secondary CSS class, which I think makes sense given the other button is primary. I've also altered the text so it's clear what clicking it will do:

image

I think a good use case for filtering by a specific country, instead of just your own is if you're planning on attending a conference/event outside of your country and you want to try and arrange some trades ahead of time. You could narrow it by country and then check to see if any of the remaining traders will also be at the conference/event. For me, I'd currently have to go through 15 pages worth of trades whereas if I was attending a conference in the UK, I'd perhaps narrow it down to show UK only traders which is only 1 or 2 pages.

@teiling88
Copy link
Collaborator

@JonPurvis can you please rebase this branch? So we can merge it after the final test? :-)

@JonPurvis
Copy link
Contributor Author

@teiling88 done! Feel free to give it a test 😄

@teiling88
Copy link
Collaborator

@JonPurvis sorry - can you please rebase your branch one last time please? :-)

@JonPurvis
Copy link
Contributor Author

@teiling88 this one should be good now too! 😄

@teiling88
Copy link
Collaborator

Will test it local the next days and merge it afterwards :-)

@JonPurvis
Copy link
Contributor Author

Excellent, thank you!

Would you be able to give the following 2 PRs a test too? 😄

Thanks!

@teiling88 teiling88 merged commit c55494c into jgrossi:master Dec 8, 2023
1 check passed
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

3 participants