Group Nav: Use ScrollView instead of View

[im-adithya]

Description

This PR updates TitleGroup.js to use ScrollView so that the avatars don't overflow
and can be scrolled to view entire contents.

Issues related to PR

Fixes issue: #3700

Screenshots

Normal:

On rotating screen:

[chrisbobbe]

Thanks, @im-adithya! I've just tried this out on my iPhone, and it's looking good.

A nice-to-have would be a more graceful way to handle the left and right sides of the fixed rectangle, the one that acts as a "window" onto the content that scrolls left and right. Right now, it just ends abruptly:

I seem to remember a pattern where a rectangle like this fades into the background at the edge. Here's a Stack Overflow answer with an image that conveys what I'm thinking of (screenshot below):

(We'd leave out the users' names.)

Am I imagining things, or is that a common pattern to use?

I see a prop for ScrollView called FadingEdgeLength that might do something like what I'm thinking of. What happens if you use something greater than zero for that prop? Looks like it's Android-only, though, hmm.

A small nit in the commit message: instead of "Fixes #3700", our habit is to say "Fixes: #3700" (note the colon).

[chrisbobbe]

What's the reason for removing the style={styles.navWrapper}? 🙂 I don't think I'm attached to it (though I haven't thought super hard about it yet). But if we remove it, I'd like to record a reason for doing so.

…Ah, I see that I get the following error if I try to add that back in on the ScrollView:

Invariant Violation: ScrollView child layout (["alignItems","justifyContent"]) must be applied through the contentContainerStyle prop.

What happens if you follow that recommendation, and apply alignItems and justifyContent through the contentContainerStyle prop, instead of the style prop?

Or is there some other motivation for dropping styles.navWrapper?

[im-adithya]

Or is there some other motivation for dropping styles.navWrapper?

I thought it was of no use as the ScrollView horizontal is equivalent to flexDirection: row, and the spacing is taken care of by the marginRight property of the avatars

[chrisbobbe]

ScrollView horizontal is equivalent to flexDirection: row

This may sound a bit nit-picky, but this isn't really true. 🙂 Strictly speaking, they aren't equivalent: passing true for horizontal will help us have horizontal scrolling behavior, while flexDirection: 'row' will not (and it hasn't been; we've been setting that, and we don't have horizontal scrolling). Since we know of at least that difference, it might be useful to explore more, to help avoid accidentally introducing bugs.

What happens if you follow that recommendation, and apply alignItems and justifyContent through the contentContainerStyle prop, instead of the style prop?

Have you tried this out? 🙂 Do you notice anything?

[im-adithya]

Looks like it's Android-only, though, hmm.

So what should we do now? Can we use a component to solve this for both platforms or leave it as is for iOS? I found react-native-fading-edge as a good option after some research.

[im-adithya]

Sorry for being late! Added the fadingEdgeLength for now. See thread here

Here are the screenshots:

@chrisbobbe, Please review it now :)

[chrisbobbe]

Sorry for being late!

No problem! 🙂

One nit I see is let's put a code comment on the Android-only prop that gives the fade effect, so we can tell it's Android-only without having to look it up (that seems like a useful thing to know right away when reading that code).

[im-adithya]

Done!