Skip to content
This repository has been archived by the owner on Feb 8, 2020. It is now read-only.

fix: don't fade incoming background when fading header #127

Merged
merged 2 commits into from
Oct 16, 2019

Conversation

leethree
Copy link
Contributor

@leethree leethree commented Oct 16, 2019

During a cross-fade, the header on the back should still have opacity=1 to prevent the header become translucent and avoid anything in the back showing up.

The problem with the current animation is that opacities of layers are multiplied instead of added. Two white view with opacity=0.5 on top of each other isn't equivalent to one white view with opacity=1.

opacity

@codecov-io
Copy link

codecov-io commented Oct 16, 2019

Codecov Report

Merging #127 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #127   +/-   ##
=======================================
  Coverage   93.47%   93.47%           
=======================================
  Files          36       36           
  Lines         736      736           
  Branches      189      182    -7     
=======================================
  Hits          688      688           
  Misses         40       40           
  Partials        8        8

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0127757...00574c1. Read the comment docs.

Copy link
Member

@satya164 satya164 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. I think changing backgroundStyle: { opacity } to backgroundStyle: { opacity: current.progress } would achieve the effect we're looking for while still cross-fading the text. Wdyt?

@@ -101,8 +101,8 @@ export function forFade({
}: StackHeaderInterpolationProps): StackHeaderInterpolatedStyle {
const progress = add(current.progress, next ? next.progress : 0);
const opacity = interpolate(progress, {
inputRange: [0, 1, 2],
outputRange: [0, 1, 0],
inputRange: [0, 1, 1.9999, 2],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reason to have 1.9999 instead of changing 0 to 1 in the outputRange?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is to hide the screen when the transition is finished. I can remove it if it's not necessary.

@leethree
Copy link
Contributor Author

@satya164 yeah it should achieve the same thing but I feel interpolate is more generic and easier to understand.

@satya164
Copy link
Member

It's not the same thing, changing background opacity only means the text still cross-fades. Also progress is a value ranging from 0 to 1 so interpolation is unnecessary for opacity.

@leethree
Copy link
Contributor Author

@satya164 I see what you mean. I updated the code.

@satya164 satya164 changed the title fix: header opacity for fade transition fix: don't fade incoming background when fading header Oct 16, 2019
@satya164 satya164 merged commit c6d0c19 into react-navigation:master Oct 16, 2019
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.

3 participants