Repository: callstack/react-native-paper
Pull request #785 (2019-01-30 07:14:07)

fix: consistent elevation behavior across platforms
<!-- Please provide enough information so that others can review your pull request. -->
<!-- Keep pull requests small and focused on a single change. -->

### Motivation

This PR introduces a consistent behavior across platforms for elevations. It seems that on web we need to animate width and height in order to trigger the animation correctly. However, when I did that with the existing interpolations the shadow was pretty dark. So I changed the height to 0 as well and in my opinion it still looks good on native. 

### Test plan

Open example app on web and press "Pressable Chameleon" card. Before this PR, the card elevation does not appear. After this PR, the card elevation does appear and is animated.

#### Web:
Unpressed:
![unpressed](https://user-images.githubusercontent.com/5358638/54359404-d5d8b800-4662-11e9-9ac7-fa33fc8cd6ac.png)

Pressed:
![pressed](https://user-images.githubusercontent.com/5358638/54359583-41228a00-4663-11e9-920b-1892d6695a2a.png)

#### iOS:
Unpressed:
![Simulator Screen Shot - iPhone 8 - 2019-03-14 at 14 02 56](https://user-images.githubusercontent.com/5358638/54358973-e76d9000-4661-11e9-86fe-9de8ae181ac1.png)

Pressed:
![Simulator Screen Shot - iPhone 8 - 2019-03-14 at 14 03 05](https://user-images.githubusercontent.com/5358638/54358978-e9375380-4661-11e9-8c11-e78f75d80917.png)


jaulz: @satya164 could you please check if the look on native is fine for you? If you confirm it should fix https://github.com/callstack/react-native-paper/issues/782 😊 If not, we could think about setting height to 0 on web and keep the existing interpolation on native.
satya164: cc @Trancever 
Trancever: Hey @jaulz It looks good to me on native, do you have any reference how it should look like on web? I couldn't find any good example on material.io..
jaulz: @Trancever I don't think there is any specification for web but instead it should be the same on native and web but without this PR there is no shadow at all.
Trancever: @satya164 Any comments on this? For me this shadow on web is a bit too sharp, but if there is nothing we can do, I am ok with merging it.
ferrannp: Is this PR not affecting android and iOS? Are they still working the same?
callstack-bot: Hey @jaulz, thank you for your pull request 🤗. The documentation from this branch can be viewed [here](https://8950-71323749-gh.circle-artifacts.com/0/docs/index.html). Please remember to update Typescript types if you changed API.
jaulz: @ferrannp I just pushed another commit which should make it absolutely equal on web and native. Please have a look at the screenshots I just took and added to the original text. And indeed @Trancever was right that the shadow on web was too sharp and the issue was that the opacity and color were not respected as long as they were plain numbers and no `Animated.Value`s. 
Trancever: @satya164 Can we merge this?
Trancever: @jaulz I am not sure why but when I am on this branch I can't see Ripple effect, both on web and mobile. Could you check it?
jaulz: @Trancever I think it's because the ripple effect for cards was only added to 3.0 in https://github.com/callstack/react-native-paper/pull/983 and this PR is for master
Trancever: @jaulz I am talking about whole app, every component :D Even `TouchableRipple` behaves like `TouchableHighlight` on Android when I am on this branch. I am not sure if something is wrong with this PR or it's just my device... Will check that later on different Android. 
jaulz: @Trancever a rebase fixed it 😊 
jaulz: Can we merge this anytime soon? 
Trancever: @jaulz I'll try to check once again this PR and merge it today
Trancever: @jaulz There are some issues with native animations. Example app crashed when I tried to use dialog or menu. 
jaulz: Strange. Can you tell me how to reproduce it? I will have a look next week once I am back from vacation. 
Trancever: Sure, Just checkout this branch, run example app on iOS or Android and try to open dialog or menu. Thanks for taking care of it!
jaulz: @Trancever fix is in... could you please check again? Still there is a warning:
```
Failed prop type: Invalid prop `shadowOffset.width` of type `object` supplied to `SafeAreaView`, expected `number`.
```

I'm not exactly sure why because the docs clearly state the it should be an object: https://facebook.github.io/react-native/docs/shadow-props#shadowoffset
Trancever: @jaulz Thanks for your patience :D We're shipping it finally!

Patch:
diff --git a/src/components/Surface.js b/src/components/Surface.js
index a83eed38e..7cb7fcf5d 100644
--- a/src/components/Surface.js
+++ b/src/components/Surface.js
@@ -65,7 +65,7 @@ class Surface extends React.Component<Props> {
         {...rest}
         style={[
           { backgroundColor: theme.colors.surface },
-          elevation && shadow(elevation),
+          shadow(elevation),
           style,
         ]}
       />
diff --git a/src/styles/shadow.js b/src/styles/shadow.js
index ef0c31687..d7d115739 100644
--- a/src/styles/shadow.js
+++ b/src/styles/shadow.js
@@ -1,44 +1,44 @@
 /* @flow */
 
-import * as Colors from './colors';
 import { Animated } from 'react-native';
+import * as Colors from '../styles/colors';
 
-export default function shadow(elevation: number | Animated.Value) {
-  let height, radius;
+type Shadow = {
+  shadowColor: string,
+  shadowOffset: {
+    width: Animated.Value,
+    height: Animated.Interpolation,
+  },
+  shadowOpacity: Animated.Value,
+  shadowRadius: Animated.Interpolation,
+};
 
-  if (elevation instanceof Animated.Value) {
-    height = elevation.interpolate({
-      inputRange: [0, 1, 2, 3, 8, 24],
-      outputRange: [0, 0.5, 0.75, 2, 7, 23],
-    });
+export default function shadow(elevation: ?number | Animated.Value): Shadow {
+  const animatedElevation: Animated.Value =
+    elevation instanceof Animated.Value
+      ? elevation
+      : new Animated.Value(elevation || 0);
+  const inputRange = [0, 1, 2, 3, 8, 24];
 
-    radius = elevation.interpolate({
-      inputRange: [0, 1, 2, 3, 8, 24],
-      outputRange: [0, 0.75, 1.5, 3, 8, 24],
-    });
-  } else {
-    switch (elevation) {
-      case 1:
-        height = 0.5;
-        radius = 0.75;
-        break;
-      case 2:
-        height = 0.75;
-        radius = 1.5;
-        break;
-      default:
-        height = elevation - 1;
-        radius = elevation;
-    }
-  }
+  // Important: we need to return Animated values to ensure they are properly used in the Animated.View
+  const shadowOpacity = new Animated.Value(0.24);
+  const width = new Animated.Value(0);
+  const height = animatedElevation.interpolate({
+    inputRange,
+    outputRange: [0, 0.5, 0.75, 2, 7, 23],
+  });
+  const shadowRadius = animatedElevation.interpolate({
+    inputRange,
+    outputRange: [0, 0.75, 1.5, 3, 8, 24],
+  });
 
   return {
     shadowColor: Colors.black,
     shadowOffset: {
-      width: 0,
+      width,
       height,
     },
-    shadowOpacity: 0.24,
-    shadowRadius: radius,
+    shadowOpacity,
+    shadowRadius,
   };
 }


Test patch:
diff --git a/src/components/__tests__/__snapshots__/Banner.test.js.snap b/src/components/__tests__/__snapshots__/Banner.test.js.snap
index 50efb250a..f1255747d 100644
--- a/src/components/__tests__/__snapshots__/Banner.test.js.snap
+++ b/src/components/__tests__/__snapshots__/Banner.test.js.snap
@@ -195,6 +195,13 @@ exports[`renders visible banner, with action buttons and with image 1`] = `
               "elevation": 0,
               "margin": 4,
               "minWidth": "auto",
+              "shadowColor": "#000000",
+              "shadowOffset": Object {
+                "height": 0,
+                "width": 0,
+              },
+              "shadowOpacity": 0.24,
+              "shadowRadius": 0,
             }
           }
         >
@@ -357,6 +364,13 @@ exports[`renders visible banner, with action buttons and without image 1`] = `
               "elevation": 0,
               "margin": 4,
               "minWidth": "auto",
+              "shadowColor": "#000000",
+              "shadowOffset": Object {
+                "height": 0,
+                "width": 0,
+              },
+              "shadowOpacity": 0.24,
+              "shadowRadius": 0,
             }
           }
         >
@@ -440,6 +454,13 @@ exports[`renders visible banner, with action buttons and without image 1`] = `
               "elevation": 0,
               "margin": 4,
               "minWidth": "auto",
+              "shadowColor": "#000000",
+              "shadowOffset": Object {
+                "height": 0,
+                "width": 0,
+              },
+              "shadowOpacity": 0.24,
+              "shadowRadius": 0,
             }
           }
         >
diff --git a/src/components/__tests__/__snapshots__/Button.test.js.snap b/src/components/__tests__/__snapshots__/Button.test.js.snap
index 7072a3874..c695c0738 100644
--- a/src/components/__tests__/__snapshots__/Button.test.js.snap
+++ b/src/components/__tests__/__snapshots__/Button.test.js.snap
@@ -11,6 +11,13 @@ exports[`renders button with color 1`] = `
       "borderWidth": 0,
       "elevation": 0,
       "minWidth": 64,
+      "shadowColor": "#000000",
+      "shadowOffset": Object {
+        "height": 0,
+        "width": 0,
+      },
+      "shadowOpacity": 0.24,
+      "shadowRadius": 0,
     }
   }
 >
@@ -94,6 +101,13 @@ exports[`renders button with icon 1`] = `
       "borderWidth": 0,
       "elevation": 0,
       "minWidth": 64,
+      "shadowColor": "#000000",
+      "shadowOffset": Object {
+        "height": 0,
+        "width": 0,
+      },
+      "shadowOpacity": 0.24,
+      "shadowRadius": 0,
     }
   }
 >
@@ -311,6 +325,13 @@ exports[`renders disabled button 1`] = `
       "borderWidth": 0,
       "elevation": 0,
       "minWidth": 64,
+      "shadowColor": "#000000",
+      "shadowOffset": Object {
+        "height": 0,
+        "width": 0,
+      },
+      "shadowOpacity": 0.24,
+      "shadowRadius": 0,
     }
   }
 >
@@ -399,6 +420,13 @@ exports[`renders loading button 1`] = `
       "borderWidth": 0,
       "elevation": 0,
       "minWidth": 64,
+      "shadowColor": "#000000",
+      "shadowOffset": Object {
+        "height": 0,
+        "width": 0,
+      },
+      "shadowOpacity": 0.24,
+      "shadowRadius": 0,
     }
   }
 >
@@ -495,6 +523,13 @@ exports[`renders outlined button with mode 1`] = `
       "borderWidth": 0.5,
       "elevation": 0,
       "minWidth": 64,
+      "shadowColor": "#000000",
+      "shadowOffset": Object {
+        "height": 0,
+        "width": 0,
+      },
+      "shadowOpacity": 0.24,
+      "shadowRadius": 0,
     }
   }
 >
