Es pja add new item #28

erostribe · 2020-04-18T05:27:48Z

For an example of how to fill this template out, see this Pull Request.

Description

Related Issue

Closes #6

Acceptance Criteria

User is presented with a form that allows them to enter the name of the item and select how soon they anticipate needing to buy it again (Soon, Kind of soon, Not soon)
When the user submits the form, the item is saved to the database, associated with the user’s token
Along with the item name, an integer corresponding to the estimated number of days until next purchase is saved: 7 for “Soon”, 14 for “Kind of soon”, and 30 for “Not soon”
The last purchased date should be set to null initially (i.e. you can create an item without purchasing it)

Type of Changes

	Type
	🐛 Bug fix
✓	✨ New feature
	🔨 Refactoring
	💯 Add tests
	🔗 Update dependencies
	📜 Docs

Updates

Before

After

Testing Steps / QA Criteria

Git pull the branch using git pull origin es-pja-add-new-item and git checkout es-pja-add-new-item to switch to the branch.
Run npm install to add dependencies, then npm start to run the app.
When the app runs in the browser, you will be presented with a page titled "Shopping List" with two buttons "Shopping" and "Add Item."
Clicking "Add Item" should present a form with a "Name" input, a Schedule option with a dropdown menu and 3 choices, and a Submit button.

skillitzimberg · 2020-04-18T20:16:15Z

src/components/AddItemForm.js

@@ -0,0 +1,71 @@
+import React from 'react';
+import { format } from 'prettier';


I think this prettier import might be what's causing the checks to fail. I'm not sure. I did run npm install and I still get these messages in the terminal. Commenting out this import fixed it.

./src/components/AddItemForm.js Line 2:10: 'format' is defined but never used no-unused-vars ./node_modules/prettier/parser-typescript.js Module not found: Can't resolve '@microsoft/typescript-etw' in '/Users/ForeignFood/Development/collab_lab/tcl-6-smart-shopping-list/node_modules/prettier'

skillitzimberg · 2020-04-18T20:22:34Z

src/components/AddItemForm.js

