feat: design registration form pop up #5
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
syedsajjadkazmii
requested changes
Apr 1, 2024
There was a problem hiding this comment.
can you use the dir structure and messages names as in https://github.com/edx/frontend-component-authn-edx/pull/4/files? this is for consistency
attiyaIshaque
force-pushed
the
attiya/VAN-1815-registration-form
branch
2 times, most recently
from
04fbb67
to
5661a59
Compare
attiyaIshaque
force-pushed
the
attiya/VAN-1815-registration-form
branch
2 times, most recently
from
9c707df
to
d913ac1
Compare
attiyaIshaque
force-pushed
the
attiya/VAN-1815-registration-form
branch
3 times, most recently
from
0a6ce38
to
33c277c
Compare
attiyaIshaque
force-pushed
the
attiya/VAN-1815-registration-form
branch
from
33c277c
to
7f21f8c
Compare
syedsajjadkazmii
approved these changes
Apr 3, 2024
zainab-amir
reviewed
Apr 4, 2024
src/forms/register/index.jsx
Outdated
There was a problem hiding this comment.
Please change the file name to registration-popup
src/forms/register/index.jsx
Outdated
| import SocialAuthButtons from '../../common-ui/SocialAuthButtons'; | ||
| import './index.scss'; | ||
|
|
||
| const RegisterForm = () => { |
There was a problem hiding this comment.
Suggested change
| const RegisterForm = () => { | |
| const RegistrationForm = () => { |
src/authn-component/index.jsx
Outdated
Comment on lines
26
to
31
| <BaseContainer open={open} setOpen={setOpen}> | ||
| <LoginForm /> | ||
| </BaseContainer> | ||
| <BaseContainer open={open} setOpen={setOpen} footerText={registrationFooterText}> | ||
| <RegisterForm /> | ||
| </BaseContainer> |
There was a problem hiding this comment.
I think we decided to use a single BaseContainer in the standup.
src/authn-component/messages.jsx
Outdated
| footerText: { | ||
| id: 'footer.text', | ||
| defaultMessage: 'By creating an account, you agree to the Terms of Service and Honor Code and you acknowledge that edX and each Member process your personal data in accordance with the Privacy Policy.', | ||
| description: 'Text for footer', |
There was a problem hiding this comment.
Suggested change
| description: 'Text for footer', | |
| description: 'Text that appears on registration form stating honor code and privacy policy', |
src/forms/register/index.jsx
Outdated
| <Form.Group controlId="email" className="w-100 mb-0"> | ||
| <Form.Control | ||
| type="email" | ||
| floatingLabel={<span>{formatMessage(messages.registerFormEmailFieldLabel)}</span>} |
There was a problem hiding this comment.
Why do we need span around it?
src/forms/register/messages.jsx
Outdated
Comment on lines
34
to
38
| registerFormContinueButton: { | ||
| id: 'register.form.continue.button', | ||
| defaultMessage: 'Continue', | ||
| description: 'Text for submit button', | ||
| }, |
src/forms/register/messages.jsx
Outdated
| registerFormAlreadyHaveAccountText: { | ||
| id: 'register.form.already.have.account.text', | ||
| defaultMessage: 'Already have an account?', | ||
| description: 'Text for already have an account', |
There was a problem hiding this comment.
Suggested change
| description: 'Text for already have an account', | |
| description: 'Text for registration button', |
src/forms/register/messages.jsx
Outdated
| registerFormSignInLink: { | ||
| id: 'register.form.sign.in.link', | ||
| defaultMessage: 'Sign In', | ||
| description: 'Text for signing in', |
There was a problem hiding this comment.
Suggested change
| description: 'Text for signing in', | |
| description: 'Text for sign in link', |
src/forms/register/messages.jsx
Outdated
| registerFormAccountSchoolOrganizationText: { | ||
| id: 'register.form.account.school.organization.text', | ||
| defaultMessage: 'Have an account through school or organization?', | ||
| description: 'Text for having an account through school or organization', |
There was a problem hiding this comment.
Suggested change
| description: 'Text for having an account through school or organization', | |
| description: 'Label for link that leads learners to the institution login page', |
src/forms/register/messages.jsx
Outdated
| defaultMessage: 'Sign In', | ||
| description: 'Text for signing in', | ||
| }, | ||
| registerFormAccountSchoolOrganizationText: { |
There was a problem hiding this comment.
Suggested change
| registerFormAccountSchoolOrganizationText: { | |
| registrationFormSchoolOrOrganizationLink: { |
attiyaIshaque
force-pushed
the
attiya/VAN-1815-registration-form
branch
2 times, most recently
from
16605be
to
5c6c003
Compare
zainab-amir
approved these changes
Apr 4, 2024
src/authn-component/messages.jsx
Outdated
| defaultMessage: 'By creating an account, you agree to the Terms of Service and Honor Code and you acknowledge that edX and each Member process your personal data in accordance with the Privacy Policy.', | ||
| description: 'Text that appears on registration form stating honor code and privacy policy', | ||
| }, | ||
|
|
Comment on lines
4
to
13
| registrationFormHeading: { | ||
| id: 'registration.form.heading', | ||
| defaultMessage: 'Create account', | ||
| description: 'registration form main heading', | ||
| }, | ||
| registrationFormOrText: { | ||
| id: 'registration.form.or.message', | ||
| defaultMessage: 'or', | ||
| description: 'Heading that appears between social auth and basic registration form', | ||
| }, |
There was a problem hiding this comment.
Can we name similar to how we have named in Login form?
Suggested change
| registrationFormHeading: { | |
| id: 'registration.form.heading', | |
| defaultMessage: 'Create account', | |
| description: 'registration form main heading', | |
| }, | |
| registrationFormOrText: { | |
| id: 'registration.form.or.message', | |
| defaultMessage: 'or', | |
| description: 'Heading that appears between social auth and basic registration form', | |
| }, | |
| registrationFormHeading1: { | |
| id: 'registration.form.heading.1', | |
| defaultMessage: 'Create account', | |
| description: 'registration form main heading', | |
| }, | |
| registrationFormHeading2: { | |
| id: 'registration.form.heading.2', | |
| defaultMessage: 'or', | |
| description: 'Heading that appears between social auth and basic registration form', | |
| }, |
| registrationFormCreateAccountButton: { | ||
| id: 'registration.form.continue.button', | ||
| defaultMessage: 'Create an account for free', | ||
| description: 'Text for submit button', |
There was a problem hiding this comment.
Suggested change
| description: 'Text for submit button', | |
| description: 'Text for submit button on registration form', |
| registrationFormAlreadyHaveAccountText: { | ||
| id: 'registration.form.already.have.account.text', | ||
| defaultMessage: 'Already have an account?', | ||
| description: 'Text for registration button', |
There was a problem hiding this comment.
Suggested change
| description: 'Text for registration button', | |
| description: 'Login button help text', |
attiyaIshaque
force-pushed
the
attiya/VAN-1815-registration-form
branch
from
5c6c003
to
0f6b354
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
In this pull request, I've implemented the registration form in alignment with the Figma style guidelines. I've included components such as the social authentication button, dividers, links, and footer text as specified in the design.
JIRA
VAN-1815
How Has This Been Tested?
Tested locally
Screenshots/sandbox (optional):
Merge Checklist