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

[C-3255] Sub-route logic & redirects for sign up flow #6489

Merged
merged 13 commits into from
Oct 28, 2023
12 changes: 9 additions & 3 deletions packages/web/src/pages/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,8 @@ import {
PURCHASES_PAGE,
SALES_PAGE,
WITHDRAWALS_PAGE,
EXPLORE_PREMIUM_TRACKS_PAGE
EXPLORE_PREMIUM_TRACKS_PAGE,
SIGN_UP_START_PAGE
} from 'utils/route'
import { getTheme as getSystemTheme } from 'utils/theme/theme'

Expand Down Expand Up @@ -179,7 +180,7 @@ import { WithdrawalsPage } from './purchases-and-sales/WithdrawalsPage'
import SettingsPage from './settings-page/SettingsPage'
import { SubPage } from './settings-page/components/mobile/SettingsPage'
import { SignInPage } from './sign-in-page'
import { SignUpRootPage } from './sign-up-page'
import { SignUpRootPage, SignUpRoute } from './sign-up-page'
import SmartCollectionPage from './smart-collection/SmartCollectionPage'
import SupportingPage from './supporting-page/SupportingPage'
import TopSupportersPage from './top-supporters-page/TopSupportersPage'
Expand Down Expand Up @@ -514,11 +515,16 @@ class App extends Component {
</Route>
<Route exact path={SIGN_UP_PAGE} isMobile={isMobileClient}>
{isSignInRedesignEnabled ? (
<SignUpRootPage />
<Redirect to={SIGN_UP_START_PAGE} />
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Keeps all the links the same (i.e. /signup will redirect to /signup/create-account)

) : (
<SignOn signIn={false} initialPage={initialPage} />
)}
</Route>
{isSignInRedesignEnabled ? (
<SignUpRoute path={SIGN_UP_PAGE} isMobile={isMobileClient}>
<SignUpRootPage />
</SignUpRoute>
) : null}
<Route
exact
path={FEED_PAGE}
Expand Down
170 changes: 119 additions & 51 deletions packages/web/src/pages/sign-up-page/SignUpRootPage.tsx
Original file line number Diff line number Diff line change
@@ -1,60 +1,128 @@
import { useState } from 'react'
import { useSelector } from 'react-redux'
import { Redirect, Route, RouteProps, Switch } from 'react-router-dom'

import { AppCtaPage, AppCtaState } from './pages/AppCtaPage'
import { getSignOn } from 'common/store/pages/signon/selectors'
import SignOnPageState from 'common/store/pages/signon/types'
import { AppState } from 'store/types'
import {
CreatePasswordPage,
CreatePasswordState
} from './pages/CreatePasswordPage'
import {
FinishProfilePage,
FinishProfileState
} from './pages/FinishProfilePage'
import { PickHandlePage, PickHandleState } from './pages/PickHandlePage'
import {
SelectArtistsPage,
SelectArtistsState
} from './pages/SelectArtistsPage'
import { SelectGenrePage, SelectGenreState } from './pages/SelectGenrePage'
import { SignUpPage, SignUpState } from './pages/SignUpPage'

type SignUpRootState =
| SignUpState
| CreatePasswordState
| PickHandleState
| FinishProfileState
| SelectGenreState
| SelectArtistsState
| AppCtaState
SIGN_UP_ARTISTS_PAGE,
SIGN_UP_EMAIL_PAGE,
SIGN_UP_FINISH_PROFILE_PAGE,
SIGN_UP_GENRES_PAGE,
SIGN_UP_HANDLE_PAGE,
SIGN_UP_PASSWORD_PAGE,
SignUpPath,
TRENDING_PAGE
} from 'utils/route'

