Skip to content

Conversation

@cmhorsey
Copy link

Wrote a controlled filter and form component.

Copy link

@stephenmckeon stephenmckeon left a comment

Choose a reason for hiding this comment

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

Awesome job!!

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

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
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.

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('')

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)}
>

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