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

Why set background on android? #17

Open edusperoni opened 6 years ago

edusperoni commented 6 years ago

This is more of a question about the implementation. On Android, the background is set to a GradientDrawable with color and corner (https://github.com/Especializa/nativescript-ng-shadow/blob/master/src/common/shadow.ts#L52). Why is this done?

It seems most of the shadow code is in the elevation bit, and this is serving only to remove the CSS background. Am I missing something?

edusperoni commented 6 years ago

Nevermind, I figured it out. I'm trying to find a way to adapt this plugin in a way that we no longer override the CSS. One way is using the nativeElement property borderRadius, but we can't set specific values for topleft and topright, for example. We also lose the image, if there is any.

Edit:

Ok, got this working:

        var outerRadii = Array.create("float", 8);
        java.util.Arrays.fill(outerRadii, Shadow.androidDipToPx(nativeView, 30)); // test value
        var r = new android.graphics.drawable.shapes.RoundRectShape(outerRadii, null, null);
        var shapeDrawable = new android.graphics.drawable.ShapeDrawable(r);
        shapeDrawable.getPaint().setColor(android.graphics.Color.TRANSPARENT);
        var arr = Array.create(android.graphics.drawable.Drawable, 2);
        arr[0] = shapeDrawable;
        arr[1] = nativeView.getBackground();
        nativeView.setBackgroundDrawable(new android.graphics.drawable.LayerDrawable(arr));

We use a Layer to add the shape to the background while keeping it unaltered. With a RoundRectShape we can then get the CSS border from nativescript to create the correct shape (copied the code I'm using for the ripple plugin I'll be publishing next week):

        outerRadii[0] = outerRadii[1] = this.getIn((<View>this.el.nativeElement).borderTopLeftRadius);
        outerRadii[2] = outerRadii[3] = this.getInPX((<View>this.el.nativeElement).borderTopRightRadius);
        outerRadii[4] = outerRadii[5] = this.getInPX((<View>this.el.nativeElement).borderBottomLeftRadius);
        outerRadii[6] = outerRadii[7] = this.getInPX((<View>this.el.nativeElement).borderBottomRightRadius);

I can confirm this worked for me on Android. I used a background-image and border-radius and the shadow was the right size and shape:

        <StackLayout shadow="10" ripple style="padding:30; border-radius: 30; background-image: url('res://icon');" (tap)="testtap()">
            <Label text="label1"></Label>
            <Label text="label2"></Label>
        </StackLayout>

Do you want me to submit a PR?

JoshDSommer commented 6 years ago

@edusperoni , feel free to make a PR to my fork of this project. https://github.com/TheOriginalJosh/nativescript-ngx-shadow

edusperoni commented 6 years ago

@TheOriginalJosh have you disabled issues on your fork? I was going to open an issue there for discussion but it seems it'll have to be done here

I'm putting some effort on adding this to the plugin without affecting the current functionality. One problem that arose is that changing the background via CSS, for example, completely removes the custom background I set. I have this issue with my own plugin as well (https://github.com/edusperoni/nativescript-ng-ripple).

Unfortunately, there is no way to listen to background changes in {N}, so I've experimentally monkey patched Nativescript's View method _redrawNativeBackground https://github.com/edusperoni/nativescript-ng-ripple/commit/bc39b5fe65801f0a1e01bfab3dc4f8404060b545

This solves the issue, but is very ugly, and may lead to performance problems with animations that change the background. I'm thinking about a similar solution for the nativescript-ng-shadow. What are your thoughts?

NOTE: if you try my plugin, ensure to import it's module AFTER the NgShadow, otherwise the shadow overrides the ripple

JoshDSommer commented 6 years ago

@edusperoni thanks for the heads up I did not notice I had issues disabled.

As for the monkeypatch, I'm not sure, I do think if it was added it could be something turned on when importing the module. That way if performance problems arise, it could be turned off quickly to see if it was possibly the cause. Just a thouht.

edusperoni commented 6 years ago

@TheOriginalJosh yeah, it actually patches the object method, not the class itself, so it's patched when the view is loaded and unpatched when it's unloaded.

Now that I think about it, if we animate the current background color property, we'd have almost the same performance as the monkey patch.

If performance is really an issue, whoever uses the plugin can just add another layout inside the shadow layout and animate it, but I think it's better that we have a plugin that doesn't stop working on a simple background change.

I'll make the PR tomorrow.