Skip to content
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

Native Sign Up Various QA Fixes #7607

Merged
merged 9 commits into from
Feb 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion packages/mobile/src/components/fields/PasswordField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ export const PasswordField = (props: PasswordFieldProps) => {
<PasswordInput
{...field}
error={hasError}
helperText={hasError ? error : undefined}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Matches pattern on web where we dont show any text since the completion checks handle that

onChangeText={(e) => {
if (clearErrorOnChange) {
setError(undefined)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@ import type { CornerRadiusOptions } from '@audius/harmony-native'

import { useTheme } from '../../foundations/theme'

type CoverPhotoImage = Image | ImageSourcePropType | null | undefined

export type CoverPhotoProps = {
profilePicture?: Image | ImageSourcePropType | null | undefined
coverPhoto?: Image | ImageSourcePropType | null | undefined
profilePicture?: CoverPhotoImage
Copy link
Contributor

Choose a reason for hiding this comment

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

a little weird calling it CoverPhotoImage, but i guess i get it :)

coverPhoto?: CoverPhotoImage
style?: StyleProp<ImageStyle>
children?: ReactNode
topCornerRadius?: CornerRadiusOptions
Expand Down Expand Up @@ -51,20 +53,38 @@ export const CoverPhoto = (props: CoverPhotoProps) => {
const fullHeightStyle = css({ height: '100%' })

const getSource = () => {
// Having .url means its a useable image source
if (coverPhoto && !isEmpty(coverPhoto)) {
return { source: coverPhoto }
let source: Exclude<CoverPhotoImage, number> = {
Copy link
Contributor

Choose a reason for hiding this comment

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

thanks a ton for improving this

uri: undefined
}
let usingProfilePicture = false
if (typeof source === 'number') {
return { source, usingProfilePicture }
}
if (profilePicture && !isEmpty(profilePicture)) {
return { source: profilePicture, usingProfilePicture: true }
source = profilePicture as Exclude<CoverPhotoImage, number>
usingProfilePicture = true
}
if (coverPhoto && !isEmpty(coverPhoto)) {
source = coverPhoto as Exclude<CoverPhotoImage, number>
usingProfilePicture = false
}

// Android upload format does not quite match the expected format, so we have to drill into 'file' to workaround for android
if (source && 'file' in source && !('uri' in source)) {
return { source: source.file, usingProfilePicture }
} else {
return { source, usingProfilePicture }
}
return { source: { uri: undefined } }
}

const { source, usingProfilePicture } = getSource()

return (
<ImageBackground source={source} style={[rootStyle, style]} {...other}>
<ImageBackground
source={source as ImageSourcePropType}
style={[rootStyle, style]}
{...other}
>
{!profilePicture && !coverPhoto ? (
<LinearGradient
colors={['rgba(0, 0, 0, 0.20)', 'rgba(0, 0, 0, 0.00)']}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -306,15 +306,9 @@ export const TextInput = forwardRef(
direction='row'
alignItems='center'
justifyContent='space-between'
gap='2xs'
>
{startAdornmentText && shouldShowAdornments ? (
<Text
variant='label'
size='l'
color='subdued'
style={css({ lineHeight: 0 })}
>
<Text variant='title' size='l' color='subdued'>
{startAdornmentText}
</Text>
) : null}
Expand Down Expand Up @@ -352,12 +346,7 @@ export const TextInput = forwardRef(
{...other}
/>
{endAdornmentText && shouldShowAdornments ? (
<Text
variant='label'
size='l'
color='subdued'
style={css({ lineHeight: 0 })}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The lineHeight was making the text dissappear on Android

>
<Text variant='label' size='l' color='subdued'>
{endAdornmentText}
</Text>
) : null}
Expand Down
9 changes: 8 additions & 1 deletion packages/mobile/src/screens/root-screen/RootScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { useDispatch, useSelector } from 'react-redux'

import useAppState from 'app/hooks/useAppState'
import { useDrawer } from 'app/hooks/useDrawer'
import { useNavigation } from 'app/hooks/useNavigation'
import { useFeatureFlag } from 'app/hooks/useRemoteConfig'
import { useUpdateRequired } from 'app/hooks/useUpdateRequired'
import { useSyncCodePush } from 'app/screens/root-screen/useSyncCodePush'
Expand Down Expand Up @@ -66,6 +67,7 @@ export const RootScreen = () => {
const { isEnabled: isSignUpRedesignEnabled } = useFeatureFlag(
FeatureFlags.SIGN_UP_REDESIGN
)
const { navigate } = useNavigation()

const { onOpen: openWelcomeDrawer } = useDrawer('Welcome')

Expand Down Expand Up @@ -107,14 +109,19 @@ export const RootScreen = () => {
if (showHomeStack && startedSignUp && !welcomeModalShown) {
openWelcomeDrawer()
setWelcomeModalShown(true)
// On iOS this will auto-navigate when we un-render sign up but on Android we have to navigate intentionally
if (navigate) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Kept getting left on a blank page on Android 💀

Copy link
Contributor

Choose a reason for hiding this comment

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

odd, sg!

navigate('HomeStack')
}
}
}
}, [
isSignUpRedesignEnabled,
openWelcomeDrawer,
showHomeStack,
startedSignUp,
welcomeModalShown
welcomeModalShown,
navigate
])

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ export const Page = (props: PageProps) => {
style={[
css({
zIndex: 1,
minHeight:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

From what I can tell, Android seems to shrink height: 100% with the keyboard, whereas iOS doesnt?

Copy link
Contributor

Choose a reason for hiding this comment

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

that sounds right, unfortunate

Dimensions.get('window').height - insets.top - insets.bottom,
paddingBottom: insets.bottom
}),
style
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useCallback, useState } from 'react'
import { useCallback } from 'react'

import { createPasswordPageMessages } from '@audius/common/messages'
import { passwordSchema } from '@audius/common/schemas'
Expand All @@ -8,7 +8,6 @@ import { useDispatch } from 'react-redux'
import { toFormikValidationSchema } from 'zod-formik-adapter'

import { Flex } from '@audius/harmony-native'
import { KeyboardAvoidingView } from 'app/components/core'
import { PasswordField } from 'app/components/fields'
import { useNavigation } from 'app/hooks/useNavigation'

Expand Down Expand Up @@ -49,50 +48,36 @@ export const CreatePasswordScreen = () => {
[dispatch, navigation]
)

const [keyboardOffset, setKeyboardOffset] = useState(0)

return (
<Formik
initialValues={initialValues}
onSubmit={handleSubmit}
validationSchema={passwordFormikSchema}
>
<KeyboardAvoidingView
keyboardShowingOffset={keyboardOffset - 32}
style={{ overflow: 'hidden' }}
>
<Page>
<Heading
heading={createPasswordPageMessages.createYourPassword}
description={createPasswordPageMessages.description}
<Page>
<Heading
heading={createPasswordPageMessages.createYourPassword}
description={createPasswordPageMessages.description}
/>
<Flex direction='column' h='100%' gap='l'>
<ReadOnlyField
label={createPasswordPageMessages.yourEmail}
value={email}
/>
<PasswordField
name='password'
clearErrorOnChange={false}
label={createPasswordPageMessages.passwordLabel}
/>
<PasswordField
name='confirmPassword'
clearErrorOnChange={false}
label={createPasswordPageMessages.confirmPasswordLabel}
/>
<Flex
direction='column'
h='100%'
gap='l'
onLayout={(e) => {
e.currentTarget.measureInWindow((x, y, w, h) => {
setKeyboardOffset(y)
})
}}
>
<ReadOnlyField
label={createPasswordPageMessages.yourEmail}
value={email}
/>
<PasswordField
name='password'
label={createPasswordPageMessages.passwordLabel}
/>
<PasswordField
name='confirmPassword'
label={createPasswordPageMessages.confirmPasswordLabel}
/>
<PasswordCompletionChecklist />
</Flex>
<PageFooter prefix={<SignUpAgreementText />} />
</Page>
</KeyboardAvoidingView>
<PasswordCompletionChecklist />
</Flex>
<PageFooter prefix={<SignUpAgreementText />} />
</Page>
</Formik>
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ export const FollowArtistCard = (props: FollowArtistCardProps) => {
</Pressable>
</Flex>
<Flex pt='unit12' ph='s' pb='l' alignItems='center' gap='l'>
<Flex gap='s'>
<Flex gap='s' alignItems='center'>
<UserDisplayName userId={user_id} />
<Flex direction='row' gap='s' alignItems='center'>
<Flex direction='row' gap='xs' alignItems='center'>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import { useCallback } from 'react'

import { selectArtistsPageMessages } from '@audius/common/messages'
import { finishSignUp } from 'audius-client/src/common/store/pages/signon/actions'
import {
addFollowArtists,
finishSignUp,
completeFollowArtists
} from 'audius-client/src/common/store/pages/signon/actions'
import { EditingStatus } from 'audius-client/src/common/store/pages/signon/types'
import {
getFollowIds,
Expand Down Expand Up @@ -55,12 +59,15 @@ export const SelectArtistsScreen = () => {
)

const handleSubmit = useCallback(() => {
// Follow selected artists
dispatch(addFollowArtists(selectedArtists))
dispatch(completeFollowArtists())
// This call is what eventually triggers the RootScreen to redirect to the home page (via conditional rendering)
dispatch(finishSignUp())
if (accountCreationStatus === EditingStatus.LOADING) {
navigation.navigate('AccountLoading')
}
}, [accountCreationStatus, dispatch, navigation])
}, [accountCreationStatus, dispatch, navigation, selectedArtists])

return (
<SelectArtistsPreviewContextProvider>
Expand Down
6 changes: 4 additions & 2 deletions packages/web/src/common/store/pages/signon/sagas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -514,8 +514,10 @@ function* signUp() {
FeatureFlags.SIGN_UP_REDESIGN
)

if (isNativeMobile && !isSignUpRedesignEnabled) {
yield* put(requestPushNotificationPermissions())
if (isNativeMobile) {
if (!isSignUpRedesignEnabled) {
yield* put(requestPushNotificationPermissions())
}
Comment on lines +517 to +520
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was the thing causing crashes on android (the else triggers window.localstorage)

Copy link
Contributor

Choose a reason for hiding this comment

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

yes great find!

} else {
// Set the has request browser permission to true as the signon provider will open it
setHasRequestedBrowserPermission()
Expand Down