@@ -578,6 +613,13 @@ exports[`renders text button by default 1`] = `
       "borderWidth": 0,
       "elevation": 0,
       "minWidth": 64,
+      "shadowColor": "#000000",
+      "shadowOffset": Object {
+        "height": 0,
+        "width": 0,
+      },
+      "shadowOpacity": 0.24,
+      "shadowRadius": 0,
     }
   }
 >
@@ -661,6 +703,13 @@ exports[`renders text button with mode 1`] = `
       "borderWidth": 0,
       "elevation": 0,
       "minWidth": 64,
+      "shadowColor": "#000000",
+      "shadowOffset": Object {
+        "height": 0,
+        "width": 0,
+      },
+      "shadowOpacity": 0.24,
+      "shadowRadius": 0,
     }
   }
 >
diff --git a/src/components/__tests__/__snapshots__/Chip.test.js.snap b/src/components/__tests__/__snapshots__/Chip.test.js.snap
index 27977bdde..fe1003bbc 100644
--- a/src/components/__tests__/__snapshots__/Chip.test.js.snap
+++ b/src/components/__tests__/__snapshots__/Chip.test.js.snap
@@ -10,6 +10,13 @@ exports[`renders chip with close button 1`] = `
       "borderStyle": "solid",
       "borderWidth": 0.5,
       "elevation": 0,
+      "shadowColor": "#000000",
+      "shadowOffset": Object {
+        "height": 0,
+        "width": 0,
+      },
+      "shadowOpacity": 0.24,
+      "shadowRadius": 0,
     }
   }
 >
@@ -184,6 +191,13 @@ exports[`renders chip with icon 1`] = `
       "borderStyle": "solid",
       "borderWidth": 0.5,
       "elevation": 0,