export const SignUpRootPage = () => {
const [signUpState, setSignUpState] = useState<SignUpRootState>({
stage: 'sign-up'
})
import { CreatePasswordPage } from './pages/CreatePasswordPage'
import { FinishProfilePage } from './pages/FinishProfilePage'
import { PickHandlePage } from './pages/PickHandlePage'
import { SelectArtistsPage } from './pages/SelectArtistsPage'
import { SelectGenrePage } from './pages/SelectGenrePage'
import { SignUpPage } from './pages/SignUpPage'

/**
* Checks against existing sign up redux state,
* then determines if the requested path should be allowed or not
* if not allowed, also returns furthest step possible based on existing state
*/
const determineAllowedRoute = (
signUpState: SignOnPageState,
requestedRoute: string | SignUpPath // this string should have already trimmed out /signup/
) => {
const attemptedPath = requestedRoute.replace('/signup/', '')
// Have to type as string[] to avoid too narrow of a type for comparing against
let allowedRoutes: string[] = [SignUpPath.createEmail] // create email is available by default
if (signUpState.email.value) {
// Already have email
allowedRoutes.push(SignUpPath.createPassword)
}
if (signUpState.password.value) {
// Already have password
allowedRoutes.push(SignUpPath.pickHandle)
}
if (signUpState.handle.value) {
// Already have handle
allowedRoutes.push(SignUpPath.finishProfile)
}
if (signUpState.name.value) {
// Already have display name
// At this point the account is fully created & logged in; now user can't back to account creation steps
allowedRoutes = [SignUpPath.selectGenres]
}

// TODO: These checks below here may need to fall under a different route umbrella separate from sign up
if (signUpState.genres) {
// Already have genres selected
allowedRoutes.push(SignUpPath.selectArtists)
}

if (signUpState.followArtists?.selectedUserIds?.length >= 3) {
// Already have 3 artists followed
// Done with sign up if at this point so we return early (none of these routes are allowed anymore)
// TODO: trigger welcome modal when redirecting from here
return { isAllowedRoute: false, correctedRoute: TRENDING_PAGE }
}

const isAllowedRoute = allowedRoutes.includes(attemptedPath)
// If requested route is allowed return that, otherwise return the last step in the route stack
const correctedPath = isAllowedRoute
? attemptedPath
: allowedRoutes[allowedRoutes.length - 1]
return {
isAllowedRoute,
correctedRoute: `/signup/${correctedPath}`
}
}

/**
* <Route> wrapper that handles redirecting through the sign up page flow
*/
export function SignUpRoute({ children, ...rest }: RouteProps) {
const existingSignUpState = useSelector((state: AppState) => getSignOn(state))
return (
<Route
{...rest}
render={({ location }) => {
// Check if the route is allowed, if not we redirect accordingly
const { isAllowedRoute, correctedRoute } = determineAllowedRoute(
existingSignUpState,
location.pathname
)
return isAllowedRoute ? (
<>{children}</>
) : (
<Redirect to={correctedRoute} />
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, very nice

)
}}
/>
)
}

export const SignUpRootPage = () => {
return (
<div>
{signUpState.stage === 'sign-up' ? (
<SignUpPage onNext={setSignUpState} />
) : null}
{signUpState.stage === 'create-password' ? (
<CreatePasswordPage
onPrevious={setSignUpState}
onNext={setSignUpState}
/>
) : null}
{signUpState.stage === 'pick-handle' ? (
<PickHandlePage onNext={setSignUpState} />
) : null}
{signUpState.stage === 'finish-profile' ? (
<FinishProfilePage onNext={setSignUpState} />
) : null}
{signUpState.stage === 'select-genre' ? (
<SelectGenrePage onNext={setSignUpState} />
) : null}
{signUpState.stage === 'select-artists' ? (
<SelectArtistsPage onNext={setSignUpState} />
) : null}
{signUpState.stage === 'app-cta' ? <AppCtaPage /> : null}
<Switch>
<Route exact path={SIGN_UP_EMAIL_PAGE}>
Copy link
Contributor

Choose a reason for hiding this comment

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

my expectations was that this would be the "SignupRoute" and each one would check if they are allowed to render, and if not, redirect? i think its probably fine to do this all in a root route like you did though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah either way works technically.
I had moved it to the root since all the routes are using it; so I was thinking of it as a small possible optimization.
However, I moved it down to each route and don't see any difference so I think any real optimization is probably super negligible 🤷
Thinking about both I think I agree with you that wrapped around each route is probably a bit more readable since the logic is colocated so I'll keep it on the individual routes 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also I guess since we're probably rearranging it anyways, the individual wrappers makes it simpler to move around

<SignUpPage />
</Route>
<Route exact path={SIGN_UP_PASSWORD_PAGE}>
<CreatePasswordPage />
</Route>
<Route exact path={SIGN_UP_HANDLE_PAGE}>
<PickHandlePage />
</Route>
<Route exact path={SIGN_UP_FINISH_PROFILE_PAGE}>
<FinishProfilePage />
</Route>
<Route exact path={SIGN_UP_GENRES_PAGE}>
<SelectGenrePage />
</Route>
<Route exact path={SIGN_UP_ARTISTS_PAGE}>
<SelectArtistsPage />
</Route>
</Switch>
</div>
)
}
25 changes: 8 additions & 17 deletions packages/web/src/pages/sign-up-page/pages/CreatePasswordPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@ import { useDispatch } from 'react-redux'

import { setValueField } from 'common/store/pages/signon/actions'
import { TextField } from 'components/form-fields'

import { PickHandleState } from './PickHandlePage'
import { SignUpState } from './SignUpPage'
import { useNavigateToPage } from 'hooks/useNavigateToPage'
import { SIGN_UP_HANDLE_PAGE } from 'utils/route'

const messages = {
header: 'Create Your Password',
Expand All @@ -20,10 +19,6 @@ const messages = {
continue: 'Continue'
}

export type CreatePasswordState = {
stage: 'create-password'
}

const initialValues = {
password: '',
confirmPassword: ''
Expand All @@ -34,23 +29,19 @@ type CreatePasswordValues = {
confirmPassword: string
}

type CreatePasswordPageProps = {
onPrevious: (state: SignUpState) => void
onNext: (state: PickHandleState) => void
}

export const CreatePasswordPage = (props: CreatePasswordPageProps) => {
const { onNext } = props
const { email = '' } = {}
export const CreatePasswordPage = () => {
// TODO: PR #6443 replaces this logic
const { email } = { email: '' }
const dispatch = useDispatch()
const navigate = useNavigateToPage()

const handleSubmit = useCallback(
(values: CreatePasswordValues) => {
const { password } = values
dispatch(setValueField('password', password))
onNext({ stage: 'pick-handle' })
navigate(SIGN_UP_HANDLE_PAGE)
},
[dispatch, onNext]
[dispatch, navigate]
)

return (
Expand Down
20 changes: 6 additions & 14 deletions packages/web/src/pages/sign-up-page/pages/FinishProfilePage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@ import { useDispatch } from 'react-redux'

import { setValueField } from 'common/store/pages/signon/actions'
import { TextField } from 'components/form-fields'
import { useNavigateToPage } from 'hooks/useNavigateToPage'
import { SIGN_UP_GENRES_PAGE } from 'utils/route'

import { CoverPhotoField } from '../components/CoverPhotoField'
import { ProfilePictureField } from '../components/ProfilePictureField'

import { SelectGenreState } from './SelectGenrePage'

const messages = {
header: 'Finish Your Profile',
description:
Expand All @@ -21,14 +21,6 @@ const messages = {
continue: 'Continue'
}

export type FinishProfileState = {
stage: 'finish-profile'
}

type FinishProfilePageProps = {
onNext: (state: SelectGenreState) => void
}

type FinishProfileValues = {
profile_picture: Nullable<{ file: File; url: string }>
cover_photo: Nullable<{ file: File; url: string }>
Expand All @@ -41,17 +33,17 @@ const initialValues = {
displayName: ''
}

export const FinishProfilePage = (props: FinishProfilePageProps) => {
const { onNext } = props
export const FinishProfilePage = () => {
const dispatch = useDispatch()
const navigate = useNavigateToPage()

const handleSubmit = useCallback(
(values: FinishProfileValues) => {
const { displayName } = values
dispatch(setValueField('name', displayName))
onNext({ stage: 'select-genre' })
navigate(SIGN_UP_GENRES_PAGE)
},
[dispatch, onNext]
[dispatch, navigate]
)

return (
Expand Down
20 changes: 6 additions & 14 deletions packages/web/src/pages/sign-up-page/pages/PickHandlePage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import { useDispatch } from 'react-redux'

import { setValueField } from 'common/store/pages/signon/actions'
import { TextField } from 'components/form-fields'

import { FinishProfileState } from './FinishProfilePage'
import { useNavigateToPage } from 'hooks/useNavigateToPage'
import { SIGN_UP_FINISH_PROFILE_PAGE } from 'utils/route'

const messages = {
header: 'Pick Your Handle',
Expand All @@ -17,10 +17,6 @@ const messages = {
continue: 'Continue'
}

export type PickHandleState = {
stage: 'pick-handle'
}

const initialValues = {
handle: ''
}
Expand All @@ -29,21 +25,17 @@ type PickHandleValues = {
handle: string
}

type PickHandlePageProps = {
onNext: (state: FinishProfileState) => void
}

export const PickHandlePage = (props: PickHandlePageProps) => {
const { onNext } = props
export const PickHandlePage = () => {
const dispatch = useDispatch()
const navigate = useNavigateToPage()

const handleSubmit = useCallback(
(values: PickHandleValues) => {
const { handle } = values
dispatch(setValueField('handle', handle))
onNext({ stage: 'finish-profile' })
navigate(SIGN_UP_FINISH_PROFILE_PAGE)
},
[dispatch, onNext]
[dispatch, navigate]
)

return (
Expand Down
Loading