-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CP-9398: Sign up and Sign back in Flow #2155
Conversation
@@ -144,6 +145,9 @@ dependencies { | |||
implementation fileTree(dir: "libs", include: ["*.jar"]) | |||
implementation 'com.google.firebase:firebase-messaging:24.0.1' | |||
|
|||
implementation 'com.github.bumptech.glide:glide:4.12.0' | |||
kapt 'com.github.bumptech.glide:compiler:4.12.0' | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is part of expo-image configuration.
import com.bumptech.glide.module.AppGlideModule | ||
|
||
@GlideModule | ||
class CustomGlideModule : AppGlideModule() {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we don't create this module, expo-image fails to load images. (source)
glassType={colorScheme === 'dark' ? 'dark' : 'light'} | ||
/> | ||
) | ||
return <GlassView style={{ flex: 1 }} /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you set the glassType, it applies a background color to the blur, which causes a color difference with the backgroundView. Therefore, we don’t use glassType for the blurView used as the background for the header and tab bar.
router.replace('/portfolio') | ||
} | ||
} | ||
}, [walletState, router, canGoBackToWallet, navigation]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here, the navigation logic corresponding to walletState is handled.
</View> | ||
position: 'absolute' | ||
}} | ||
/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’ve set up a blank screen at the top level. The reason is that when navigation occurs due to walletState, there’s a slight delay, causing the top screen to appear very briefly (hardly noticeable on iOS and for less than 500ms on Android). It would be great to eliminate the flicker entirely, but I haven’t found a way to do that yet. In the meantime, showing the blank screen during the flicker felt the most natural.
// workaround for images not appearing | ||
// https://github.com/DylanVann/react-native-fast-image/issues/974 | ||
fallback={true} | ||
<Image |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add this change given that we haven't figured out a way to fix the expo slowness issue? 👀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we can’t fix Expo’s Android performance issues, we’ll somehow need to roll back quite a few things besides expo-image
, like expo-router
. Maybe we need to decide whether to quickly address the performance issues or abandon Expo before moving further.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm. 🔥
Description
Ticket: CP-9398
Screenshots/Videos
Signup Flow
Simulator.Screen.Recording.-.iPhone.15.Pro.-.2024-12-12.at.16.19.30.mp4
Pin/Bio login flow
Simulator.Screen.Recording.-.iPhone.15.Pro.-.2024-12-12.at.16.23.41.mp4
Checklist