+      "shadowColor": "#000000",
+      "shadowOffset": Object {
+        "height": 0,
+        "width": 0,
+      },
+      "shadowOpacity": 0.24,
+      "shadowRadius": 0,
     }
   }
 >
@@ -307,6 +321,13 @@ exports[`renders chip with onPress 1`] = `
       "borderStyle": "solid",
       "borderWidth": 0.5,
       "elevation": 0,
+      "shadowColor": "#000000",
+      "shadowOffset": Object {
+        "height": 0,
+        "width": 0,
+      },
+      "shadowOpacity": 0.24,
+      "shadowRadius": 0,
     }
   }
 >
@@ -385,6 +406,13 @@ exports[`renders outlined disabled chip 1`] = `
       "borderStyle": "solid",
       "borderWidth": 0.5,
       "elevation": 0,
+      "shadowColor": "#000000",
+      "shadowOffset": Object {
+        "height": 0,
+        "width": 0,
+      },
+      "shadowOpacity": 0.24,
+      "shadowRadius": 0,
     }
   }
 >
@@ -467,6 +495,13 @@ exports[`renders selected chip 1`] = `
       "borderStyle": "solid",
       "borderWidth": 0.5,
       "elevation": 0,
+      "shadowColor": "#000000",
+      "shadowOffset": Object {
+        "height": 0,
+        "width": 0,
+      },
+      "shadowOpacity": 0.24,
+      "shadowRadius": 0,
     }
   }
 >
diff --git a/src/components/__tests__/__snapshots__/ListItem.test.js.snap b/src/components/__tests__/__snapshots__/ListItem.test.js.snap
index 6314b67ca..e021ecbc4 100644
--- a/src/components/__tests__/__snapshots__/ListItem.test.js.snap
+++ b/src/components/__tests__/__snapshots__/ListItem.test.js.snap
@@ -89,6 +89,13 @@ exports[`renders list item with custom description 1`] = `
                 "borderStyle": "solid",
                 "borderWidth": 0.5,
                 "elevation": 0,
