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

Fix #963 #965

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix #963 #965

wants to merge 1 commit into from

Conversation

wchen342
Copy link
Contributor

This is a fix for #963. The back button doesn't have a default listener before O..
Also the back behavior between N and O is very different. Actually it is quite a mess..So just asking, is it possible to rise minSdkVersion?

@@ -101,6 +100,16 @@ public void onSharedPreferenceChanged (SharedPreferences sharedPreferences, Stri
}
}

@Override
public boolean onOptionsItemSelected(MenuItem item) {
Copy link
Member

Choose a reason for hiding this comment

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

This is the fix for the back button, right?

Why do we need the onStart overrides?

Copy link
Contributor Author

@wchen342 wchen342 Jun 23, 2019

Choose a reason for hiding this comment

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

That is related to the old problem with the back behavior #935. I found the previous fixes only work for O, but not below N it will display a blank page instead of going back immediately.

@adrian-bl
Copy link
Member

We still want to support Android 4.4 devices, so i'd rather keep minSdkVersion.

Maybe it would be simpler to switch the whole thing over to AppCompatPreferenceActivity ?

@wchen342
Copy link
Contributor Author

I think add AppCompat package will add a lot of extra dependencies? Also if supporting of old versions are important than support libraries should be used everywhere otherwise function calls will be very inconsistent. A better idea is probably doing an overhaul on a seperately branch with 9 support?

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