To do day2 #2

omar25ahmed · 2022-02-18T16:58:16Z

Learning Objectives:

Remove all hardcoded items from the tasks array.
Create a new JavaScript file for the new functionality.
Implement a function for adding a new task (add a new element to the array).
Implement a function for deleting a task (remove an element from the array).
Implement a function for editing task descriptions.
By default new tasks should have the property completed set to false and the property index set to the value of the new array - length (i.e. if you're adding a 5th task to the list, the index of that task should equal to 5).
Deleting a task should update all remaining items' indexes, so they represent the current list order and are unique.
All changes to the To-Do List should be saved in local storage.

mateo951

Hi @omar25ahmed,

📝👌 Your project highlights 🔥😎

General Requirements 🔥
Add, Remove, and Edit methods were implemented perfectly 💪

Great 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!

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, 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

mateo951 · 2022-02-18T17:41:27Z

src/index.js

@@ -1,14 +1,135 @@
-import _ from 'lodash';


When adding or deleting a task from your to-do list its important that the tasks property index is always set to the value of the new array length.

As you can see in the image, After playing around adding, deleting, and editing tasks, the left task index is 3 when it should be 1. Kindly make the necessary changes to that your app works as mentioned 🔥

The mentioned issue with the indexes of your tasks is causing another one. If you create a single task and you remove it, works perfectly. If you create a bunch of tasks and then delete the first-created ones until only the last one is left and you try to delete it doesn't work properly. After the page is refreshed, the task is still there. I assume its something to do with the index not matching the one you think you are removing 👀

Gambit142

Hi @omar25ahmed 👋,

Great work on making the changes requested by a previous reviewer 👏
You've done well implementing some of the requested changes, but there are still some which aren't addressed yet.

Required Changes ♻️

- [ ] It would be nice to implement the first suggested change by the other reviewer. Kindly transfer all the functions for adding, editing, and removing tasks to another file, then import that file into your index.js file.

Kindly check the comments under the review for further suggestions/required changes. 👍

Optional suggestions

N/A

Cheers and Happy coding!👏👏👏

Feel free to leave any questions or comments in the PR thread if something is not 100% clear.

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.

Gambit142 · 2022-02-18T21:22:17Z

src/index.js

+      el.dataset.index = task.index - 1;
+      task.updatedIndex = task.index - 1;
+    });


I can see that you are trying to implement changes from the previous reviewer. You do not need to add an updatedIndex key when you delete an item, all that is required for you is to rearrange the values of the index key of each object in the array.

Hint:

const filteredArray = array.filter((obj) => obj.completed !== true); filteredArray.forEach((obj, index) => { obj.index = index + 1; })

👍🏽 This works fine now

Gambit142 · 2022-02-18T21:32:51Z

src/index.js

+function prepareEdit(task, btn) {
+  btn.addEventListener('click', () => {
+    const el = document.querySelector(`[data-index="${task.index}"]`);
+    const p = el.children[1];
+    const input = document.createElement('input');
+    input.type = 'text';
+    input.value = p.textContent;
+    input.id = task.index;
+    el.insertBefore(input, p);
+    el.removeChild(p);
+    input.focus();
+    input.select();
+    input.classList.add('edit');
+  });
+}


I like the way you implemented the edit. However, when I edit a task, the edited description is not saved to the localStorage, so when I reload the page, it shows undefined as the value for the description. What you want to do is to edit the value of the description and save the array with the edited value to the localStorage.

Hint: You should filter through the array, select that particular object, set the value of the description key of that object to the input value then save the whole array to the local storage.

👍🏽 This works fine now

Gambit142 · 2022-02-18T21:34:38Z

src/index.js

+    // getTasks();
+  });


Kindly remove all codes that are commented out. Removing such codes makes your code look clean.

Gambit142 · 2022-02-18T21:35:19Z

src/index.js

+  // getTasks();
+  console.log(tasks);


Kindly remove all codes that are commented out. Removing such codes makes your code look clean.

Gambit142 · 2022-02-18T21:37:33Z

src/index.js

+    // getTasks();
+    console.log(tasks);
+  });


Kindly remove all codes that are commented out. Removing such codes makes your code look clean.

Kindly remove the console.log function in your code. It is a best practice to remove all console.log statements from your code so that you do not reveal sensitive information to visitors of your site.

👍🏽 This has been implemented also

Whoistolu

Hi @omar25ahmed

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

STATUS: APPROVED 🟢

Congratulations! 🎉

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 @Whoistolu in your question so I can receive the notification.

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.

omar25ahmed added 8 commits February 16, 2022 14:39

Adding the structure

0d35618

Adding the logic

05da2a6

Styling the list

620e825

Creating module

285e3f4

fixing linters issue

9a78f8d

day 2 requiirments

6353147

Adding some styles

5942ddb

Modyfing Readme file

ced8f56

mateo951 suggested changes Feb 18, 2022

View reviewed changes

omar25ahmed added 2 commits February 18, 2022 22:37

Create new file for functionality

6c64e70

solving add and remove from LS issue

6592ad2

Gambit142 suggested changes Feb 18, 2022

View reviewed changes

address code review comments

4667859

Whoistolu approved these changes Feb 18, 2022

View reviewed changes

omar25ahmed merged commit 93de824 into main Feb 18, 2022

Repository owner deleted a comment from mateo951 Apr 15, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

To do day2 #2

To do day2 #2

omar25ahmed commented Feb 18, 2022

mateo951 left a comment

mateo951 Feb 18, 2022

Gambit142 left a comment

Gambit142 Feb 18, 2022

Whoistolu Feb 18, 2022

Gambit142 Feb 18, 2022

Whoistolu Feb 18, 2022

Gambit142 Feb 18, 2022

Gambit142 Feb 18, 2022

Gambit142 Feb 18, 2022

Whoistolu Feb 18, 2022

Whoistolu left a comment

To do day2 #2

To do day2 #2

Conversation

omar25ahmed commented Feb 18, 2022

Learning Objectives:

mateo951 left a comment

Choose a reason for hiding this comment

Hi @omar25ahmed,

Required Changes ♻️

Optional suggestions

Choose a reason for hiding this comment

Gambit142 left a comment

Choose a reason for hiding this comment

Required Changes ♻️

- [ ] It would be nice to implement the first suggested change by the other reviewer. Kindly transfer all the functions for adding, editing, and removing tasks to another file, then import that file into your index.js file.

Kindly check the comments under the review for further suggestions/required changes. 👍

Optional suggestions

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Whoistolu left a comment

Choose a reason for hiding this comment

Hi @omar25ahmed

STATUS: APPROVED 🟢