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 auto-complete to discovery country selection #6139

Merged
merged 14 commits into from Oct 15, 2022

Conversation

Pinkolik
Copy link
Contributor

@Pinkolik Pinkolik commented Oct 9, 2022

Hello!
This closes #6098.
Here's a demonstration video
https://imgur.com/a/4avAwIV

MaterialAutoCompleteTextView editText = (MaterialAutoCompleteTextView) textInput.getEditText();
editText.setAdapter(dataAdapter);
if (countryCode.equals(ItunesTopListLoader.DISCOVER_HIDE_FAKE_COUNTRY_CODE)) {
editText.setText(countryCodeNames.get(previousCountryCode));
Copy link
Member

Choose a reason for hiding this comment

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

Why do you store the previous code? It's only in a variable anyway, so it doesn't really help?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I use this variable in order to restore country user previously selected when they uncheck "Hide" checkbox, and to show previously selected country in drop-down list instead of empty string

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also made this variable stored in prefs

Copy link
Member

Choose a reason for hiding this comment

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

Instead of storing (current, previous) country codes where the current one can be DISCOVER_HIDE_FAKE_COUNTRY_CODE, I think it would be a lot more clear to store (country, hidden). Then we need to store one string and one boolean and don't need to move strings back and forth between the two variables.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

app/src/main/res/layout/fragment_itunes_search.xml Outdated Show resolved Hide resolved
app/src/main/res/layout/list_item.xml Outdated Show resolved Hide resolved
app/src/main/res/layout/select_country_dialog.xml Outdated Show resolved Hide resolved
app/src/main/res/menu/countries_menu.xml Outdated Show resolved Hide resolved
@Pinkolik
Copy link
Contributor Author

Pinkolik commented Oct 9, 2022

Hello!
Thanks for your review.
I've made changes accordingly, you can check them now :)

@Pinkolik
Copy link
Contributor Author

Hello!
Any updates on this? Does it require any other changes?

@ByteHamster ByteHamster changed the title issue-6098 add discover countries autocomplete text Add auto-complete to discovery country selection Oct 14, 2022
Copy link
Member

@ByteHamster ByteHamster left a comment

Choose a reason for hiding this comment

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

Thanks! This looks so much better than the old country selector 👍 I added some additional comments :)

@ByteHamster
Copy link
Member

Can't DISCOVER_HIDE_FAKE_COUNTRY_CODE be removed now?

@Pinkolik
Copy link
Contributor Author

Can't DISCOVER_HIDE_FAKE_COUNTRY_CODE be removed now?

Good point! Pushed new commit

@ByteHamster ByteHamster merged commit 4c30d8f into AntennaPod:develop Oct 15, 2022
@ByteHamster
Copy link
Member

Thanks! Will be released in version 3.0

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.

Replace Discover > Country selection dropdown with overflow menu
2 participants