feat(android): implement BorderDrawable outline - #7055

@manoldonev It seems there was a failing test (NativeScript/tns-core-modules-widgets#158 (comment)), but allow me to elaborate:

Currently, on Android, Button has an elevation. This elevation is not respected by BorderDrawable because it doesn't provide an outline so no shadow is cast.

This is the exact reason this PR was created, so any BorderDrawable can cast shadow. Once @EddyVerbruggen and I properly finish the elevation property for all views, the developer will be able to specify elevations for views.

Currently, adding borders do buttons gets rid of Ripple (because it resets the background) and Shadow (because the outline is not provided).

I believe we might have to discuss how a button should behave. If the current implementation is an issue, I suggest we change the Button's elevation to 0 to keep things consistent

@edusperoni thank you for elaborating on this. I think we are good with this change and will update our ui tests next week to accept it.

@manoldonev Awesome! Thanks!

We'll open a PR to support elevation (via CSS too) some time in the future

test

oh man, I hope this comes out soon

@edusperoni do you have an estimate when you'll be able to add the elevation support?

We have a specific scenario with outline buttons that clearly doesn't benefit from the BorderDrawable outline support:
Actual:

Expected:

One can restore the old behavior (no shadow in this scenario) by setting button.nativeViewProtected.setStateListAnimator(null); but that's not a good solution for public API and I don't think we'd like to release a minor / major NativeScript version like this.

My understanding is that with elevation support (your future PR), users will be able to set button.elevation = 0;, or <Button elevation="0" ... />, or set elevation=0 in css theme (button-outline class) to achieve the outline button scenario above -- is this correct?

With this in mind I see the following options if elevation support will not be added in the immediate future:

• Revert this PR and merge it again together with the elevation support PR when ready (ensuring that outline button scenario can be achieved)
• Add property that disables the BorderDrawable outline functionality by default

What do you think?

I could make a quick patch for elevation which would be android only, iOS support coming after. You can also switch to a Label for that case, since it has no elevation by default and also no statelistanimator.

So we can better determine the elevation properties, I invite you to check this repo https://github.com/JoshDSommer/nativescript-ngx-shadow#list-of-attributes

And this PR: https://github.com/JoshDSommer/nativescript-ngx-shadow/pull/19/files

Currently, all that code is done natively. My PR in that repo is a mimic of this feature (and also compatible with it).

While we can set the elevation, the statelistanimator is a whole other issue. Should we always override the statelistanimator? Just for views that have an animator? Or should we expose extra android-only properties for this?

Or maybe we just set the elevation for the view and set statelistanimator to null, allowing the user to use external plugins if they want to extend the default behavior?

If we can't find a good solution now, we can just set the Outline alpha to 0 to disable it (bound to a property) for the time being

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.