Refactor SceneWrapper, header, and footer #4721
Conversation
samholmes
force-pushed
the
sam/scene-footer
branch
from
0ed050f to
6be076c
Compare
| onPress={() => navigation.navigate('walletsTab', { screen: 'walletList' })} | ||
| alignSelf="center" | ||
| /> | ||
| <View style={styles.bottomButton}> |
There was a problem hiding this comment.
Rant: This is what Jon has been harping on - we should be using the ButtonsView to put a button on a scene in a particular layout, instead of centering it ourselves.
However, I see that this is a special case, since we are floating the button at the bottom of the screen. Since we are in a hurry, there is no need to go adding a special mode to the ButtonsView! Arguably this could even go in the scene footer area, but again, no need to do that in this PR.
There was a problem hiding this comment.
Ah. Good to know.
I also was inclined to revisit this and other scenes with floating components to instead render these components int he footer.
| return <BlurView blurType={theme.isDark ? 'dark' : 'light'} style={styles.blurView} overlayColor={overlayColor} /> | ||
| } | ||
|
|
||
| const getStyles = cacheStyles(() => ({ | ||
| blurView: { | ||
| ...StyleSheet.absoluteFillObject, | ||
| backgroundColor: isAndroid ? '#00000055' : undefined | ||
| } | ||
| })) |
There was a problem hiding this comment.
This is all wrong. By default, the BlurView looks consistent on both iOS and Android. However, Android lets you manually adjust the tint color using the overlayColor, so we ourselves started making Android look different. But now we don't like the inconsistency, so we tweak it back a different way!!!
What about just deleting the overlayColor prop and letting things automatically be the same on both platforms? This will also deal with the light-mode, which your current solution doesn't address.
There was a problem hiding this comment.
overlayColor applied on the whole screen on Android. That was the reason for the transparent value.
There was a problem hiding this comment.
Oh, interesting. So it's a bug-fix to the blur library!? In that case, we should document it as such.
samholmes
force-pushed
the
sam/scene-footer
branch
from
6be076c to
aac5013
Compare
samholmes
force-pushed
the
sam/scene-footer
branch
from
085a81e to
f32d4fe
Compare
The `insetStyles` should simply be a convenient set of padding styles derived from the `insets` without modification to the values.
Instead of long summations of terms with separate conditions inline, we factor out each term and its respective condition to make the logic easy to reason about and maintain.
A style is a set of rules. It was a mistake to name this property in the plural form.
We can use `avoidKeyboard` prop directly to know if we are in this mode in the `SceneWrapper`.
Some scenes don't register the scroll handler and therefore don't have local scroll state to stomp on the global scroll state. In this case, they'll be left with another scene's scroll state. To fix this, we set the scroll state back to its default scroll state when the scene is no longer in focus. We also fix a bug where local state never stomps on scrollXDelta and scrollYDelta.
samholmes
force-pushed
the
sam/scene-footer
branch
from
f32d4fe to
08079ca
Compare
This change makes for a more straight forward API where the scene component may always expect padding and scene wrapper size to be applied consistently. The scene is responsible for applying negative margins using `undoInsetStyle` if it wishes to overlap under UI components (header/tabs/etc).
We add a black transparent background on Android to BlurBackground for presentation consistency across platforms.
Before we were adding a minimum padding to the bottom of the MenuTabs. This clearly caused bugs. The new approach is to just use the bottom inset for the bottom padding in the MenuBar so that way the SceneFooter can include it in position calculations.
samholmes
force-pushed
the
sam/scene-footer
branch
from
08079ca to
0f193cf
Compare
We remove the overlayColor property from BlurBackground which is Android-only. Instead, we add a backgroundColor to the blur view to match the light/dark modes in iOS.
samholmes
force-pushed
the
sam/scene-footer
branch
from
0f193cf to
ecbd4b0
Compare
CHANGELOG
Does this branch warrant an entry to the CHANGELOG?
Dependencies
noneRequirements
If you have made any visual changes to the GUI. Make sure you have: