Skip to content

Complete Contact List Eval#30

Open
cjedrasik03 wants to merge 5 commits into
projectshft:mainfrom
cjedrasik03:main
Open

Complete Contact List Eval#30
cjedrasik03 wants to merge 5 commits into
projectshft:mainfrom
cjedrasik03:main

Conversation

@cjedrasik03
Copy link
Copy Markdown

No description provided.

Comment thread app/contacts/new/page.js

const newContact = {
...formData,
id: Math.floor(Math.random() * 100000000)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

extract to a function for readability

Comment thread app/contacts/new/page.js
};

// Handles submit button
const handleSubmit = e => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

don't be cheap with the param declaration

Suggested change
const handleSubmit = e => {
const handleSubmit = event => {

Comment thread app/contacts/[id]/page.js

const allContacts = [defaultContact, ...savedContacts];

const found = allContacts.find(c => c.id.toString() === params.id);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

beware of readability

Suggested change
const found = allContacts.find(c => c.id.toString() === params.id);
const found = allContacts.find(contact => contact.id.toString() === params.id);

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