Skip to content

Add Onboarding page and redirect from login via auth-redirect#82

Merged
MainlyColors merged 12 commits into
mainfrom
#76-Create-OnBoarding-Page-and-Skip-Functionality
Apr 6, 2023
Merged

Add Onboarding page and redirect from login via auth-redirect#82
MainlyColors merged 12 commits into
mainfrom
#76-Create-OnBoarding-Page-and-Skip-Functionality

Conversation

@MainlyColors

@MainlyColors MainlyColors commented Apr 4, 2023

Copy link
Copy Markdown
Collaborator

Closes #76

Description

  • add redirect from /auth-redirect to /onboarding instead of the previous /
  • Add Onboarding page with skip button
  • skip button takes you back to / (temporary till /directory exists )

Testing

To Manually test

  1. npm run dev:server w/ docker already running
  2. Login on / route
  3. Authorize Discord
  4. Will be redirected from oAuth page to /auth-redirect
  5. On /auth-redirect route after verifying the user is authorized via a router.query.code existing, page will redirect to /onboarding instead of the previous /
  6. verify you see the onboarding page
  7. click skip
  8. will be redirected to home page with this url / (temporary till /directory exists )
  9. testing done

Type of change

  • New feature (non-breaking change which adds functionality)

Screenshots

/onboarding/
image

Checklist:

  • Changes have new/updated automated tests, if applicable
  • Changes have new/updated docs, if applicable
  • I have performed a self-review of my own code
  • I have added comments on any new, hard-to-understand code
  • Changes have been manually tested
  • Changes generate no new errors or warnings

Comment thread pages/index.tsx Outdated

@timmyichen timmyichen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

From the created ticket:

Use MUI to create a new page where users are redirected to, after they log in.
Create a dismiss button that will redirect users to the dev-directory list of profiles page.

I took this to mean:

  • When the user logs in, instead of redirecting to the index page, we send them to onboarding page
  • When the user skips onboarding, they get sent to the home page (i left a comment on #59 around changing where the actual directory lives)
  • If they refresh while on the home page, they are not prompted to go through onboarding again

This PR does the following:

  • Anyone who lands on the home page, will be sent to the onboarding page
  • When the user skips onboarding, they get sent back to the home page
  • If they refresh while on the home page, they will be redirected to the onboarding page again

I think we may want to change this to be more like the former - when users log in via Discord, their code is validated on the auth-redirect page. This is probably where we want to be making changes related to redirecting to onboarding.

General nit: "onboarding" is one word, so any instances of "onBoarding" or "xOnBoarding" should probably just be changed to "onboarding" and "xOnboarding" respectively

Comment thread pages/index.tsx Outdated
Comment thread pages/index.tsx Outdated
Comment thread pages/index.tsx Outdated
Comment thread pages/onBoarding/index.tsx Outdated

@devonzara devonzara left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good, just does a bit more than asked. Also, JSX should default to double quotes. 👍🏼

Comment thread pages/onBoarding/index.tsx Outdated
Comment thread pages/index.tsx Outdated
Comment thread pages/index.tsx Outdated
Comment thread pages/index.tsx Outdated
@devonzara

Copy link
Copy Markdown
Collaborator

Oof, I wish the review page would update to show other reviewer comments as they post them like it does when there's a new commit... Tim hit on essentially all the same points; I could have been watching TV instead. 😅

@jmoldyvan

Copy link
Copy Markdown
Collaborator

I got caught up with some stuff, but I will look on my machine as soon as I can

@DevinCLane

Copy link
Copy Markdown
Collaborator

Also if we're switching to onboarding from onBoarding, let's update the directory name to /pages/onboarding instead of the current /pages/onBoarding

@MainlyColors

Copy link
Copy Markdown
Collaborator Author

Also if we're switching to onboarding from onBoarding, let's update the directory name to /pages/onboarding instead of the current /pages/onBoarding

Thought I did that before because it changed on my end but I guess there is a specific git way to do a rename lol

Fixed now

@devonzara devonzara left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

👍🏼 Few additional touch-ups.

Comment thread pages/onboarding/index.tsx Outdated
Comment thread pages/onboarding/index.tsx Outdated
@devonzara

Copy link
Copy Markdown
Collaborator

nit: might be worth editing the PR's title, but otherwise looks good. 👍🏼

@MainlyColors MainlyColors changed the title Add OnBoarding page and redirect from home page Add Onboarding page and redirect from login via auth-redirect Apr 6, 2023
@devonzara devonzara added the enhancement New feature or request label Apr 6, 2023

@timmyichen timmyichen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Mostly LGTM, just some small comments. Thanks for making the requested changes!

Comment thread pages/onboarding/index.tsx Outdated
Comment thread pages/onboarding/index.tsx
@MainlyColors MainlyColors merged commit 19da996 into main Apr 6, 2023
@MainlyColors MainlyColors deleted the #76-Create-OnBoarding-Page-and-Skip-Functionality branch April 6, 2023 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Create OnBoarding Page and Skip Functionality

5 participants