React-native-windows: button click from react-native-paper lib crashes on windows

[capc0]

Environment

Run the following in your terminal and copy the results here.

1. npx react-native --version: 4.13.0
2. npx react-native info:

System:
    OS: Windows 10 10.0.18362
    CPU: (16) x64 AMD Ryzen 7 1700 Eight-Core Processor
    Memory: 9.18 GB / 15.94 GB
  Binaries:
    Node: 12.13.0 - C:\Program Files\nodejs\node.EXE
    Yarn: Not Found
    npm: 6.14.2 - C:\Program Files\nodejs\npm.CMD
    Watchman: Not Found
  SDKs:
    Android SDK:
      API Levels: 25, 29
      Build Tools: 25.0.3, 26.0.1, 28.0.3, 29.0.2
      System Images: a...google_apis | Google APIs Intel x86 Atom Sys..., a... | Intel x86 Atom_64, a...google_apis | Google APIs Intel x86 Atom Sys...
      Android NDK: Not Found
    Windows SDK:
      AllowDevelopmentWithoutDevLicense: Enabled
      AllowAllTrustedApps: Enabled
      Versions: 10.0.15063.0, 10.0.16299.0, 10.0.18362.0
  IDEs:
    Android Studio: Version  2.3.0.0 AI-162.4069837
    Visual Studio: 15.4.27004.2002 (Visual Studio Enterprise 2017), 16.7.30621.155 (Visual Studio Community 2019)
  Languages:
    Java: 1.8.0_262 - C:\Program Files\OpenJDK\openjdk-8u262-b10\bin\javac.EXE
    Python: 2.7.18 - C:\Python27\python.EXE
  npmPackages:
    @react-native-community/cli: Not Found
    react: 16.13.1 => 16.13.1
    react-native: 0.63.3 => 0.63.3
    react-native-windows: ^0.63.0-0 => 0.63.5
  npmGlobalPackages:
    *react-native*: Not Found

Steps To Reproduce

Provide a detailed list of steps that reproduce the issue.

1. steps desribed in https://github.com/callstack/react-native-paper/issues/2329

Expected Results

it should not crash ;)

other

im not sure if this is something to be fixed in the windows platform or the rn-paper lib, thats why im cross-posting here. If it is out of scope feel free to close this issue.

[asklar]

@capc0 can you please provide the call stack at the time of the crash? Also, is this a hard crash or do you get a redbox?

[creambyemute]

Ugh, I'm soon going to add RN-Windows to our App again (we postponed it when vNext was announced) and we're using RN-Paper as well.

I remember before vNext, RN-Paper with with version 2.16.x worked, at least there the button didn't crash.
Not sure if 2.16 of RN-Paper would still work with RN-Windows vNext.

[capc0]

its a hard crash.

[asklar]

Thanks @capc0 . It looks like either the Paper button or your app code is trying to animate the elevation property, which only exists on Android. We arguably shouldn't crash, but it sounds like Paper/your app should be opting out of the elevation animation on non-Android platforms.

[capc0]

I checked the rn-paper button code and found exactly what you said in
https://github.com/callstack/react-native-paper/blob/master/src/components/Button.tsx#L146

I will report it to the rn-paper team.

You want to keep this issue here open and prevent calls to the elevation prop - or should I close it?

Thanks!

[creambyemute]

We are using RN-Paper on iOS and Android and iOS does not crash when doing that, just fyi. (Elevation does not exist on iOS either, there it would be shadow*)

[capc0]

I also think it would be handy if the rn-windows platform would prevent/ignore calls to the elevation API. This avoids propably many code changes needed in different UI libraries.

[chrisglein]

At least to match iOS it seems like we should not crash. Catching this earlier than when we've already spun up the animations and are deep in facades would be good. Perhaps a yellow box, as technically you're trying to animate something that's going to no-op? I don't know if that's considered a graceful decline in behavior or not. Windows does have a notion of elevation that feeds into its shadow behavior but I don't know if it's a good match for Android elevation.

tldr: Let's have Windows not crash.

[creambyemute]

RN-Paper V2.16 does not crash (we are using that one at the moment).
RN-Paper V4+ does crash as reported above because of the elevation prop animation.

RN-Paper Buttons on V2.16 as well as V4+ seem to have problems with RN-Windows in general:
RN-Paper Button component does not accept clicks outside of the text, only the text is clickable/pressable on RN-Windows, not the rest of the button.

My temporary/hacky solution on V2.16 without having to rewrite a lot of stuff to get the Buttons to accept clicks:

export const Button = React.forwardRef((props: ButtonProps, ref: React.LegacyRef<RNPaperButton>) => {
    if (Platform.OS === "windows") {
        return (
            <TouchableOpacity {...props} activeOpacity={0.8}>
                <RNPaperButton ref={ref} {...props} style={{}} onPress={undefined}>
                    {props.children}
                </RNPaperButton>
            </TouchableOpacity>
        );
    }
    return (
        <RNPaperButton ref={ref} {...props}>
            {props.children}
        </RNPaperButton>
    );
});

So on RN-Windows I wrap the RN-Paper Button within a TouchableOpacity.

The same applies to TouchableRipple from RN-Paper where u can do smth like this for the moment:

/**
 * Defaults to TouchableOpacity on Windows and TouchableRipple on Android/iOS
 */
export const TouchableRipple = React.forwardRef(
    (
        props:
            | (TouchableRippleProps & TouchableRippleCustomProps)
            | (TouchableOpacityProps & TouchableRippleCustomProps)
            | (TouchableHighlightProps & TouchableRippleCustomProps),
        ref: React.LegacyRef<PaperTouchableRipple | TouchableOpacity | TouchableHighlight>
    ) => {
        if (props.touchableHighlight) {
            return (
                <TouchableHighlight ref={ref} {...props}>
                    {props.children}
                </TouchableHighlight>
            );
        }
        if (props.touchableOpacity) {
            return (
                <TouchableOpacity ref={ref} {...props}>
                    {props.children}
                </TouchableOpacity>
            );
        }
        if (Platform.OS === "windows") {
            return (
                <TouchableOpacity ref={ref} {...props}>
                    {props.children}
                </TouchableOpacity>
            );
        } else {
            return (
                <PaperTouchableRipple ref={ref} {...props}>
                    {props.children}
                </PaperTouchableRipple>
            );
        }
    }
);

The ActivityIndicator does also crash RN-Windows, easy workaround:

export const ActivityIndicator = (props: ActivityIndicatorProps) => {
    if (Platform.OS === "windows") {
        return <RNActivityIndicator {...props} color={Theme.colors.primary} />;
    }
    return <PaperActivityIndicator {...props} />;
};

If anyone tries out RN-Paper with RN-Windows these changes should make it work flawlessly.

Edit: The Modal of RN-Paper works :fire: