Add remove feature #3

Mwapsam · 2022-01-14T10:48:47Z

CRUD - Task

Removed all hardcoded items from the tasks array
Created a TasksManager class for the new functionality:
Creating a new task
Updating task descriptions
Deleting a task
Continuous index number value
All changes are saved in local storage.

…-remove-feature

rloterh

Hi Mwape,

You did extremely great on this project! 👏 👏
There are however a few issues that you still need to work on to go to the next project but you are almost there! 🚦 🚦

Good Points 👍

Descriptive Pull Request
No linter errors

Required Changes ♻️

Kindly 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.

Kindly check the comments marked [OPTIONAL] under the review.

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 2 more limited reviews per this project. If you think that the code review was not fair, you can request a second opinion using this form.

rloterh · 2022-01-14T11:07:36Z

README.md

+It will open your browser with the project homepage
+
+
+
 ## Authors


OPTIONAL
Kindly change the header title Authors to Author, as this project is done solely. This will make your ReadMe appear more professional.

rloterh · 2022-01-14T11:13:32Z

src/index.js

@@ -1,60 +1,55 @@
+/* eslint-disable default-case */


You have disabled linter check on your code. Please enable all linter checks on your code to ensure that your code is robust code and clean.

As requested by the previous reviewer, Please enable all linter checks on your code to ensure that your code is robust code and clean.

rloterh · 2022-01-14T11:17:50Z

src/modules/crud.js

+const list = document.getElementById('todos-list');
+const addInput = document.getElementById('todo-input');
+
+export const createTodo = () => {


There are no indexes associated with task entries. Kindly ensure that new tasks have the property completed set to false by default and the property index set to the value of the new array length (i.e. if a 5th task is added to the list, the index of that task should equal to 5). This should be reflected in your local storage

rloterh · 2022-01-14T11:25:03Z

src/modules/crud.js

+  const div = document.createElement('div');
+  div.classList.add('TextItems');
+
+  const remove = document.createElement('span');


When elements are removed they are still reflected in the local storage. Kindly ensure that removed items are deleted from the local storage.

Deleting error:

…-List into add-remove-feature

julie-ify

Hi @Mwapsam,

Good job so far!

Highlight!

Good job implementing some of the required changes from the previous reviewer ✔️

Hello ✋, you have done well reaching this point of the milestone 🤝 . But there are some issues that you still need to work on to go to the next project but you are almost there!

Status: Required Changes ♻️⛔️

Check the comments under the review.

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.

julie-ify · 2022-01-14T16:43:37Z

src/index.js

@@ -1,60 +1,55 @@
+/* eslint-disable default-case */


As requested by the previous reviewer, Please enable all linter checks on your code to ensure that your code is robust code and clean.

julie-ify · 2022-01-14T16:44:50Z

src/modules/crud.js

+export const removeTodo = (removeElement, index) => {
+  const getLocalStorageData = localStorage.getItem('New Todo');
+  const listArray = JSON.parse(getLocalStorageData);
+  listArray.splice(index, 1);
+  localStorage.setItem('New Todo', JSON.stringify(listArray));
+  removeElement.parentElement.parentElement.remove();
+};


I created three tasks and they all have the correct index [1,2,3]. However, when I deleted one task, the indexes of the remaining tasks did not update, that is the indexes were [2, 3] instead of [1,2]. Please let's make sure that deleting a task updates all remaining items' indexes, so they represent the current list order and are unique.

When I delete a task, the task gets deleted from the page but surprisingly, another task gets deleted from the local storage instead of the task I deleted. Let's fix this.

Also when I added a new task, there appears to be two tasks with the same index. Please let's fix that. Please take a look below:

Before I deleted eat task:

After I deleted eat task, I can still see eat task in the local storage:

When I added a new task:

MrBrN197

Hi, @Mwapsam 👋

Great work making the changes from the previous reviewers.
Your project is complete! There is nothing else to say other than... it's time to merge it
Congratulations! 🎉

Cheers and Happy coding!👏👏👏

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

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.

MrBrN197 · 2022-01-14T21:41:45Z

src/modules/lodash.js

+/* eslint-disable import/prefer-default-export */
+import _ from 'lodash';
+
+export function component() {


[OPTIONAL] Thank you for remove all other eslint-disables you only have one here now. The way you could fix this is by making component a default export.

something like this

Suggested change

/* eslint-disable import/prefer-default-export */

import _ from 'lodash';

export function component() {

import _ from 'lodash';

export default function component() {

[OPTIONAL] also it seems that you are not using this file. Since it came with the default webpack setup you can just remove this file and also remove lodash as a dependency on your package.json

Mwapsam and others added 7 commits January 13, 2022 08:56

add todo functionalities

5f16d8d

Merge branch 'main' of https://github.com/Mwapsam/To-Do-List into add…

1ae4472

…-remove-feature

add local storage functionality

992afb0

change layout

e0b878f

complete add & remove feature

338b2db

Update README file

a54f5a3

Update README.md

8746040

rloterh suggested changes Jan 14, 2022

View reviewed changes

Mwapsam added 2 commits January 14, 2022 17:49

work on reviewer's recommendations

2e89730

Merge branch 'add-remove-feature' of https://github.com/Mwapsam/To-Do…

3676d44

…-List into add-remove-feature

julie-ify suggested changes Jan 14, 2022

View reviewed changes

Mwapsam added 3 commits January 14, 2022 22:41

fix index update in local storage

5d3ef59

fix linter errors

1f887be

fix linter errors

b90635f

MrBrN197 approved these changes Jan 14, 2022

View reviewed changes

Mwapsam merged commit 49654d2 into main Jan 14, 2022

github-pages bot temporarily deployed to github-pages January 15, 2022 14:52 Inactive

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add remove feature #3

Add remove feature #3

Mwapsam commented Jan 14, 2022

rloterh left a comment

rloterh Jan 14, 2022

rloterh Jan 14, 2022

julie-ify Jan 14, 2022

rloterh Jan 14, 2022

rloterh Jan 14, 2022

julie-ify left a comment

julie-ify Jan 14, 2022

julie-ify Jan 14, 2022

MrBrN197 left a comment

MrBrN197 Jan 14, 2022

		It will open your browser with the project homepage



		## Authors

Add remove feature #3

Add remove feature #3

Conversation

Mwapsam commented Jan 14, 2022

rloterh left a comment

Choose a reason for hiding this comment

Good Points 👍

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

julie-ify left a comment

Choose a reason for hiding this comment

Hi @Mwapsam,

Highlight!

Status: Required Changes ♻️⛔️

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MrBrN197 left a comment

Choose a reason for hiding this comment

Hi, @Mwapsam 👋

Choose a reason for hiding this comment

something like this