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

SignUp Page #2148

Merged
merged 25 commits into from
Apr 18, 2021
Merged

SignUp Page #2148

merged 25 commits into from
Apr 18, 2021

Conversation

PedroFonsecaDEV
Copy link
Contributor

Issue This PR Addresses

Fixes #1643
Co-authored-by: @PedroFonsecaDEV
Co-authored-by: @Meneguini
Co-authored-by: @DukeManh

Type of Change

  • Bugfix: Change which fixes an issue
  • New Feature: Change which adds functionality
  • Documentation Update: Change which improves documentation
  • UI: Change which improves UI

Description

This PR adds the signup page. The style and the design still need to be polished, but we believe that we need to land this feature on our staging serv so we can test with our users (us).
The styles of the page will be fixed in follow-ups.

Checklist

  • Quality: This PR builds and passes our npm test and works locally
  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not (if applicable)
  • Documentation: This PR includes updated/added documentation to user exposed functionality or configuration variables are added/changed or an explanation of why it does not(if applicable)

@humphd
Copy link
Contributor

humphd commented Apr 14, 2021

Tagging some more reviewers. I'd like to prioritize this PR so we can move toward getting the signup flow working in the front-end soon, and have time to fix bugs before 2.0.

Copy link

@huyxgit huyxgit left a comment

Choose a reason for hiding this comment

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

is it a page but looks like a dialog?
I think we should give user a chance to go back to homepage other than clicking on "back" button.

@PedroFonsecaDEV
Copy link
Contributor Author

is it a page but looks like a dialog?
I think we should give user a chance to go back to homepage other than clicking on "back" button.

@huynguyez
Follow ups. We are landing the functionality.

Copy link
Contributor

@yuanLeeMidori yuanLeeMidori left a comment

Choose a reason for hiding this comment

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

image

I assume this is part of the sign-up and there will be more follow-up PRs, but should the "Login" button be "Next"? Or, should there be a "Next" button based on the instruction in this page? Or, it'll be in the upcoming PRs?

Great design as usual!

@DukeManh
Copy link
Contributor

@yuanLeeMidori, you are right, Pedro will update it. More PRs are coming but feel free to comment on anything.

Copy link
Contributor

@HyperTHD HyperTHD left a comment

Choose a reason for hiding this comment

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

Don't forget to add the USERS_URL and the FEED_DISCOVERY_URL as args in both production.yml and the Dockerfile in the root directory, otherwise, you won't be able to test this in staging and production.

yuanLeeMidori
yuanLeeMidori previously approved these changes Apr 15, 2021
Copy link
Contributor

@humphd humphd left a comment

Choose a reason for hiding this comment

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

This is so awesome to see, thank you to all of you for pushing on it. I've tested it, and it's generally working. There are things I'd like to improve about wording, UI, but they don't need to block this. I left a few notes about issues, questions, and areas for follow-up.

src/web/next.config.js Show resolved Hide resolved
src/web/src/components/BannerButtons.tsx Show resolved Hide resolved
src/web/src/components/SignUp/Forms/GitHubAccount.tsx Outdated Show resolved Hide resolved
src/web/src/components/SignUp/Forms/GitHubAccount.tsx Outdated Show resolved Hide resolved
src/web/src/components/SignUp/Forms/RSSFeeds.tsx Outdated Show resolved Hide resolved
src/web/src/components/SignUp/Schema/FormSchema.tsx Outdated Show resolved Hide resolved
src/web/src/pages/signup.tsx Outdated Show resolved Hide resolved
src/web/src/pages/signup.tsx Outdated Show resolved Hide resolved
src/web/src/pages/signup.tsx Outdated Show resolved Hide resolved
@DukeManh
Copy link
Contributor

@humphd Thanks for the review. I left some comments.

@humphd
Copy link
Contributor

humphd commented Apr 15, 2021

This needs a rebase.

@huyxgit huyxgit self-requested a review April 18, 2021 17:28
@huyxgit
Copy link

huyxgit commented Apr 18, 2021

kinda broken CSS in small screen.

@PedroFonsecaDEV
Copy link
Contributor Author

PedroFonsecaDEV commented Apr 18, 2021

kinda broken CSS in small screen.

@huynguyez
I need this to land to work on the CSS.
#2171 is waiting for this PR to land.

@PedroFonsecaDEV PedroFonsecaDEV merged commit 0299b0d into Seneca-CDOT:master Apr 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve Telescope sign-up flow
7 participants