+                "shadowColor": "#000000",
+                "shadowOffset": Object {
+                  "height": 0,
+                  "width": 0,
+                },
+                "shadowOpacity": 0.24,
+                "shadowRadius": 0,
               }
             }
           >
diff --git a/src/components/__tests__/__snapshots__/Menu.test.js.snap b/src/components/__tests__/__snapshots__/Menu.test.js.snap
index 0e298c723..57226cbcd 100644
--- a/src/components/__tests__/__snapshots__/Menu.test.js.snap
+++ b/src/components/__tests__/__snapshots__/Menu.test.js.snap
@@ -14,6 +14,13 @@ exports[`renders not visible menu 1`] = `
         "borderWidth": 0.5,
         "elevation": 0,
         "minWidth": 64,
+        "shadowColor": "#000000",
+        "shadowOffset": Object {
+          "height": 0,
+          "width": 0,
+        },
+        "shadowOpacity": 0.24,
+        "shadowRadius": 0,
       }
     }
   >
@@ -101,6 +108,13 @@ exports[`renders visible menu 1`] = `
         "borderWidth": 0.5,
         "elevation": 0,
         "minWidth": 64,
+        "shadowColor": "#000000",
+        "shadowOffset": Object {
+          "height": 0,
+          "width": 0,
+        },
+        "shadowOpacity": 0.24,
+        "shadowRadius": 0,
       }
     }
   >
diff --git a/src/components/__tests__/__snapshots__/Snackbar.test.js.snap b/src/components/__tests__/__snapshots__/Snackbar.test.js.snap
index 000431ed9..08ceece21 100644
--- a/src/components/__tests__/__snapshots__/Snackbar.test.js.snap
+++ b/src/components/__tests__/__snapshots__/Snackbar.test.js.snap
@@ -150,6 +150,13 @@ exports[`renders snackbar with action button 1`] = `
           "marginHorizontal": 8,
           "marginVertical": 6,
           "minWidth": "auto",
+          "shadowColor": "#000000",
+          "shadowOffset": Object {
+            "height": 0,
+            "width": 0,
+          },
+          "shadowOpacity": 0.24,
+          "shadowRadius": 0,
         }
       }
     >