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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add&remove #2

Merged
merged 7 commits into from Feb 24, 2022
Merged

Add&remove #2

merged 7 commits into from Feb 24, 2022

Conversation

ZahraArshia
Copy link
Owner

@ZahraArshia ZahraArshia commented Feb 23, 2022

Hello 馃槉

in this step of project:

  • a new JavaScript file is added with the name of CRUD.js including class "Task" with methods for add, remove, edit and update
  • index key of each task is updating according to its position in storage array
  • edit function don't affect the position of the item in list, just changes the description

in the feature:

the clear all completed button is disable for now, in next step its functionality will be implemented according to Microverse schedule

Copy link

@micheaol micheaol left a comment

Choose a reason for hiding this comment

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

Hi @ZahraArshia ,

Good job so far!
There are some issues that you still need to work on to go to the next project but you are almost there!

TO HIGHLIGHT:

  • Your project is professional 馃挴
  • Your repo is professional 馃挴
  • Well structured files 馃挴

Required Changes 鈾伙笍

Check the comments under the review.

Optional suggestions

Every comment with the [OPTIONAL] prefix is not crucial enough to stop the approval of this PR. However, I strongly recommend you to take them into account as they can make your code better.

Cheers and Happy coding!馃憦馃憦馃憦

Feel free to leave any questions or comments in the PR thread if something is not 100% clear.
Please, remember to tag me in your question so I can receive the notification.

Please, do not open a new Pull Request for re-reviews. You should use the same Pull Request submitted for the first review, either valid or invalid unless it is requested otherwise.


_As described in the Code reviews limits policy you have a limited number of reviews per project (check the exact number in your Dashboard). If you think that the code review was not fair, you can request a second opinion using this form.

src/CRUD.js Outdated
Comment on lines 18 to 25
add() {
const temp = new Task(this.taskList.length, document.getElementById('inputDescription').value, false);
this.taskList.push(temp);
localStorage.setItem('taskList', JSON.stringify(this.taskList));
this.taskList = JSON.parse(localStorage.getItem('taskList'));
this.update();
location.reload();
}

Choose a reason for hiding this comment

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

Please ensure that items are numbered from 1 according to the project requirement and NOT 0.
image.

@ZahraArshia
Copy link
Owner Author

Hello;
the indexes are starting from 1 now 馃槉

Copy link

@Rustamxon7 Rustamxon7 left a comment

Choose a reason for hiding this comment

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

Hi @ZahraArshia 馃憢,

Your project is complete! There is nothing else to say other than... it's time to merge it :shipit:

Congratulations! 馃帀 馃帀 馃帀 馃帀 馃帀,

Cheers and Happy coding!馃憦馃憦馃憦

Developer-512px-10


As described in the Code reviews limits policy you have a limited number of reviews per project (check the exact number in your Dashboard). If you think that the code review was not fair, you can request a second opinion using this form.

@ZahraArshia ZahraArshia merged commit ba3a70a into main Feb 24, 2022
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.

None yet

3 participants