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

Added Android SDK validation (>= 21) #12

Merged
merged 1 commit into from
Dec 29, 2017
Merged

Conversation

mrnkr
Copy link

@mrnkr mrnkr commented Dec 26, 2017

Hey!

So, as the title says, I added the validation for the Android SDK to have the setElevation method available and added a quick heads up to whoever reads the README that shadows are not shown on any version of Android older than Lollipop.

Since design didn't rely that much on shadows back then (as far as I can remember...) don't think it's a big deal, if it were I guess I could look for an alternative implementation that's compatible.

I tested my changes on Android 4.4 and made sure no unexpected behaviors were found by testing on Android 8.1 and iOS 11.2.

Hope you find this useful!

Copy link
Contributor

@berardo berardo left a comment

Choose a reason for hiding this comment

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

Thanks for looking into that, Alvaro

@berardo berardo merged commit d94ab77 into Especializa:master Dec 29, 2017
@mrnkr
Copy link
Author

mrnkr commented Jan 3, 2018

Hey there!

Saw you changed my code a little bit when you moved it... Although your version looks better it has a problem...
Running this code:

android.os.Build.VERSION.SDK_INT >= android.os.Build.VERSION_CODES.LOLLIPOP

on a phone that is running Android 4.4 results in the following exception:

JS: ERROR Error: java.lang.NoSuchFieldError: no static field with name='LOLLIPOP' signature='I' in class Landroid/os/Build$VERSION_CODES;
JS:     com.tns.Runtime.callJSMethodNative(Native Method)
JS:     com.tns.Runtime.dispatchCallJSMethodNative(Runtime.java:1088)
JS:     com.tns.Runtime.callJSMethodImpl(Runtime.java:970)
JS:     com.tns.Runtime.callJSMethod(Runtime.java:957)
JS:     com.tns.Runtime.callJSMethod(Runtime.java:941)
JS:     com.tns.Runtime.callJSMethod(Runtime.java:933)
JS:     com.tns.gen.org.nativescript.widgets.Async_CompleteCallback.onComplete(Async_CompleteCallback.java:12)
JS:     org.nativescript.widgets.Async$Http$HttpRequestTask.onPostExecute(Async.java:585)
JS:     org.nativescript.widgets.Async$Http$1$1.run(Async.java:486)
JS:     android.os.Handler.handleCallback(Handler.java:733)
JS:     android.os.Handler.dispatchMessage(Handler.java:95)
JS:     android.os.Looper.loop(Looper.java:136)
JS:     android.app.ActivityThread.main(ActivityThread.java:5584)
JS:     java.lang.reflect.Method.invokeNative(Native Method)
JS:     java.lang.reflect.Method.invoke(Method.java:515)
JS:     com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1268)
JS:     com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1084)
JS:     dalvik.system.NativeStart.main(Native Method)

For that, instead of using android.os.Build.VERSION_CODES.LOLLIPOP it is better to use just the value it returns (21).

The second I can I'll submit a PR with a quick fix unless you beat me to it!
Have a good one!

berardo added a commit that referenced this pull request Jan 6, 2018
@berardo
Copy link
Contributor

berardo commented Jan 6, 2018

Oh sorry for that.
6e37455 should fix that.
I'm not able to test it though, would you mind checking that for me so I can bump the version?

Thanks a lot

@mrnkr
Copy link
Author

mrnkr commented Jan 9, 2018

Just checked 6e37455 and it is perfect, no issues :)

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