Repository: callstack/react-native-paper
Pull request #1037 (2019-04-30 12:00:48)

fix: elevation being applied after mode changes on a button while it shouldn't
### Motivation
The wrong elevation is being applied to an outlined button when it is first being rendered as a contained button.

Reproduce steps:
1. Render a button using `mode={this.propsIsContained ? 'contained' : 'outline'}` and set `propsIsContained = 'true'`.
2. Change the property in the component to false during runtime `propsIsContained = false`
3. Observe that elevation is applied on the outlined button while it shouldn't.

**WRONG**
![image](https://user-images.githubusercontent.com/18186115/56661857-e5f0a800-66a2-11e9-968c-480b168c3c29.png)

**GOOD**
![image](https://user-images.githubusercontent.com/18186115/56661889-f6a11e00-66a2-11e9-90b6-5a5165730a9b.png)

FYI: duplicate of #1019 didn't know how to update that one.

callstack-bot: Hey @jordy-wijman, thank you for your pull request 🤗. The documentation from this branch can be viewed [here](https://10086-71323749-gh.circle-artifacts.com/0/docs/index.html). Please remember to update Typescript types if you changed API.

Patch:
diff --git a/src/components/Button.js b/src/components/Button.js
index 6625fab5a..8b595c7e4 100644
--- a/src/components/Button.js
+++ b/src/components/Button.js
@@ -221,7 +221,8 @@ class Button extends React.Component<Props, State> {
     };
     const touchableStyle = { borderRadius: roundness };
     const textStyle = { color: textColor, fontFamily };
-    const elevation = disabled ? 0 : this.state.elevation;
+    const elevation =
+      disabled || mode !== 'contained' ? 0 : this.state.elevation;
 
     return (
       <Surface
diff --git a/src/components/Menu/Menu.js b/src/components/Menu/Menu.js
index 3b961f47e..afc74ab12 100644
--- a/src/components/Menu/Menu.js
+++ b/src/components/Menu/Menu.js
@@ -11,6 +11,7 @@ import {
   I18nManager,
   BackHandler,
   Platform,
+  findNodeHandle,
 } from 'react-native';
 import { withTheme } from '../../core/theming';
 import type { Theme } from '../../types';
@@ -51,6 +52,7 @@ type Props = {
 };
 
 type State = {|
+  rendered: boolean,
   top: number,
   left: number,
   windowLayout: {| height: number, width: number |},
@@ -126,7 +128,16 @@ class Menu extends React.Component<Props, State> {
     statusBarHeight: APPROX_STATUSBAR_HEIGHT,
   };
 
+  static getDerivedStateFromProps(nextProps, prevState) {
+    if (nextProps.visible && !prevState.rendered) {
+      return { rendered: true };
+    }
+
+    return null;
+  }
+
   state = {
+    rendered: this.props.visible,
     top: 0,
     left: 0,
     windowLayout: { width: 0, height: 0 },
@@ -143,12 +154,11 @@ class Menu extends React.Component<Props, State> {
   }
 
   componentWillUnmount() {
-    BackHandler.removeEventListener('hardwareBackPress', this._handleDismiss);
-    Dimensions.removeEventListener('change', this._handleDismiss);
+    this._removeListeners();
   }
 
-  _anchor: ?View;
-  _menu: ?View;
+  _anchor: View | null = null;
+  _menu: View | null = null;
 
   _measureMenuLayout = () =>
     new Promise(resolve => {
@@ -168,7 +178,11 @@ class Menu extends React.Component<Props, State> {
       }
     });
 
-  _updateVisibility = () => {
+  _updateVisibility = async () => {
+    // Menu is rendered in Portal, which updates items asynchronously
+    // We need to do the same here so that the ref is up-to-date
+    await Promise.resolve();
+
     if (this.props.visible) {
       this._show();
     } else {
@@ -176,6 +190,23 @@ class Menu extends React.Component<Props, State> {
     }
   };
 
+  _isBrowser = () => 'document' in global;
+
+  _focusFirstDOMNode = (el: View | null) => {
+    if (el && this._isBrowser()) {
+      // When in the browser, we want to focus the first focusable item on toggle
+      // For example, when menu is shown, focus the first item in the menu
+      // And when menu is dismissed, send focus back to the button to resume tabbing
+      const node: any = findNodeHandle(el);
+      const focusableNode = node.querySelector(
+        // This is a rough list of selectors that can be focused
+        'button, [href], input, select, textarea, [tabindex]:not([tabindex="-1"])'
+      );
+
+      focusableNode && focusableNode.focus();
+    }
+  };
+
   _handleDismiss = () => {
     if (this.props.visible) {
       this.props.onDismiss();
@@ -183,10 +214,29 @@ class Menu extends React.Component<Props, State> {
     return true;
   };
 
-  _show = async () => {
+  _handleKeypress = (e: KeyboardEvent) => {
+    if (e.key === 'Escape') {
+      this.props.onDismiss();
+    }
+  };
+
+  _attachListeners = () => {
     BackHandler.addEventListener('hardwareBackPress', this._handleDismiss);
     Dimensions.addEventListener('change', this._handleDismiss);
 
+    this._isBrowser() &&
+      document.addEventListener('keyup', this._handleKeypress);
+  };
+
+  _removeListeners = () => {
+    BackHandler.removeEventListener('hardwareBackPress', this._handleDismiss);
+    Dimensions.removeEventListener('change', this._handleDismiss);
+
+    this._isBrowser() &&
+      document.removeEventListener('keyup', this._handleKeypress);
+  };
+
+  _show = async () => {
     const windowLayout = Dimensions.get('window');
     const [menuLayout, anchorLayout] = await Promise.all([
       this._measureMenuLayout(),
@@ -207,7 +257,6 @@ class Menu extends React.Component<Props, State> {
       !anchorLayout.width ||
       !anchorLayout.height
     ) {
-      BackHandler.removeEventListener('hardwareBackPress', this._handleDismiss);
       setTimeout(this._show, ANIMATION_DURATION);
       return;
     }
@@ -230,6 +279,8 @@ class Menu extends React.Component<Props, State> {
         },
       },
       () => {
+        this._attachListeners();
+
         Animated.parallel([
           Animated.timing(this.state.scaleAnimation, {
             toValue: { x: menuLayout.width, y: menuLayout.height },
@@ -243,14 +294,17 @@ class Menu extends React.Component<Props, State> {
             easing: EASING,
             useNativeDriver: true,
           }),
-        ]).start();
+        ]).start(({ finished }) => {
+          if (finished) {
+            this._focusFirstDOMNode(this._menu);
+          }
+        });
       }
     );
   };
 
   _hide = () => {
-    BackHandler.removeEventListener('hardwareBackPress', this._handleDismiss);
-    Dimensions.removeEventListener('change', this._handleDismiss);
+    this._removeListeners();
 
     Animated.timing(this.state.opacityAnimation, {
       toValue: 0,
@@ -259,7 +313,10 @@ class Menu extends React.Component<Props, State> {
       useNativeDriver: true,
     }).start(finished => {
       if (finished) {
+        this._focusFirstDOMNode(this._anchor);
+
         this.state.scaleAnimation.setValue({ x: 0, y: 0 });
+        this.setState({ rendered: false });
       }
     });
   };
@@ -276,6 +333,7 @@ class Menu extends React.Component<Props, State> {
     } = this.props;
 
     const {
+      rendered,
       windowLayout,
       menuLayout,
       anchorLayout,
@@ -398,38 +456,29 @@ class Menu extends React.Component<Props, State> {
         collapsable={false}
       >
         {anchor}
-        <Portal>
-          {visible ? (
+        {rendered ? (
+          <Portal>
             <TouchableWithoutFeedback onPress={onDismiss}>
               <View style={StyleSheet.absoluteFill} />
             </TouchableWithoutFeedback>
-          ) : null}
-          <View
-            ref={ref => {
-              // This hack is needed to properly show menu
-              // when visible is `true` initially
-              // because in componentDidMount _menu ref is undefined
-              // because it's rendered in portal
-              if (!this._menu) {
+            <View
+              ref={ref => {
                 this._menu = ref;
-                if (visible) {
-                  this._show();
-                }
-              }
-            }}
-            collapsable={false}
-            pointerEvents={visible ? 'auto' : 'none'}
-            style={[styles.wrapper, positionStyle, style]}
-          >
-            <Animated.View style={{ transform: positionTransforms }}>
-              <Surface
-                style={[styles.shadowMenuContainer, shadowMenuContainerStyle]}
-              >
-                {children}
-              </Surface>
-            </Animated.View>
-          </View>
-        </Portal>
+              }}
+              collapsable={false}
+              accessibilityViewIsModal={visible}
+              style={[styles.wrapper, positionStyle, style]}
+            >
+              <Animated.View style={{ transform: positionTransforms }}>
+                <Surface
+                  style={[styles.shadowMenuContainer, shadowMenuContainerStyle]}
+                >
+                  {children}
+                </Surface>
+              </Animated.View>
+            </View>
+          </Portal>
+        ) : null}
       </View>
     );
   }
diff --git a/typings/components/Chip.d.ts b/typings/components/Chip.d.ts
index d51c67022..00355163d 100644
--- a/typings/components/Chip.d.ts
+++ b/typings/components/Chip.d.ts
@@ -8,6 +8,7 @@ export interface ChipProps {
   icon?: IconSource;
   avatar?: React.ReactNode;
   selected?: boolean;
+  selectedColor?: string;
   disabled?: boolean;
   accessibilityLabel?: string;
   onPress?: () => any;
diff --git a/typings/components/Menu.d.ts b/typings/components/Menu.d.ts
index 730ea72cf..53a1587e5 100644
--- a/typings/components/Menu.d.ts
+++ b/typings/components/Menu.d.ts
@@ -7,7 +7,6 @@ export interface MenuProps {
   anchor: React.ReactNode;
   onDismiss: () => void;
   children: React.ReactNode;
-  statusBarHeight?: number,
   style?: StyleProp<ViewStyle>;
   theme?: ThemeShape;
 }
@@ -21,6 +20,6 @@ export interface ItemProps {
   style?: StyleProp<ViewStyle>;
 }
 
-export declare class Menu extends React.Component<MenuProps> {
-    static Item: React.ComponentType<ItemProps>;
+export class Menu extends React.Component<MenuProps> {
+  static Item: React.ComponentType<ItemProps>;
 }
diff --git a/typings/components/Modal.d.ts b/typings/components/Modal.d.ts
index b1f1564ff..d58048a20 100644
--- a/typings/components/Modal.d.ts
+++ b/typings/components/Modal.d.ts
@@ -1,4 +1,5 @@
 import * as React from 'react';
+import { StyleProp, ViewStyle } from 'react-native';
 import { ThemeShape } from '../types';
 
 export interface ModalProps {
@@ -6,6 +7,7 @@ export interface ModalProps {
   onDismiss?: () => any;
   visible: boolean;
   children: React.ReactNode;
+  contentContainerStyle?: StyleProp<ViewStyle>;
   theme?: ThemeShape;
 }
 
diff --git a/typings/components/Searchbar.d.ts b/typings/components/Searchbar.d.ts
index 5381d8977..451ab9dcd 100644
--- a/typings/components/Searchbar.d.ts
+++ b/typings/components/Searchbar.d.ts
@@ -1,8 +1,9 @@
 import * as React from 'react';
-import { TextInputProps } from 'react-native';
+import { StyleProp, TextInputProps, TextStyle } from 'react-native';
 import { ThemeShape, IconSource } from '../types';
 
 export interface SearchbarProps extends TextInputProps {
+  inputStyle?: StyleProp<TextStyle>;
   icon?: IconSource;
   theme?: ThemeShape;
   clearIcon?: IconSource;


Test patch:
diff --git a/__ts-tests__/Chip.test.tsx b/__ts-tests__/Chip.test.tsx
index ff3959429..9fc2fdd5b 100644
--- a/__ts-tests__/Chip.test.tsx
+++ b/__ts-tests__/Chip.test.tsx
@@ -3,7 +3,7 @@ import * as React from 'react';
 import { Chip } from '..';
 
 const MyComponent = () => (
-  <Chip icon="info" onPress={() => console.log('Pressed')}>Example Chip</Chip>
+  <Chip icon="info" selectedColor="#fff" onPress={() => console.log('Pressed')}>Example Chip</Chip>
 );
 
 export default MyComponent;
diff --git a/src/components/__tests__/__snapshots__/Banner.test.js.snap b/src/components/__tests__/__snapshots__/Banner.test.js.snap
index f1255747d..50efb250a 100644
--- a/src/components/__tests__/__snapshots__/Banner.test.js.snap
+++ b/src/components/__tests__/__snapshots__/Banner.test.js.snap
@@ -195,13 +195,6 @@ 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,
             }
           }
         >
@@ -364,13 +357,6 @@ 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,
             }
           }
         >
@@ -454,13 +440,6 @@ 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 0f99c199f..7072a3874 100644
--- a/src/components/__tests__/__snapshots__/Button.test.js.snap
+++ b/src/components/__tests__/__snapshots__/Button.test.js.snap
@@ -11,13 +11,6 @@ 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,
     }
   }
 >
@@ -101,13 +94,6 @@ 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,
     }
   }
 >
@@ -413,13 +399,6 @@ exports[`renders loading button 1`] = `
       "borderWidth": 0,
       "elevation": 0,
       "minWidth": 64,
-      "shadowColor": "#000000",
-      "shadowOffset": Object {
-        "height": 0,
-        "width": 0,
-      },
-      "shadowOpacity": 0.24,
-      "shadowRadius": 0,
     }
   }
 >
@@ -516,13 +495,6 @@ 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,
     }
   }
 >
@@ -606,13 +578,6 @@ 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,
     }
   }
 >
@@ -696,13 +661,6 @@ 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__/Menu.test.js.snap b/src/components/__tests__/__snapshots__/Menu.test.js.snap
index 57226cbcd..0e298c723 100644
--- a/src/components/__tests__/__snapshots__/Menu.test.js.snap
+++ b/src/components/__tests__/__snapshots__/Menu.test.js.snap
@@ -14,13 +14,6 @@ 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,
       }
     }
   >
@@ -108,13 +101,6 @@ 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 08ceece21..000431ed9 100644
--- a/src/components/__tests__/__snapshots__/Snackbar.test.js.snap
+++ b/src/components/__tests__/__snapshots__/Snackbar.test.js.snap
@@ -150,13 +150,6 @@ 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,
         }
       }
     >