Skip to content

The todo - w.11 JL#437

Open
JohLeo wants to merge 19 commits into
Technigo:masterfrom
JohLeo:master
Open

The todo - w.11 JL#437
JohLeo wants to merge 19 commits into
Technigo:masterfrom
JohLeo:master

Conversation

@JohLeo

@JohLeo JohLeo commented Apr 23, 2023

Copy link
Copy Markdown


const capitalizeFirstLetter = (str) => {
return str.charAt(0).toUpperCase() + str.slice(1);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice detail!

const onFormSubmit = (event) => {
event.preventDefault();
const newNote = {
id: Date.now().toString(),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This id works for this project, but when building larger applications with multiple users there's a risk that two posts get the same id if posted at the same time. There are id-generators like uuid (npm package) that prevents this if you want to have look.

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

Nice app! Clean responsive design and it works as it should.
Very well structured, easy to follow with good naming and use of variables, components etc.
Because you assign variable names to the data you pass around the text doesn't get cluttered and repetitive. Good job!
One small comment on the design: Is the contrast on the buttons a bit hard to see (accessibility)?

PS. See additional comments in the code.

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