FlatList blank area

[fortmarek]

I have added a computation of blank area for FlatList on both iOS and Android.

The usage is the following:

<BlankAreaView>
   <FlatList ... />
<BlankAreaView />

BlankAreaView, a native component, will then find the relevant views in its subviews (children on Android) and do similar computations as we do in AutoLayoutView. To report the blank area, we then use the same event as we do for RecyclerFlatList - this means the blank area is also already reported to the Flipper plugin.

[naqvitalha]

Why do we need list size in this event?

[fortmarek]

I am using it in the flipper plugin to trim blank spaces bigger than the listSize which skews the graph too much. We report it on iOS in AutoLayoutView, too, it might be missing there on Android.

[naqvitalha]

I think we can drop blankArea and listSize. For listSize you can set any max value like window height and blank area is just max of other two values.
The event payload size will reduce by half. Will keep the bridge free

[fortmarek]

For listSize you can set any max value like window height and blank area is just max of other two values.

This assumes the list is as big as the window height - I'd keep it for now and remove listSize once we move to instance events. I jotted this down here.

I'll get rid of blankArea from the event.

[naqvitalha]

it != null will never be true. View children cannot be null. Correct me if I'm wrong. Both the iterations don't seem to be necessary

[fortmarek]

If I remove this check, I am getting null cannot be cast to non-null type android.view.ViewGroup so I think the the assumption of that view children cannot be null is incorrect.

[fortmarek]

I also need to cast the children to ViewGroup as they are by default View which does not have methods like getChildAt

[naqvitalha]

There's an empty check above already. Can this exception ever be thrown?

[fortmarek]

It is possible that either cells.first or cells.last will not be able to resolve any element, so yes, this can be potentially thrown, I believe.

[ElviraBurchik]

This method is basically the same as the one in AutoLayoutView. We could reuse it and have tests covering this logic for both lists

[fortmarek]

It's almost the same, but AutoLayoutView has renderAheadOffset. We could pass the renderAheadOffset as zero for BlankAreaView but either way, eventually, AutoLayoutView and BlankAreaView should not be part of the same module, so I don't think reusing a couple of lines of code is worth the hassle.

[ElviraBurchik]

Nit, but I think this is easier to read

Suggested change

	let cells = scrollView.subviews.first(where: { $0 is RCTScrollContentView })?.subviews ?? []
	guard !cells.isEmpty else { return (0, 0, 0) }
	let cells = scrollView.subviews.first(where: { $0 is RCTScrollContentView })?.subviews
	guard let cells = cells, !cells.isEmpty else { return (0, 0, 0) }

[fortmarek]

I personally think it's conceptually easier to reason about a variable of type [Element] rather than [Element]?, so I think it's better to keep it as-is.

[ElviraBurchik]

I get multiple blank area offset events with753 on layout. I guess it's screen size, so it means that the screen is fully blank on open?

[davebcn87]

Is KVO working consistent in this case? Shouldn't we use ScrollView delegate method instead?

[fortmarek]

the scrollView already has a set delegate which is not under our direct control (it's RCTScrollView). This code is asynchronous, so there might be some delays and it'd be better to find synchronous alternative. However, it should not lead to incorrect measurements since we're reading the current scrollView.contentOffset - but if you have an idea how to get a callback for each frame or change in contentOffset synchronously, let me know.

[fortmarek]

I get multiple blank area offset events with753 on layout. I guess it's screen size, so it means that the screen is fully blank on open?

That's correct - I think of keeping those blank events and ignore them in the JS layer. This is a good place to compute the final TTI - I'll look into if we can get RecyclerFlatList to report these empty frames, too, as they should be there as well. Additionally, I believe it's more representable of what the user sees as they indeed see a few frames of empty screen and it'd be nice if our measurements reflected this. cc @davebcn87 @naqvitalha