#3 ~ events page recent tweets

[mts]

• Adds recent tweets to events page filtering re-tweets.
• Opens tweet when clicked
• Moves recent tweets without header to upcoming events section
• Documents how to change getRecentTweets Lambda function environment variables
  docs/getRecentTweets.md

[mvdam]

It looks really good so far! I have two more suggestions;

• Can you give the tweet date a bit more contrast? Something like the color of the suggestion of @Tenchi2xh ?;

  

• Maybe hide the retweets and only show the tweets posted by the author? (no sure tho)

[mvdam]

I think it's a bit more readable when you use an early return. Something like;

const FancyTweetList = ({ tweets }: IFancyTweetListProps) => {
  if (!tweets.length) {
    return null
  }

  return (
    <Card>
      ...
    </Card>
  )
}

[mts]

I removed the render condition anyway. It's checked one level up in NextEventsBlock.

[mvdam]

What especially makes this TweetList 'fancy'? 😉

[mts]

TweetList also fine. Renamed.

[mdvanes]

In response to this review of @mvdam

It looks really good so far! I have two more suggestions;

• Can you give the tweet date a bit more contrast? Something like the color of the suggestion of > @Tenchi2xh ?;

• Maybe hide the retweets and only show the tweets posted by the author? (no sure tho)

The current design is already a great improvement, much easier to digest. I would suggest for this version: Just add an alternate text color for the date/time and leave the rest of the design. I would like to (but this can be in a card for a later version):

• on clicking the tweet it will open that tweet directly.
• move (without header) to be displayed in the "next event details" section, to make it clear that it belongs to that event.

Another question: the event is now in the past, how do we switch to another Twitter account? Can you document this somewhere?

[mvdam]

The layout looks really nice now, great job!
I've added some comments still. Let me know how you think about it!

[mvdam]

These kind of logic and filtering belongs in the container component. Can you move it to EventsContainer? It might be convenient to use a filter instead of checking tweet.text.includes('RT ') for each tweet.

[mvdam]

And I have one more suggestions as wel; Can you also hide tweets that start with @ (public mentions)?

[mts]

I do not mind either of these. Tweets are now filtered.

[mvdam]

I still have some doubts if this is really helping us. There is a lot of code duplication in here. Do you have concerns using -just- <h1 className={...}>...</h1>, <h2 className={...}>...</h2>, .... ?

[mts]

Using className is an option depending on bootstrap class text-white. Refined to reuse class names from props instead.

[mdvanes]

Hi @mts, I am ready to approve this but before we merge it in: how do we turn it off / switch to a different Twitter account?