Add podcast series list component

[CaleCrawford]

Uses the generic series tile component to create a list of podcasts. Once merged will be used to create lists for meditations and liturgies.

[brettdh]

LGTM overall. A couple questions regarding some style things that we're figuring out in parallel :)

[brettdh]

High-level observation: we have a "podcast series tile" which is a specialization of a generic "series tile". We also have a "liturgy series tile" and a "meditation series tile". They are all very similar, with one distinction: they differ in their description content.

I've made a similar observation in #50, with the generic "list card" component, which has similar specializations. However, where you've left it to the containing component (in this case, PodcastSeriesTileList), I've chosen to extract a component (e.g. PodcastEpisodeListCard) that encapsulates this.

I slightly prefer the approach I've taken (naturally, since that's how I wrote it 🙂 ), but I think consistency is more important than which one is better. What do you think?

[brettdh]

This should be named the same as its file: PodcastSeriesList. (I like the lack of Tile in that name, btw, since it allows flexibility in case the layout needs to change in the future.)

[brettdh]

I think this should just be called podcast, since it's just a data object.

[brettdh]

I lean towards imageSource rather than image (since it's the same prop type as the source prop of Image) - but again, we should just agree on one or the other and be consistent.

[brettdh]

It just occurred to me - maybe we should just use Image.propTypes.source instead of using our own here, similar to how we've started using View.propTypes.style. (This will get cleaner once jsx-eslint/eslint-plugin-react#1671 is merged.)

[brettdh]

Suggest podcasts

[brettdh]

Do you need the wrapper View here?

[CaleCrawford]

Pushed some updates. Would love to get your feedback when you get a chance just to make sure that I understood your advice correctly. Waited on changing out Image.propTypes.source until the eslint issue is settled.
One piece that I wasn't sure of how to do was the description line. I had a hard time just passing in a "string" because the different descriptions need different styles and I was not sure how to do that on the parent component? Does that make sense? If not I can explain that further.

[brettdh]

This is looking great! I think you understood my advice perfectly. 🙂 I only have a few minor things below.

One piece that I wasn't sure of how to do was the description line. I had a hard time just passing in a "string" because the different descriptions need different styles and I was not sure how to do that on the parent component? Does that make sense? If not I can explain that further.

Do they really need different styles? I was having a hard time seeing that when I glanced through the designs. One thing I recall is that you used an icon to render the separator bullet, where I used the unicode character '•'. Does that make a difference? Then I think it's a single style for the Text that SeriesTile uses to render the description.

If that doesn't work because of something I'm missing, let's discuss further, but I think it just might work.

[brettdh]

This works for me without lint errors:

const PodcastSeriesList = ({ podcasts, onPressPodcast }) => (
  <ScrollView>
    <View style={styles.list}>
      {podcasts.map(podcast => (
        <PodcastSeriesTitle
          key={podcast.title}
          podcast={podcast}
          onPressPodcast={() => onPressPodcast(podcast)}
        />
      ))}
    </View>
  </ScrollView>
);

[brettdh]

I think you meant Tile here, not Title? (This is typo'd in a few other places too)

[brettdh]

Maybe just onPress, since there's only one thing that can be pressed here? (Unlike in the list, where you're pressing part of the list, not the whole list)

[brettdh]

Suggest podcast instead of tile here, since the callback receives a data object, not a Tile component.

[brettdh]

Perhaps selectedPodcast here to match the data type below.

[brettdh]

Tiniest of nits: "The Liturgists Podcast" :)

[brettdh]

I wonder if PodcastSeriesDescription really needs to be a separate thing from PodcastSeriesTile. I don't see it being reused outside that context, and it might be good to have all the logic for rendering a PodcastSeriesTile in one place, since there's not much to it. formatPodcastDescription could take care of that without delegating further.

[brettdh]

This is the data element that I think makes more sense as a moment. Then PodcastSeriesTile is responsible for formatting it for presentation.

[CaleCrawford]

Okay updated the branch with your suggestions and added meditations & liturgies. If you're all good I'll merge, otherwise ill make further changes.

[brettdh]

Looks great! I might suggest using PureComponent, since none of these use state. (I’ve been mainly using functional components so far, but that’s a bigger transformation, and I don’t feel terribly strongly about it. I’ll leave that up to you.)

Feel free to merge with that one minor tweak. Thanks!