Feature signup login frontend #42

chris-alexiuk · 2021-06-15T13:36:29Z

What this PR does (required):

Adds styling for the Login/Signup pages as per the 'Sign up' mockup we received.

Screenshots / Videos (required):

Login page, I've cropped the remaining right hand side of the screen, as it is currently empty - the same is true of the screenshot of the Signup page. Let me know if I should've instead included the entire webpage!

Signup page

Any information needed to test this feature (required):

N/A

Any issues with the current functionality (optional):

Not exactly a problem, but: The mockup indicated that the login/signup page should be in a card centered in the middle of the screen. However, our login/signup pages will eventually be cast into the left container on the homepage

So, I've let the card fill the entire left-side container and style relative to the card (I believe I did that, anyway)

I also did change /client/themes/theme.ts, which may impact others' styling if they were referencing the 'primary' variable at all. However, it was discussed in Slack - and I believe this is better suited to our primary colour since it's the primary colour in the mockups!

Thanks for bearing with me, this is my first 'official' PR on git! :D

moffatethan · 2021-06-15T17:43:41Z

client/src/context/useAuthContext.tsx

+  /*
+  This code will automatically cause a redirect to the login page if
+  I attempt to navigate to the "/signup" route

+  Commenting out this functionality until it is implemented:
+
+  '''


Pushing up commented code to main is not a good practice. This sign up functionality actually still works, the issue lies with the <Link /> tag. We are using Material-UIs link tag, this is good for anchor tags however, anchor tags refresh the page which triggers the auth context to redo it's logic causing the issue.

To fix this, on the login page where it says "Register" make sure you utilize react-router-dom's Link component.

Thank you!

I have switched to the 'react-router-dom' Link and it works like a charm. That information is invaluable - thank you!

moffatethan · 2021-06-15T17:44:43Z

client/src/pages/Login/Login.tsx

+          {/* --Commented out until thew new authHeader is completed
+          <AuthHeader linkTo="/signup" asideText="Don't have an account?" btnText="Create account" />*/}


If this is related to the routing issue, same applies here. Just make sure the AuthHeader is using the react-router-dom Link component.

moffatethan · 2021-06-15T18:16:24Z

client/src/pages/SignUp/SignUp.tsx

+          {/* --Commented out until thew new authHeader is completed
+          <AuthHeader linkTo="/signup" asideText="Don't have an account?" btnText="Create account" />*/}


Same here with regards to react-router-dom Link component

I have un-commented all code, as requested!

moffatethan

Hey Chris! This is looking good. Left you some comments to address as for the layout, it's coming together.

bonnieli · 2021-06-15T19:23:19Z

client/src/pages/Login/Login.tsx

                </Typography>
              </Grid>
            </Grid>
            <LoginForm handleSubmit={handleSubmit} />
          </Box>
-          <Box p={1} alignSelf="center" />
+          <Box p={1} alignSelf="center" padding="0" />


what is this Box doing, is it just for added space? if so - i'd remove this and utilizing styling from sibling components to create this space

Hello!

The box was there from the original code, I have removed it from Login.tsx, as well as Signup.tsx!

bonnieli · 2021-06-15T19:24:29Z

client/src/pages/SignUp/SignUpForm/useStyles.ts

    padding: 10,
    width: 160,
    height: 56,
    borderRadius: theme.shape.borderRadius,
    marginTop: 49,
    fontSize: 16,
-    backgroundColor: '#3a8dff',
+    backgroundColor: '#primary',


what is #primary color here? is this a valid color? did you mean to use the theme's primary colour?

bonnieli · 2021-06-15T19:25:36Z

client/src/pages/Login/LoginForm/useStyles.ts

    width: 160,
    height: 56,
    borderRadius: theme.shape.borderRadius,
    marginTop: 49,
    fontSize: 16,
-    backgroundColor: '#3a8dff',
+    backgroundColor: 'primary',


is this actually changing to the string primary? what is primary colour?

As with :

"what is #primary color here? is this a valid color? did you mean to use the theme's primary colour?

Originally posted by @bonnieli in #42 (comment)"

I have fixed both of the references to our primary colour:

