New app by GormanGit · Pull Request #18 · GormanGit/javascript-workbook

GormanGit · 2017-12-04T16:54:20Z

Checkpoint Rubric

This is the rubric that your instructor will use to grade your checkpoints. Please do not edit.

Checkpoint 1

All tests passed: 40 points
Proper use of documentation (commenting on code): 15 points
Properly indented code: 15 points
Demonstrated effective use of JavaScript: 30 points

Checkpoint 2

The application works as it should: 40 points
Proper use of documentation (commenting on code): 15 points
Properly indented code: 15 points
Demonstrated effective use of JavaScript and the DOM API: 30 points

Checkpoint 3

Use of React: 25 points
Accesses an API: 25 points
Proper use of documentation (commenting on code): 25 points
The application functions as it should: 25 points

stu-k

Nice use of the Router! There was a little issue with the inputs on the login page because they weren't being set properly, but I made a note about that. Otherwise, you HAVE to make your code look nicer. Logically, this is fine code. Stylistically, this was a mess. React doesn't see whitespace, so having empty lines within JSX is useless. You're still using both 2 and 4 spaces, which on its own is a big no-no, but I also found lines with an ODD number of spaces. Fix the login page thing and all the unnecessary whitespace/horrible indentation and I'll give you a better grade.

stu-k · 2017-12-12T16:45:19Z

08week/newapp/src/components/pages/HomePage.js

+import RaisedButton from 'material-ui/RaisedButton';
+
+
+const HomePage = () => (


What's with the horrible indentation?

stu-k · 2017-12-12T16:46:23Z

08week/newapp/src/components/pages/LoginPage.js

@@ -0,0 +1,16 @@
+import React from 'react';
+import LoginForm from '../forms/LoginForm';
+


Unnecessary whitespace.

stu-k · 2017-12-12T16:46:57Z

08week/newapp/src/components/pages/LoginPage.js

+
+const LoginPage = () => (
+    <div>
+    <h1>Login page</h1>


Indentation.

stu-k · 2017-12-12T16:48:30Z

08week/newapp/src/components/pages/LoginPage.js

+    <h1>Login page</h1>
+
+    <LoginForm  />
+


White space.

stu-k · 2017-12-12T16:51:45Z

08week/newapp/src/components/forms/LoginForm.js

+                    value={data.password}
+                    onChange={this.onChange}
+                /><br />
+


White space.

stu-k · 2017-12-12T16:52:06Z

08week/newapp/src/components/forms/LoginForm.js

+
+                <br />
+
+            </Form>


Should be in-line with top <Form>

stu-k · 2017-12-12T16:53:01Z

08week/newapp/src/components/forms/LoginForm.js

+                    hintText="email"
+                    value={data.email}
+                    onChange={this.onChange}
+                /><br />


Would be easier to follow if the <br /> was on the next line.

stu-k · 2017-12-12T16:53:35Z

08week/newapp/src/components/forms/LoginForm.js

+        data: {
+          email: '',
+          password: ''
+    },


Indentation. This makes it look like state ends here.

stu-k · 2017-12-12T16:55:21Z

08week/newapp/src/components/forms/LoginForm.js

+
+
+class LoginForm extends React.Component {
+    state = {


Using both 4 spaces 2 here, don't.

stu-k · 2017-12-12T17:13:46Z

08week/newapp/src/components/forms/LoginForm.js

+    //in state change data, use spread to save everything that is in data, then what is changed
+    //which is an e.target
+    onChange = e => this.setState({
+        data: {...this.state.data, [e.target.name]: e.target.value}


e.target doesn't have a .name property, so you'll probably be unable to change the input values through typing in them. You could do e.target.dataset.name and then also give your TextFields a data name (<TextField data-name="email">) to achieve the same effect.

stu-k

Yay intentions! Now whitespace and fix your login page onChange.

stu-k · 2017-12-12T20:12:42Z

08week/newapp/src/App.js

+    return (
+      //  this is a semantic-ui-react also semantic-ui-css container that auto margins
+      <div className="ui container">
+


White space

stu-k · 2017-12-12T20:12:52Z

08week/newapp/src/components/forms/LoginForm.js

+              hintText="email"
+              value={data.email}
+              onChange={this.onChange}
+


White space

stu-k · 2017-12-12T20:13:03Z

08week/newapp/src/components/forms/LoginForm.js

+              value={data.email}
+              onChange={this.onChange}
+
+          /><br/>


<br /> On new line

stu-k · 2017-12-12T20:13:08Z

08week/newapp/src/components/forms/LoginForm.js

+              hintText="password"
+              value={data.password}
+              onChange={this.onChange}
+


White space

stu-k · 2017-12-12T20:13:21Z

08week/newapp/src/components/forms/LoginForm.js

+              value={data.password}
+              onChange={this.onChange}
+
+          /><br/>


<br /> on new line

stu-k · 2017-12-12T20:13:26Z

08week/newapp/src/components/forms/LoginForm.js

+          <RaisedButton primary={true}>
+            <Link to='main'>LOGIN</Link>
+          </RaisedButton>
+


White space

stu-k · 2017-12-12T20:13:34Z

08week/newapp/src/components/pages/HomePage.js

+      <h1>Home Page</h1>
+      <RaisedButton primary={true}>
+        <Link to='login'>LOGIN PAGE</Link>
+


White space

stu-k · 2017-12-12T20:13:40Z

08week/newapp/src/components/pages/MainPage.js

+const MainPage = () => (
+    <div>
+      <h1>Main page</h1>
+


White space

stu-k · 2017-12-12T20:13:44Z

08week/newapp/src/components/pages/MainPage.js

+
+      <img className="pic" src="http://www.simonstalenhag.se/bilderbig/by_procession_1920.jpg">
+      </img>
+


White space

stu-k · 2017-12-12T20:14:04Z

08week/newapp/src/components/forms/LoginForm.js

+//in state change data, use spread to save everything that is in data, then what is changed
+//which is an e.target
+  onChange = e => this.setState({
+    data: {...this.state.data, [e.target.name]: e.target.value}


Still not fixed

stu-k

Some instances of both 4 spaces and 2, but you get the idea!

Edit: DON'T MERGE. Your branches are weird enough as is.

jon-gorman added 6 commits November 29, 2017 21:52

first commit with working pages

08ccd99

semantic and material?

471de92

changes

be2c1cc

before class

821822b

added api from class

225dbf9

main page

ad25763

stu-k suggested changes Dec 12, 2017

View reviewed changes

jon-gorman added 3 commits December 12, 2017 13:22

Indentions!

7dc1503

white_space

69b5be2

white_space2

c5e51ae

stu-k suggested changes Dec 12, 2017

View reviewed changes

everything fixed

206a124

stu-k approved these changes Dec 13, 2017

View reviewed changes

		import RaisedButton from 'material-ui/RaisedButton';


		const HomePage = () => (

		@@ -0,0 +1,16 @@
		import React from 'react';
		import LoginForm from '../forms/LoginForm';


		<img className="pic" src="http://www.simonstalenhag.se/bilderbig/by_procession_1920.jpg">
		</img>

Conversation

GormanGit commented Dec 4, 2017

Checkpoint Rubric

Uh oh!

stu-k left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stu-k left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stu-k left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

stu-k left a comment •

edited

Loading