Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

react1-week3/nishadi #311

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

react1-week3/nishadi #311

wants to merge 1 commit into from

Conversation

nishadipri
Copy link

Please review my homework.

@github-actions github-actions bot changed the title react1-week3/nishadi react1-week3/nishadi Jan 28, 2024
Copy link

@babak-f babak-f left a comment

Choose a reason for hiding this comment

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

Good job

date.setHours(0, 0, 0, 0)

if (deadline < date) {
return window.alert('Please select a deadline in present or future')
Copy link

Choose a reason for hiding this comment

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

Since handleSubmit does not expect a return value, you can rewrite this into:

alert('...');
return;

Here the purpose of the return statement will be simply to stop executing the rest of the function, which is correct.

/>
<br />
<label >Deadline</label>
<div>
Copy link

Choose a reason for hiding this comment

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

You don't need the div here, since it is not doing anything.


const deadLineText = `${date.getFullYear()}-${date.getMonth()+1}-${date.getDate()}`

const decorateDone = props.done ?
Copy link

Choose a reason for hiding this comment

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

Good one 👍

}

return (
<li key={props.taskId} className="todoItem">
Copy link

Choose a reason for hiding this comment

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

As a convention, class names are always written in kebab case (https://developer.mozilla.org/en-US/docs/Glossary/Kebab_case).

);
}

TodoItem.propTypes = {
Copy link

Choose a reason for hiding this comment

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

Good one

const onAddNewTask = (description, deadline) => {
const newTask = {
id: new Date().getTime(),
description: description,
Copy link

Choose a reason for hiding this comment

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

Optional: since both sides are called description, you can just write it once, i.e. description, .

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