+    super(props);
+    this.state = {
+      name: '',
+      next_purchase: null,


I couldn't find a record of it the ticket or AC, but is there supposed to be a default value for next_purchase?

For AC next_purchase should have an integer saved with the item. You might need an onChange={this.handleNextPurchaseChange} similar to the one you have for the item name.

The next_purchase state is not updating and saving properly to the firestore database. Your item name is updating properly. To have the select drop-down update the proper numerical value to the database, there are a couple of things you need to do.

this.handleSchdeule = this.handleSchdeule.bind(this); handleSchdeule(event) { this.setState({ next_purchase: event.target.value, }); } // in the form <select value={this.state.value} onChange={this.handleSchdeule}> // add this like you have on the text field <option value="7">Soon</option> <option value="14">Kind Of Soon</option> <option value="30">Not Soon</option> </select>

The select element expects to have some value passed to it. null throws an error:

I would make the default 14 (kinda soon).

skillitzimberg · 2020-04-18T20:23:48Z

src/components/AddItemForm.js

+    //To reset the input field after the user hits submit
+    this.setState({
+      name: '',
+      ext_purchase: null,


Tyypo . . . : ) next_purchase: null,

You could make the state reset more consistent by defining an initialState variable.

const initialState = { name: 'some default or blank' } class Form extends Component { state = initialState handleSubmit = event => { ...submitLogic this.setState(initialState); } }

skillitzimberg · 2020-04-18T20:25:23Z

src/components/AddItemForm.js

+        <br />
+        <label class="schedule">
+          Schedule:
+          <select value={this.state.schedule}>


this.state.schedule should be this.state.next_purchase

SaraSweetie · 2020-04-18T22:14:01Z

src/components/AddItemForm.js

+            value={this.state.name}
+            onChange={this.handleChange}
+          />
+          <input type="submit" value="Submit" />


The submit button should be at the end of the form. You have a text field then the submit button, followed by two more form inputs. That is not good for accessibility.

stevelikesmusic

This is some great work @espain16 @erostribe 👏

There are only a couple things I would add/change before merging:

Set a default for the select element that is not null
Use className instead of class on the form
Use useToken now that we have it, so we save the item with the list token the user created.

The other comments are suggestions or 🎉 Lot's of good stuff here!!

stevelikesmusic · 2020-04-20T03:12:49Z

src/components/AddItemForm.js

+            onChange={this.handleChange}
+          />
+        </label>
+        <br />


❤️ this semantic and accessible form 😍

Alternatively, you could remove the <br /> tags and use css:

#addItemForm label { display: block; }

stevelikesmusic · 2020-04-20T03:13:25Z

src/components/AddItemForm.js

+  render() {
+    return (
+      <form onSubmit={this.handleSubmit}>
+        <label class="name">


🚨 React wants className rather than class. class is a keyword in JS so we can't use it in jsx.

stevelikesmusic · 2020-04-20T03:14:27Z

src/components/AddItemForm.js

+          </select>
+        </label>
+        <br />
+        <label class="lastpurchase">Last Purchased Date:</label>


We can probably omit this value. We'll set this value in a later story when we mark that the item has been purchased.

stevelikesmusic · 2020-04-20T03:23:34Z

src/components/AddItemForm.js

+      user_token: 'shpongle',
+    };
+
+    this.handleChange = this.handleChange.bind(this);


Not a blocker, just FYI in case you didn't know. With create-react-apps (CRA), we have a babel feature that will let you define method without explicitly binding this in the constructor.

handleChange = event => { this.setState({ name: event.target.value }); }

Like the docs mention, it's an experimental feature so your mileage may vary. But it's definitely a common pattern.

stevelikesmusic · 2020-04-20T03:30:11Z

src/components/AddItemForm.js

+    //To reset the input field after the user hits submit
+    this.setState({
+      name: '',
+      ext_purchase: null,


You could make the state reset more consistent by defining an initialState variable.

const initialState = { name: 'some default or blank' } class Form extends Component { state = initialState handleSubmit = event => { ...submitLogic this.setState(initialState); } }

stevelikesmusic · 2020-04-20T03:38:16Z

src/components/TestHistory.js

@@ -0,0 +1,3 @@
+import { createBrowserHistory as history } from 'history';


What is this component for? Could it be deleted?

stevelikesmusic · 2020-04-20T03:44:16Z

src/components/AddItemForm.js

+  }
+
+  handleSubmit(event) {
+    this.props.firestore


Not at all a requirement for this ticket—just a question: How might you redirect the user back to their list after they successfully add an item?

stevelikesmusic · 2020-04-20T03:45:30Z

src/components/AddItemForm.js

@@ -0,0 +1,80 @@
+import React from 'react';
+import { withFirestore } from 'react-firestore';
+import '../CSS/AddItemForm.css';


Now that #5 is merged, you could add useToken to this component and utilize the same token that the user has created and saved to localStorage.

stevelikesmusic · 2020-04-20T03:46:05Z

.vscode/settings.json

@@ -0,0 +1,3 @@
+{
+  "liveServer.settings.port": 5502


If we don't want to save this file to the repo, you can add it to .gitignore.

Like Steve said, we should exclude this file as it's a code editor configuration

stevelikesmusic · 2020-04-20T03:52:25Z

src/components/AddItemForm.js

+  }
+
+  handleSchedule(event) {
+    this.setState({ next_purchase: event.target.value });


We will expect to use this value as a number later on to calculate the next purchase date. What data type is event.target.value here? Is it a number? If not, how could we make it one?

stevelikesmusic · 2020-04-20T03:59:45Z

One other comment:

I like the before/after in the PR details—I might add an additional step in the testing steps talking about what to expect once we submit the form. Let's get in the habit of writing a thorough description (especially about what you did and why you made the choices you did). You should be able to use these PRs as a resource showing how you communicate with a remote team.

Again, lot's of great work 👏👏👏

segdeha

Approving so that once you add the token as the other data point you're free to merge 💪

segdeha · 2020-04-20T17:25:22Z

src/components/AddItemForm.js

+  }
+
+  handleSubmit(event) {
+    const { name, next_purchase, last_purchase } = this.state;


Sorry I didn't mention this in our Slack thread, but I believe you want to store the user's token along with the other data points, correct?

segdeha · 2020-04-20T17:26:00Z

src/components/AddItemForm.js

+    const { name, next_purchase, last_purchase } = this.state;
+    this.props.firestore
+      .collection('itemform')
+      .add({ name, next_purchase, last_purchase })


Connected to line 32 above, store the token along with the other data

segdeha

This looks good 👍

segdeha · 2020-04-20T21:16:21Z

.vscode/settings.json

@@ -0,0 +1,3 @@
+{
+  "liveServer.settings.port": 5502


Like Steve said, we should exclude this file as it's a code editor configuration

espain16 and others added 8 commits April 13, 2020 16:38

Add 'AddItemForm' component.

fb40a00

Updated the state for 'AddItemForm'

e72c999

Updated 'AddItemForm' state along form details

d3d6f4d

imported with Firestore from React Firestore.

7295818

To reset the input field after the user hits submit

555c747

updated add item form input fields

8498b17

created proper route for add item form

df3b2b5

added css styling to form

d5ff690

erostribe requested review from skillitzimberg, SaraSweetie and espain16 April 18, 2020 05:27

skillitzimberg reviewed Apr 18, 2020

View reviewed changes

SaraSweetie reviewed Apr 18, 2020

View reviewed changes

espain16 added 3 commits April 18, 2020 21:32

Removed 'prettier' import.

45108f5

Schedule input field now saved to database

943fe6e

Changed form layout

640dc80

stevelikesmusic approved these changes Apr 20, 2020

View reviewed changes

espain16 added 2 commits April 20, 2020 09:25

Resolved select element errors

544d892

Updated form fields saved to the data base

84baf55

segdeha approved these changes Apr 20, 2020

View reviewed changes

espain16 added 5 commits April 20, 2020 10:32

Removed user token from state

dcd57b9

Fixed routing issue for form component

af8dfc3

Make updates to the user token in state

915197a

Refactored code

f5d9b4b

Updated collections name to 'items'

f796209

segdeha approved these changes Apr 20, 2020

View reviewed changes

SaraSweetie approved these changes Apr 20, 2020

View reviewed changes

Added '.vscode' folder to .gitignore

6a4fa87

espain16 merged commit 0a7102d into master Apr 20, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Es pja add new item #28

Es pja add new item #28

erostribe commented Apr 18, 2020

skillitzimberg Apr 18, 2020 •

edited

Loading

skillitzimberg Apr 18, 2020 •

edited

Loading

SaraSweetie Apr 18, 2020 •

edited

Loading

stevelikesmusic Apr 20, 2020

skillitzimberg Apr 18, 2020 •

edited

Loading

stevelikesmusic Apr 20, 2020

skillitzimberg Apr 18, 2020

SaraSweetie Apr 18, 2020

stevelikesmusic left a comment

stevelikesmusic Apr 20, 2020

stevelikesmusic Apr 20, 2020

stevelikesmusic Apr 20, 2020

stevelikesmusic Apr 20, 2020

stevelikesmusic Apr 20, 2020

stevelikesmusic Apr 20, 2020

stevelikesmusic Apr 20, 2020

stevelikesmusic Apr 20, 2020

stevelikesmusic Apr 20, 2020

segdeha Apr 20, 2020

stevelikesmusic Apr 20, 2020

stevelikesmusic commented Apr 20, 2020

segdeha left a comment

segdeha Apr 20, 2020

segdeha Apr 20, 2020

segdeha left a comment

segdeha Apr 20, 2020

		@@ -0,0 +1,71 @@
		import React from 'react';
		import { format } from 'prettier';

		@@ -0,0 +1,3 @@
		import { createBrowserHistory as history } from 'history';

Es pja add new item #28

Es pja add new item #28

Conversation

erostribe commented Apr 18, 2020

Description

Related Issue

Acceptance Criteria

Type of Changes

Updates

Before

After

Testing Steps / QA Criteria

skillitzimberg Apr 18, 2020 • edited Loading

Choose a reason for hiding this comment

skillitzimberg Apr 18, 2020 • edited Loading

Choose a reason for hiding this comment

SaraSweetie Apr 18, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skillitzimberg Apr 18, 2020 • 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

stevelikesmusic 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

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

stevelikesmusic commented Apr 20, 2020

segdeha left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

segdeha left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skillitzimberg Apr 18, 2020 •

edited

Loading

skillitzimberg Apr 18, 2020 •

edited

Loading

SaraSweetie Apr 18, 2020 •

edited

Loading

skillitzimberg Apr 18, 2020 •

edited

Loading