Conversation
In the official React docs, they suggest building applications by first building a static version of the app, passing data down, and only afterwards adding interactivity and passing events "up".
TODO
bc this addess a new contact, need to loop and replace
setContacts([...contacts, updatedContact])
Must use array map & replace
Deconstructing still is confusing
| margin: 0 auto; | ||
| padding: 2rem; | ||
| text-align: left; | ||
| background-color: rgb(210, 210, 210); |
There was a problem hiding this comment.
Right now, when the are no contacts, the background color only spans a part of the page. You can make it span the entire window by adding a minimum height of 100% of the viewport height: min-height: 100vh;
|
|
||
| const handleContactDelete = () => { | ||
| const updatedContacts = contacts.filter( | ||
| (contact) => contact.id !== editContactId |
There was a problem hiding this comment.
This works, but indeed, it would have been even better to pass the deletedContactId, just to make the code even more readable and robust.
| </header> | ||
|
|
||
| <section> | ||
| <ContactAdd onAddNew={ (e) => handleAddNewContact(e) } /> |
There was a problem hiding this comment.
Tip: using e as the argument name here can be misleading! I initially thought e is the submit event, but it's actually the new contact; Consider renaming this to newContact or similar.
|
|
||
| onAddNew(newContact); | ||
|
|
||
| e.target.reset(); |
There was a problem hiding this comment.
I noticed this line is highlighted with red in VSCode due to Typescript not recognising that e.target has a reset method. You can fix that by telling TS this is a form element: (e.target as HTMLFormElement).reset()
| return ( | ||
| <div className="card flex flex-col w-96 bg-base-100 card-md shadow-sm" key={ contact.id }> | ||
| <div className="card-body"> | ||
| <h1 id="cardName" className="card-title font-bold">{ contact.name }</h1> |
There was a problem hiding this comment.
You add the HTML id attribute to the h1 and div elements in the ContactCard component.
Since you are rendering this component once for each contact, as soon as you have more than 1 contact, you will end up with duplicated HTML ids! This is invalid HTML markup.
Do you really need the id attribute here? Consider simply removing it.
Alternatively, if you want to keep the id, consider generating it using React's useId hook, so each ContactCard will use unique ids.
| @@ -0,0 +1,2 @@ | |||
| @import "tailwindcss"; | |||
| @plugin "daisyui"; | |||
There was a problem hiding this comment.
you are already importing tailwind and daisyui in App.css - consider removing them from here!
|
|
||
| export default function ContactCard({ contact, onEdit }: ContactCardProps) { | ||
| return ( | ||
| <div className="card flex flex-col w-96 bg-base-100 card-md shadow-sm" key={ contact.id }> |
There was a problem hiding this comment.
you don't really need the key attribute over here
There was a problem hiding this comment.
I tried to remove key property, DevTool shows this
`
App.tsx?t=1744266628666:94 Each child in a list should have a unique "key" prop.
Check the render method of App. See https://react.dev/link/warning-keys for more information.
`
3 participants
https://reactpractice.dev/solution/tutorial-create-a-simple-contact-book-app/