-
Notifications
You must be signed in to change notification settings - Fork 9
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
feat: APP-360 redirect to project buy after google login #2494
Conversation
✅ Deploy Preview for regen-website ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
createProject: boolean = false, | ||
buyCreditsFrom?: string, |
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.
since we're adding more custom redirect routes, maybe we should just refactor this to take an optional redirectRoute
as param?
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.
Sounds good to me, passing redirectRoute
as a param definitely makes sense!
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 comment now should be:
<LoginFlow
isModalOpen={isModalOpen}
onModalClose={onModalClose}
wallets={walletsUiConfig}
modalState={modalState}
# New prop:
redirectRoute={`${projectHref}/buy`}
/>
deb922c
to
0cac65c
Compare
Maybe let's wait on #2477 to be merged before merging this |
@r41ph could you update your branch to latest dev and implement the remaining TODOs now that the main buy credits PR is merged? |
0cac65c
to
fb391eb
Compare
@r41ph could you update the testing instructions so @erikalogie can test this please? |
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.
createProject ? '?route=project-pages/draft/basic-info' : '' | ||
}`; | ||
window.location.href = `${apiUri}/marketplace/v1/auth/google | ||
${redirectRoute ? `?route=${redirectRoute}` : ''}`; |
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.
the issue above is because you have some spaces in the string
@erikalogie please see testing instructions |
b936a25
to
258a188
Compare
This looks to me like it is working |
I got redirected to the profile page |
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.
could you drop react-error-overlay in here as well so netlify previews work?
web-marketplace/src/components/organisms/LoginButton/hooks/useSocialProviders.ts
Outdated
Show resolved
Hide resolved
0f6819f
to
1b930a9
Compare
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
Just a side note that this will redirect to the dev app (hence losing progress info), because the redirect app url is set up on the server side and we use the same staging server for deploy previews and the dev app.
I could set it up temporarily to redirect to this deploy preview but I don't think it's super necessary, let me know
1b930a9
to
caa9533
Compare
Description
https://regennetwork.atlassian.net/browse/APP-360
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
How to test
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...