[NavigationExperimental] Render overlay first in NavigationAnimatedView

[tlvenn]

At the moment, the overlay is the last element rendered in the AnimationAnimatedView:
https://github.com/facebook/react-native/blob/master/Libraries/NavigationExperimental/NavigationAnimatedView.js#L165

The drawback of this approach is you have to position the overlay in absolute to get it on top and then you have to manually add a marginTop to your scene if the overlay is not a true overlay, like the navigation header.

Given RN flexbox implementation does not support order property, we cannot re arrange it to be first. I dont think there is any drawback to actually renderer it first so that we can naturally leverage flexbox if needed.

@ericvicenti any though ?

Thanks

[ericvicenti]

We still need to be able to position the header above everything else. I think it may be appropriate to also have an underlay which works in the same way as the overlay but is rendered before any scenes.

@hedgerwang, any thoughts?

[tlvenn]

I assumed that you would actually use the overlay to inject the header and it would therefore naturally be above everything else. Am missing something ?

[ericvicenti]

We still need to be able to overlay the header above the scene because in some apps (usually iOS), the header is partially transparent or blurry, and it is expected to see the scene scrolling behind it.

[tlvenn]

But if you position the overlay in absolute, don't you already get that, independently of the overlay being rendered first or not ? The render flow/order should not matter anymore.

[tlvenn]

Ha i guess you are talking about the z-index and by rendered last you make sure it's above (elevation) the scene...

[tlvenn]

So In the scenario that the overlay is solid, being able to underlay rather than overlay would be pretty handy.

[ericvicenti]

Yeah, that makes sense. So I would be in support of a new renderUnderlay prop. But I understand that naming is confusing. If this is important to you, can you file a new issue about the naming and propose some alternate names?

[tlvenn]

Sure thing, thanks for the quick feedback !

[tlvenn]

I also wonder that given you dont really want to have both an underlay and an overlay, maybe the solution would be to introduce a boolean prop renderOverlayFirst to indicate if the overlay should be rendered first or not. Or the reverse, renderOverlayLast with default value to true.

What do you think ?

[tlvenn]

But that does leave things a little bit weird with an overlay that potentially ends up not being technically over...

[ericvicenti]

I think there may be some extreme use cases where developers may want both an overlay and an underlay, and it is easy to support both, so I think we should probably do that.

Another thing to consider: we could remove the overlay entirely from this view and let the app developer do this part themselves, or use a higher-level component that has the desired behavior

[hedgerwang]

@tlvenn : could you please try to move the overlay before the scenes and remove the absolute position styles and see whether that would work with multiple scenes? I think it makes senses to not enforce fixed-height header and margin-top.

@ericvicenti: I think overlay could be removed off the AnimatedView. Otherwise we'd have to add more overlays for footer, drawer, sidebar....etc.

We could decouple the API to be something like this:

<View>
  <NavigationAnimatedView navigationState={navigationState} renderScene={renderHeader} />
  <NavigationAnimatedView navigationState={navigationState} renderScene={renderScene} />
</View>

[ericvicenti]

@hedgerwang, that is problematic because on iOS the header needs to animate at the same speed as the gesture that is happening on the scene

[tlvenn]

@hedgerwang It does work just fine as far as your overlay is in the flow and push downward the scene but as pointed out by @ericvicenti , if you want your overlay to float above your scene, out of its flow, if you render it first, it will be behind the scene (z-index) instead of being on top of it.