search

Especializa / nativescript-ng-shadow

Angular directive to apply shadows to native elements according to the elevation level guidelines of material design specification
Apache License 2.0
54 stars 38 forks source link

Added Android SDK validation (>= 21) #12

Closed mrnkr closed 6 years ago

mrnkr commented 6 years ago

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!

mrnkr commented 6 years ago

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 commented 6 years ago

Oh sorry for that. 6e3745508bc1256137c590a47ac1675dc1aab189 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 commented 6 years ago

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