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

A couple of fixes, update API target to Android 4.4 #25

Merged
merged 7 commits into from Nov 22, 2013
Merged

Conversation

rock3r
Copy link
Contributor

@rock3r rock3r commented Nov 12, 2013

This is the full list of changes:

  • Removed the + from the layout files' id fields (they're already defined, you shouldn't have the plus in there)
  • Set target API level to 19 (Android 4.4 KitKat) in the library manifest and in the Gradle build file (for both targetSdkVersion and compileSdkVersion)
  • Tweaks to the Gradle settings: use the newer 0.6.x Android plugin and version 19 build tools; declare the defaultConfig to match what is declared in the Manifest
  • Tweaked a bit the Switch class code: now it uses the original API calls that were substituted in the backporting process if running on a platform version that supports them; removed some code in onMeasure that wasn't actually used for anything

The Gradle changes should be fine as they're pretty basic but I haven't had the chance to test them as much as I'd like. I am not confident with Maven, so that will need someone else to work on it.

@@ -28,6 +28,6 @@
android:versionCode="1"
android:versionName="1.0" >

<uses-sdk android:minSdkVersion="7" />
<uses-sdk android:minSdkVersion="7" android:targetSdkVersion="17" />
Copy link
Owner

Choose a reason for hiding this comment

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

I'm not sure this attribute is useful for a library, but in any case I guess you meant 19?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, I've missed that. Shouldn't make a big difference here anyway, as you're pointing out :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you want me to fix it or are you going to do it if/when you merge?

Copy link
Owner

Choose a reason for hiding this comment

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

I would prefer if you could fix it :) Thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, will do today :)

@BoD
Copy link
Owner

BoD commented Nov 21, 2013

Thank you for this! Other than the little remark above, I would gladly merge the changes.

@rock3r
Copy link
Contributor Author

rock3r commented Nov 22, 2013

Fixed the Manifest typo :)

@rock3r
Copy link
Contributor Author

rock3r commented Nov 22, 2013

Oh, missing from the OP but in the commits, in this pull request I've also fixed the switch text color (which would show up as black on light themes, and now is always the right light grey shade) and included KitKat's XXHDPI drawables. Since those drawables are the same from API 16 onwards, instead of having them in a drawables-xxhdpi-v19, I've put them in the drawables-xxhdpi-v16 folder, so that all XXHDPI devices running 4.1+ would show them.

BoD added a commit that referenced this pull request Nov 22, 2013
A couple of fixes, update API target to Android 4.4
@BoD BoD merged commit ac76607 into BoD:master Nov 22, 2013
@BoD
Copy link
Owner

BoD commented Nov 22, 2013

Thanks a lot for this high quality contribution :)

@rock3r
Copy link
Contributor Author

rock3r commented Nov 22, 2013

You're welcome! Thanks for sharing the library :)

@BoD
Copy link
Owner

BoD commented Feb 8, 2014

Actually, the text color for the light theme was incorrect! No worries, I just fixed it in the 1.3 release :)

@rock3r
Copy link
Contributor Author

rock3r commented Feb 8, 2014

I'm sorry, totally missed it...

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