feat(android): elevation shadow support - #7136

As requested by @manoldonev (#7055 (comment))

This is a simple way to support elevation + statelistanimator on android

For some reason, setElevation and some other functions throw TS errors (it seems to be using API 17 declaration).

TODO

1. check API version >=21 (running this on API <21 will result in a crash without it)
2. remove any casts
3. implement iOS version (some views set clipsToBounds, which makes the shadow disappear. nativescript-ngx-shadow solves this by wrapping the view in a StackLayout)
4. I'm using android.R which adds over 600ms of lag on the first run of the code Running some native code for the first time results in application lag android#1330. We should consider an workaround.

@manoldonev I've switched the properties to the view. Unfortunately, when we unset the properties, they are never unset from the view (they never return to undefined). Is there any way to force them to return to undefined when unset?

About unsetting local values -- seems like this is not implemented at the moment:

Will have to think about it a bit.

My naive implementation worked:

            if (!(key in newPropertyValues)) {
                if (key in this.view.style) {
                    this.view.style[`css:${key}`] = unsetValue;
                } else {
                    // TRICKY: How do we unset local value?
                    const camelCasedProperty = key.replace(/-([a-z])/g, function (g) { return g[1].toUpperCase(); });
                    this.view[camelCasedProperty] = unsetValue;
                }
            }

if the property has been defined as a Property, this should work. This won't work when it's not a Property, though. If there's a way to check that, we should have a solution for properties with default values, for example.

Edit:

I've been experimenting with this and there doesn't seem to be a reliable way to check if the property is actually a Property. So maybe we have to keep a private property array and, if the name is in the array, treat it as a property. I believe that this is more efficient than iterating through all properties, comparing with camelCasedProperty and finally checking it with isSet().

Hey @edusperoni

I've been brainstorming a bit on the proposed elevation support implementation and after discussions with the team here I've made a commit with the following propositions:

• rename the public APIs to designate that currently we provide Android only functionality (i.e. androidElevation and androidDynamicElevationOffset); when iOS equivalent is implemented we will deprecate the android specific properties and introduce common elevation and dynamicElevationOffset properties for both platforms.
• drop the undefined / null convention for unset androidElevation / androidDynamicElevationOffset (as it is really confusing to the end user) and provide the underlying android values instead
• restore your original implementation for androidElevation / androidDynamicElevationOffset to be CssProperties (vs view Properties). We will not be trying to patch the missing functionality for unsetting local values for view properties as part of this PR.
• Drop the caching of the original stateListAnimator as part of the refreshStateListAnimator() method

test

@manoldonev Sounds good! But won't removing the null/undefined checks actually cause an issue with plugins like nativescript-floatingactionbutton and nativescript-cardview, that actually have custom elevations set by other means, or have non-defaults dynamicElevationOffest (since we're no longer caching the original animator, which was similar to what we do with android background).

test ios#rc

@edusperoni do you have a specific scenario in mind? If one is not using the new androidElevation / androidDynamicElevationOffset APIs explicitly we are not tampering with the original animators so those plugins should be working as expected.

@manoldonev Would you consider an iOS implementation if I would provide a basic PoC based on the current state of this repo? I could use the androidElevation property on iOS just to be able to discuss its implementation. We can always remove it if it's not satisfactory.

@EddyVerbruggen sure, Eddy -- you can open a separate PR as we are trying to merge this for NativeScript 5.4 release candidate now. As I see it, this whole effort should ultimately provide the solution you described here #550 (comment).

We needed the android implementation earlier as it turned out #7055 (comment) introduced a regression for outline buttons. With NativeScript 5.4 we will be releasing a patch for nativescript-theme-core that will explicitly set androidElevation to zero for such buttons thus keeping their old and expected behavior.

test ios#rc

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.