fix(fscomponents): Updated Accordion component

There was an issue where the component would not update its size when the contents changed size. The most obvious case where this would fail is if you had two accordions nested with each other, opening/closing the inside one would not update the height of the outer one until you closed and opened it again. Also added the ability to put content inside of the accordion as a child instead of having to put it in the "content" prop and set title as a string instead of forcing a JSX.Element. This makes the component serializable as long as you avoid setting any styles. I also removed the titleHeight prop, because it didn't do anything, updated the comments to reflect the current state of the component, and updated the story and sample.

packages/fscomponents/src/components/Accordion.tsx

@@ -1,7 +1,6 @@
-import React, { Component, RefObject } from 'react';
+import React, { Component } from 'react';
 import {
   Animated,
-  findNodeHandle,
   Image,
   ImageStyle,
   ImageURISource,
@@ -11,7 +10,6 @@ import {
   Text,
   TextStyle,
   TouchableHighlight,
-  UIManager,
   View,
   ViewStyle
 } from 'react-native';
@@ -47,6 +45,7 @@ export interface AccordionProps {
   closedIconStyle?: StyleProp<ImageStyle>;
   /**
    * Content of the accordion
+   * @deprecated Make the contents a child instead
    */
   content?: JSX.Element;
   /**
@@ -71,7 +70,8 @@ export interface AccordionProps {
    */
   openTitleStyle?: StyleProp<ViewStyle>;
   /**
-   * Left, right, and bottom padding
+   * Bottom padding
+   * @deprecated Put the padding on the accordion contents instead
    */
   padding?: number;
   /**
@@ -93,15 +93,11 @@ export interface AccordionProps {
   /**
    * Content of the accordion title
    */
-  title: JSX.Element;
+  title: JSX.Element | string;
   /**
    *  Styles for the accordion title container
    */
   titleContainerStyle?: StyleProp<ViewStyle>;
-  /**
-   * Height of the accordion title container
-   */
-  titleHeight?: number;
   /**
    * Styles for the accordion title
    */
@@ -123,9 +119,8 @@ export interface AccordionProps {
 export interface AccordionState {
   arrowTranslateAnimation: Animated.Value;
   contentHeightAnimation: Animated.Value;
+  contentHeight: number;
   isOpen: boolean;
-  isMeasuring: boolean;
-  hasMeasured: boolean;
 }
 
 const ACCORDION_PADDING_DEFAULT = 15;
@@ -141,6 +136,12 @@ const AccordionStyles = StyleSheet.create({
   content: {
     overflow: 'hidden'
   },
+  contentLayout: {
+    // Need to do this so that heights are still calculated inside overflow: hidden
+    left: 0,
+    right: 0,
+    position: 'absolute'
+  },
   titleContainer: {
     flex: 1,
     flexDirection: 'row',
@@ -170,11 +171,11 @@ const AccordionStyles = StyleSheet.create({
  * via the state property. The default is closed.
  *
  * Because of how padding and flexed items are handled, the dimensions of the
- * accordion can be customized via two props: titleHeight and padding. The titleHeight
- * prop controls the height of the title, and the padding prop controls the size of
+ * accordion can be customized via two props: padding and paddingHorizontal. The padding
+ * prop controls the height of the contents, and the paddingHorizontal prop controls the size of
  * the left and right padding for the title as well as the padding around the contents.
  * This is because on iOS padding does not affect the height of a flexed container; the
- * height of said container is determined solely based on the hight of its children. (On
+ * height of said container is determined solely based on the height of its children. (On
  * web the height is the padding + height of the children.)
  */
 export class Accordion extends Component<AccordionProps, AccordionState> {
@@ -187,49 +188,32 @@ export class Accordion extends Component<AccordionProps, AccordionState> {
     }
   };
 
-  private contentView: RefObject<View>;
-
   constructor(props: AccordionProps) {
     super(props);
 
-    this.contentView = React.createRef<View>();
-
     this.state = {
       arrowTranslateAnimation: new Animated.Value(props.state === 'open' ? -90 : 90),
       contentHeightAnimation: new Animated.Value(0),
-      isOpen: (props.state === 'open'),
-      isMeasuring: false,
-      hasMeasured: false
+      contentHeight: 0,
+      isOpen: (props.state === 'open')
     };
   }
 
   // tslint:disable-next-line:cyclomatic-complexity
   render(): JSX.Element {
-    let computedContentStyle;
+    let computedContentStyle = {};
+    let layoutStyle;
 
-    if (this.shouldEnableAnimation()) {
-      // If we're animating, we need to determine the height of the accordion contents
-      // so we know to what height the animation should stop. When the the contents are
-      // laid out, this.contentOnLayout will be called, after which we can use
-      // state.contentHeightAnimation. This value will be 0 if the accordion defaults
-      // to closed, the height if the accordion defaults to open, or the current height
-      // of the animation if actively animating.
-      if (this.state.isMeasuring) {
-        computedContentStyle = { position: 'absolute', opacity: 0 };
-      } else if (this.state.hasMeasured) {
-        computedContentStyle = {
-          height: this.state.contentHeightAnimation
-        };
-      } else if (this.state.isOpen) {
-        computedContentStyle = {};
-      } else {
-        // Default to height: 'auto' until we determine the height of the contents.
-        // Because RN doesn't have this option for height we simply pass an empty
-        // object as the style.
-        computedContentStyle = { height: 0 };
-      }
-    } else {
-      computedContentStyle = this.state.isOpen ? {} : { height: 0 };
+    if (this.shouldEnableAnimation() &&
+      // If the content height hasn't been calculated yet
+      // and the accordion starts open just let it autosize
+      (this.state.contentHeight || this.props.state !== 'open')) {
+      computedContentStyle = {
+        height: this.state.contentHeightAnimation
+      };
+      layoutStyle = AccordionStyles.contentLayout;
+    } else if (!this.state.isOpen) {
+      computedContentStyle = { height: 0 };
     }
 
     return (
@@ -260,23 +244,28 @@ export class Accordion extends Component<AccordionProps, AccordionState> {
                 this.state.isOpen && this.props.openTitleStyle
               ]}
             >
-              {this.props.title}
+              {typeof this.props.title === 'string' ? (
+                <Text>{this.props.title}</Text>
+              ) : this.props.title}
             </View>
             {this.renderIcon()}
           </View>
         </TouchableHighlight>
         <Animated.View
-          ref={this.contentView}
           style={[
             AccordionStyles.content,
-            {paddingHorizontal: this.props.paddingHorizontal},
-            this.props.contentStyle,
             computedContentStyle
           ]}
-          onLayout={this.contentOnLayout}
         >
-          <View>
-            {this.props.content}
+          <View
+            onLayout={this.contentOnLayout}
+            style={[
+              {paddingHorizontal: this.props.paddingHorizontal},
+              this.props.contentStyle,
+              layoutStyle
+            ]}
+          >
+            {this.props.content || this.props.children}
             {this.state.isOpen && this.props.renderContent && this.props.renderContent()}
           </View>
         </Animated.View>
@@ -296,6 +285,8 @@ export class Accordion extends Component<AccordionProps, AccordionState> {
         bounciness: 0,
         toValue: height
       }).start();
+    } else {
+      this.state.contentHeightAnimation.setValue(height);
     }
 
     // TODO - make the rotation customizable
@@ -313,16 +304,15 @@ export class Accordion extends Component<AccordionProps, AccordionState> {
    * @param {LayoutChangeEvent} event The layout event.
    */
   private contentOnLayout = (event: LayoutChangeEvent) => {
-    if (
-      !this.state.hasMeasured &&
-      !this.state.isMeasuring &&
-      this.state.isOpen
-    ) {
-      const padding = this.props.padding || 0;
-      this.state.contentHeightAnimation.setValue(
-        event.nativeEvent.layout.height + padding
-      );
-      this.setState({ hasMeasured: true });
+    const padding = this.props.padding || 0;
+    const height = event.nativeEvent.layout.height + padding;
+    if (height !== this.state.contentHeight) {
+      this.setState({
+        contentHeight: height
+      });
+    }
+    if (this.state.isOpen) {
+      this.state.contentHeightAnimation.setValue(height);
     }
   }
 
@@ -417,23 +407,7 @@ export class Accordion extends Component<AccordionProps, AccordionState> {
     if (isCurrentlyOpen) {
       this.animateContent(0, false);
     } else {
-      this.setState({ isMeasuring: true }, () => {
-        requestAnimationFrame(() => {
-          const node = findNodeHandle(this.contentView.current);
-
-          if (node) {
-            UIManager.measure(node, (x: number, y: number, width: number, height: number) => {
-              this.setState({
-                isMeasuring: false,
-                hasMeasured: true
-              });
-              const padding = this.props.padding || 0;
-
-              this.animateContent(height + padding, true);
-            });
-          }
-        });
-      });
+      this.animateContent(this.state.contentHeight, true);
     }
   }
 }

packages/fscomponents/src/components/__stories__/Accordion.story.tsx

@@ -5,15 +5,21 @@
 import React from 'react';
 import {
   Image,
+  ImageURISource,
   Text,
   View
 } from 'react-native';
+import {
+  select,
+  text
+// tslint:disable-next-line no-implicit-dependencies
+} from '@storybook/addon-knobs';
 import { storiesOf } from '@storybook/react'; // tslint:disable-line:no-implicit-dependencies
 import { Accordion } from '../Accordion';
 
 const title = <Text>Menu Item</Text>;
 
-const icons = {
+const icons: Record<string, ImageURISource> = {
   closed: require('../../../assets/images/alert.png'),
   open: require('../../../assets/images/checkmarkValidation.png')
 };
@@ -48,19 +54,31 @@ storiesOf('Accordion', module)
       content={imageContent}
     />
   ))
-  .add('w/ arrow disclosure icon', () => (
+  .add('w/ arrow or plusminus disclosure icon', () => (
     <Accordion
       title={title}
       content={content}
-      iconFormat={'arrow'}
+      iconFormat={select('Format', ['arrow', 'plusminus'], 'arrow')}
     />
   ))
   .add('w/ custom disclosure icon', () => (
     <Accordion
       title={title}
       content={content}
       iconFormat={'image'}
-      openIconImage={icons.open}
-      closedIconImage={icons.closed}
+      openIconImage={icons[select('Open Icon', Object.keys(icons), 'open')]}
+      closedIconImage={icons[select('Closed Icon', Object.keys(icons), 'closed')]}
     />
+  ))
+  .add('nested as a child with a string title', () => (
+    <Accordion
+      title={text('Parent title', 'Parent')}
+      state={'open'}
+    >
+      <Accordion
+        title={title}
+        content={content}
+        paddingHorizontal={0}
+      />
+    </Accordion>
   ));

packages/pirateship/src/components/PSAccordionGroup.tsx

@@ -55,7 +55,6 @@ export default class PSAccordionGroup extends Component<PSAccordionGroupProps> {
       <Accordion
         key={i}
         title={this.renderTitle(item.title)}
-        titleHeight={60}
         content={this.renderContent(item.items || [])}
         style={styles.container}
         plusMinusStyle={{

packages/pirateship/src/screens/AccordionSample.tsx

@@ -67,6 +67,16 @@ class AccordionSample extends Component<AccordionSampleScreenProps> {
             openIconImage={icons.open}
             closedIconImage={icons.closed}
           />
+          <Accordion
+            title={'Open Parent Accordion with Image'}
+            state={'open'}
+          >
+            <Accordion
+              title={<Text>Nested Accordion with Image</Text>}
+              content={imageContent}
+              paddingHorizontal={0}
+            />
+          </Accordion>
         </View>
       </PSScreenWrapper>
     );