Skip to content
This repository has been archived by the owner on Jun 24, 2022. It is now read-only.

Fixes blank tutorial pages when in landscape #823

Merged
merged 1 commit into from
Jul 21, 2020

Conversation

joshpc
Copy link
Contributor

@joshpc joshpc commented Jul 21, 2020

What this does

This fixes an issue where tutorial pages would render as blank when presented in landscape mode.

Screen Shot 2020-07-21 at 1 51 16 PM

Why? How?

The Carousel's rendering optimization (of not rendering off-screen elements) was not rendering the first element when the Carousel itself was completely offscreen. A re-render was properly triggering it to be visible, but the effect is a little jarring (it's a screen flicker) or broken (completely blank in landscape mode as no re-render is triggered.)

By disabling removeClippedSubviews this forces the views to be rendered and visible. A side-effect of this is that the views that are off-screen are rendered and on the stack, but given the small quantity of views, this may be an acceptable change.

An alternative fix would be to use Carousel's triggerRenderingHack-- see: https://github.com/archriss/react-native-snap-carousel/blob/master/doc/PROPS_METHODS_AND_GETTERS.md#methods -- which addresses this issue specifically.

Note: This issue was not evident when the device was in portrait mode as a weird "jiggle" is present when opening the view in portrait mode. That jiggle seems to be a rendering bug. This appears to force the view to re-render (correctly.) Gif with slow animations below:

Without fix
broken-portrait
broken-landscape

With fix
fixed-portrait
fixed-landscape

Tested

  • Tested on iOS Simulators and iPhone X

This fixes an issue where tutorial pages would render as blank when presented in landscape mode.

The `Carousel` component would clip subviews that would be rendered offscreen and when the view would be presented, it would not be re-rendered. As the entirety of the view is offscreen when presented in landscape mode, it was not rendered until the user interacted with the view.
@joshpc joshpc requested a review from a team as a code owner July 21, 2020 20:01
@timarney
Copy link
Member

Thanks @joshpc we'll take a look as soon as we can.

@timarney
Copy link
Member

timarney commented Jul 21, 2020

Nice :)

Test on device or Android and iOS

Noting this for future reference:

Seems to be okay for our use case.

meliorence/react-native-snap-carousel#492 (comment)

@timarney timarney merged commit 3a58be4 into cds-snc:master Jul 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants