Skip to content

Completed Forms Lab - #9

Open
cmhorsey wants to merge 3 commits into
learn-co-curriculum:masterfrom
cmhorsey:test_branch
Open

Completed Forms Lab#9
cmhorsey wants to merge 3 commits into
learn-co-curriculum:masterfrom
cmhorsey:test_branch

Conversation

@cmhorsey

Copy link
Copy Markdown

Wrote a controlled filter and form component.

@stephenmckeon stephenmckeon left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Awesome job!!

Comment thread src/components/Filter.js
Comment on lines +6 to +12
<input
type="text"
name="search"
value={search}
placeholder="Search..."
onChange={onSearchChange}
/>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
<input
type="text"
name="search"
value={search}
placeholder="Search..."
onChange={onSearchChange}
/>
<input
type="text"
name="search"
value={search}
placeholder="Search..."
onChange={onSearchChange}
/>

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Just a warning I continued to be terrible at indenting my component props until you showed me the right way on like Friday or Thursday so apologies in advance but I promise I have gotten better lol.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Haha, I see, no worries!


function ItemForm(props) {
function ItemForm({ onItemFormSubmit }) {
const [newItemName, setNewItem] = useState('')

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Try to keep the naming consistent:

 const [newItem, setNewItem] = useState('')

or

 const [newItemName, setNewItemName] = useState('')

Comment on lines +33 to +37
<select
name="category"
value={newCategory}
onChange={(e) => setNewCategory(e.target.value)}
>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Watch the indentation here!

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.

2 participants