chris-alexiuk · 2021-06-15T19:52:00Z

I believe I have made the requested changes, let me know if anything else needs to be done - or if I missed anything!

chris-alexiuk · 2021-06-16T15:31:41Z

I have now completed most of the landing page, I believe.

frontpage-test.mp4

Please let me know what I can/should improve!

added development folder to .gitignore to store pictures/videos related to development

chris-alexiuk · 2021-06-16T17:47:22Z

Realized I should've made the landing page bar a component, and switching it to that!

moffatethan · 2021-06-16T19:26:34Z

.gitignore

+/development
+


What's this directory?

This is just where I'm placing things like screenshots/screencaps to post here to git, I can make it public if you want!

No need, just was curious what the folder was. Fine with me 👍🏻

moffatethan · 2021-06-16T20:07:10Z

client/src/components/LandingPageBar/useStyles.ts

+import { theme } from '../../themes/theme';
+
+const useStyles = makeStyles(() => ({
+  sitterLoginSignup: {
+    position: 'fixed',


MUI will actually pass the theme as an argument, I would utilize it there as opposed to importing the theme.

Example: makeStyles((theme: Theme) => ({ . . .}));

moffatethan · 2021-06-16T20:08:21Z

client/src/components/Logo/useStyles.ts

+import { theme } from '../../themes/theme';
+
+const useStyles = makeStyles(() => ({
+  logoBox: {


As mentioned above, MUI will pass the theme as an argument.

moffatethan · 2021-06-16T20:12:05Z

client/src/components/Logo/Logo.tsx

+    <Box className={classes.logoBox}>
+      <PetsIcon className={classes.logoIcon} />
+      <Typography className={classes.logoText}>LovingSitter.</Typography>
+    </Box>


We might not even need to do a Box and styles here. Refer to Grid

I have swapped this to Grid, and refactored some of the other styling to make it fit

moffatethan · 2021-06-16T20:14:37Z

client/src/pages/Login/Login.tsx

@@ -36,25 +42,45 @@ export default function Login(): JSX.Element {
    });
  };

+  //---CODE FOR ADDING A BUTTON FOR A DEMO USER: DEVELOPMENT ONLY


This comment isn't really necessary, if the code explains itself in this case it does. It's pretty clear to another developer what "handleDemoUser" means. For future, only add comments where further explanation and context is needed.

I have removed the comment!

moffatethan · 2021-06-16T20:17:44Z

client/src/pages/Login/Login.tsx

+          <Box alignSelf="center">
+            <Typography variant="button">
+              Not a member yet?
+              <span> </span>


Is this here to separate the text and the link? Could this be refactor to be a bit more cleaner?

I've modified this to use inline CSS to add a margin to the text

moffatethan · 2021-06-16T20:19:33Z

client/src/pages/Login/LoginForm/useStyles.ts

@@ -1,33 +1,34 @@
 import { makeStyles } from '@material-ui/core/styles';
+import { SignalWifi1BarLockSharp } from '@material-ui/icons';


This isn't being used here

moffatethan · 2021-06-16T20:28:41Z

client/src/pages/SignUp/SignUp.tsx

@@ -37,25 +42,45 @@ export default function Register(): JSX.Element {
    });
  };

+  //---CODE FOR ADDING A BUTTON FOR A DEMO USER: DEVELOPMENT ONLY


As mentioned before, this comment isn't necessary.

moffatethan · 2021-06-16T20:29:08Z

client/src/pages/SignUp/useStyles.ts

 const useStyles = makeStyles(() => ({
  root: {


Theme is passed as argument here as well

moffatethan

I'm going to approve this and left you some cleanup comments to address. After that this should be good to go. Good work on this!

chris-alexiuk added 6 commits June 14, 2021 17:56

restyled login/signup pages to look like mockup

a932c2c

corrected button text on /signup

a5b06f6

improved the login/signup swap text per the mockup

5628f49

corrected authWrapper styling

9aa75a7

reverted testing changes

1b4904e

recommenting/explaining for PR

51a6a68

chris-alexiuk requested a review from bonnieli June 15, 2021 13:37

chris-alexiuk linked an issue Jun 15, 2021 that may be closed by this pull request

FE: Signup / Login Pages #21

Closed

moffatethan reviewed Jun 15, 2021

View reviewed changes

moffatethan suggested changes Jun 15, 2021

View reviewed changes

bonnieli reviewed Jun 15, 2021

View reviewed changes

chris-alexiuk added 2 commits June 15, 2021 15:44

made fixes based off of PR feedback

38ffe9a

made changes based off of PR reviews

425a0c7

chris-alexiuk added 3 commits June 15, 2021 17:41

added demo user button

56d4a8f

working on completing the front page

7590575

completed changed to the front page

37a70f2

Update .gitignore

2ac7e15

added development folder to .gitignore to store pictures/videos related to development

chris-alexiuk linked an issue Jun 16, 2021 that may be closed by this pull request

FE: Demo User #13

Closed

chris-alexiuk added 3 commits June 16, 2021 14:02

restarting due to pc issues, commit to backup

47f2a0f

restarting PC - commit to backup

9846726

finished componentizing the LandingPageBar

c14b533

moffatethan reviewed Jun 16, 2021

View reviewed changes

added logo component and added to front page

c737780

moffatethan reviewed Jun 16, 2021

View reviewed changes

moffatethan approved these changes Jun 16, 2021

View reviewed changes

chris-alexiuk added 3 commits June 16, 2021 17:37

modified to use theme as argument

0146ec6

removed extraneous comments

7084045

changed '<span> </span>' to space with inline CSS

c7fd62d

chris-alexiuk merged commit 1cfc4b9 into main Jun 16, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature signup login frontend #42

Feature signup login frontend #42

chris-alexiuk commented Jun 15, 2021

moffatethan Jun 15, 2021 •

edited

Loading

chris-alexiuk Jun 15, 2021

moffatethan Jun 15, 2021

moffatethan Jun 15, 2021

chris-alexiuk Jun 15, 2021

moffatethan left a comment

bonnieli Jun 15, 2021

chris-alexiuk Jun 15, 2021

bonnieli Jun 15, 2021

bonnieli Jun 15, 2021

chris-alexiuk Jun 15, 2021 •

edited

Loading

chris-alexiuk commented Jun 15, 2021

chris-alexiuk commented Jun 16, 2021 •

edited

Loading

chris-alexiuk commented Jun 16, 2021

moffatethan Jun 16, 2021

chris-alexiuk Jun 16, 2021

moffatethan Jun 16, 2021

moffatethan Jun 16, 2021

moffatethan Jun 16, 2021

moffatethan Jun 16, 2021

chris-alexiuk Jun 16, 2021

moffatethan Jun 16, 2021

chris-alexiuk Jun 16, 2021

moffatethan Jun 16, 2021

chris-alexiuk Jun 16, 2021

moffatethan Jun 16, 2021

moffatethan Jun 16, 2021

moffatethan Jun 16, 2021

moffatethan left a comment

		{/* --Commented out until thew new authHeader is completed
		<AuthHeader linkTo="/signup" asideText="Don't have an account?" btnText="Create account" />*/}

		@@ -1,33 +1,34 @@
		import { makeStyles } from '@material-ui/core/styles';
		import { SignalWifi1BarLockSharp } from '@material-ui/icons';

		/development

Feature signup login frontend #42

Feature signup login frontend #42

Conversation

chris-alexiuk commented Jun 15, 2021

What this PR does (required):

Screenshots / Videos (required):

Any information needed to test this feature (required):

Any issues with the current functionality (optional):

moffatethan Jun 15, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

moffatethan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chris-alexiuk Jun 15, 2021 • edited Loading

Choose a reason for hiding this comment

chris-alexiuk commented Jun 15, 2021

chris-alexiuk commented Jun 16, 2021 • edited Loading

chris-alexiuk commented Jun 16, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

moffatethan left a comment

Choose a reason for hiding this comment

moffatethan Jun 15, 2021 •

edited

Loading

chris-alexiuk Jun 15, 2021 •

edited

Loading

chris-alexiuk commented Jun 16, 2021 •

edited

Loading