Skip to content

Conversation

@ShreyaLaheri
Copy link

No description provided.

@CLAassistant
Copy link

CLAassistant commented Jul 10, 2020

CLA assistant check
All committers have signed the CLA.

package.json Outdated
"next-react-svg": "^1.1.1",
"react": "16.13.1",
"react-dom": "16.13.1",
"spectre.css": "^0.5.8",
Copy link
Contributor

Choose a reason for hiding this comment

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

Why have you installed spectre.css via npm? Uninstall it.

<label class="form-label">Email</label>
<input class="form-input" type="text" placeholder="Type your email address" />
</div>
<div class="form-group">
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure we can use class in React? We use className, right?

@@ -0,0 +1,116 @@
.loginBg{
Copy link
Contributor

Choose a reason for hiding this comment

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

Will the styling of all pages be inside this file?

Copy link
Author

Choose a reason for hiding this comment

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

Yes. I've left all the global styles in the global.css file, rest all in this file. However Ketaki hasn't done the same

@HeyItsJs
Copy link
Contributor

Few more changes after seeing the ui @ShreyaLaheri:

  • The signup/login buttons should have had 100% with as per your design.
  • The Already have an account? Login! text is not centre-aligned horizontally. Same with the text in the login page
  • Also, don't you think the login and signup pages should have been on the /login and /signup routes respectively instead of /login/login-page and /signup/signup-page? 😟 You can make your login page pages/login/index.js. Next.js automatically handles that index page properly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants