refactor: make ListItem accepts a custom description

[brunohkbx]

Motivation

Currently, there's no way to customize the description of List.Item. In order to make it able to be customized, List.Item now accepts a custom node as description.

With these changes we can implement a list item like Gmail app:

<List.Item
  left={() => (
    <Image
      source={require('../../assets/images/email-icon.png')}
      style={styles.image}
    />
  )}
  right={props => <List.Icon {...props} icon="star-border" />}
  title="List Item 1"
  description={({
    ellipsizeMode,
    color: descriptionColor,
    fontSize,
  }) => (
    <View style={[styles.container, styles.column]}>
      <Text
        numberOfLines={2}
        ellipsizeMode={ellipsizeMode}
        style={{ color: descriptionColor, fontSize }}
      >
        React Native Paper is a high-quality, standard-compliant
        Material Design library that has you covered in all major
        use-cases.
      </Text>
      <View style={[styles.container, styles.row, { paddingTop: 8 }]}>
        <Chip icon="picture-as-pdf" onPress={() => {}}>
          DOCS.pdf
        </Chip>
      </View>
    </View>
  )}
/>

Test plan

1. Download this branch
2. Run the example app
3. Open List.Section to see the new example.

[callstack-bot]

Hey @brunohkbx, thank you for your pull request 🤗. The documentation from this branch can be viewed here. Please remember to update Typescript types if you changed API.

[Trancever]

I am generally ok with render prop approach and I like it, but I don't like exposing List.Description component. Can we remove it?

Also, could you pass ellipsizeMode, color, fontSize to render function instead of ellipsizeMode and whole style obj?

[brunohkbx]

Thanks @Trancever. Done 5ae1695

[Trancever]

@brunohkbx Awesome, one more thing, can we improve description prop description? We should add info that it can be string or render prop and what arguments will be passed to that render prop. Please check other components that follow this pattern for reference.

[brunohkbx]

Good catch, thanks. Added in f00d906

[satya164]

imo the color and fontSize should be in style, not as separate props.

[Trancever]

@satya164 hmm, other components expose them as separate props, I wanted it to be consistent

[satya164]

hmm I think in most places where it's this way, it's usually for icons etc., but here looks like the render callback is passing a bunch of props (do we need to pass ellipsizeMode and fontSize for example?)

[brunohkbx]

@satya164 I found these components doing the same: ListItem, ListAccordion, Banner
I think we should pass ellipsizeMode and fontSize because could have cases when the user only wants to add more content to the description and keep the Text as the original. e.g. #686

[BenWildeman]

This should have never been merged as it's a breaking change but aside from that. the implementation here isn't really the "react way". What do I mean by that? Well you shouldn't have to pass in a callback in order to use a react component.

Because of this breaking change, it's no longer possible to pass in a text component to it which was perfectly valid beforehand. Here it checks that the description is explicitly a string, in order to put it into the text component, but then this PR #1199 has reversed it and is now explicitly checking for a function.

This PR did not remove the React.Node type which meant that this didn't throw any errors within TS

The ideal way to fix this is to do the following which would have satisfied all needs, without creating a breaking change:

// renderDescription function

const isDescReactElement = React.isValidElement(description); // would be false for anything that's not a react element, such as a string;

if (isDescReactElement) {
    const desc = React.cloneElement(description, {
        ellipsizeMode: descriptionEllipsizeMode,
        color: descriptionColor,
        fontSize: styles.description.fontSize,
    });

    return desc;
}

return (
    <Text	
         ellipsizeMode={descriptionEllipsizeMode}	
         numberOfLines={2}	
         style={[	
             styles.description,	
             {	
                 color: descriptionColor,	
              },	
              descriptionStyle,	
         ]}	
    >	
        {description}	
    </Text>
);

[brunohkbx]

@BenWildeman I didn't realize that it would break the current implementation 😞. I apologize for the inconvenience.
Thanks for showing your implementation, it's good to know but I'm not sure if it would be ok to pass a component as a prop because this entire project is using render props to return elements.
Maybe @satya164, @Trancever or @wojteg1337 can check your fix.

[BenWildeman]

it's fine with this, it's just that the original types are incorrect really. there shouldn't have been React.Node if it was only supposed to accept a string. you fixed it to be inline with the rest of the codebase. it's just annoying that a minor bump included a breaking change that broke our app in places that we didn't know about. luckily we do regression testing so it was caught