-
Notifications
You must be signed in to change notification settings - Fork 0
Description
Readme
As of now, the readme is clean and concise.
User Stories
So far, adding a task does not work on the github deployment. But it works if on a local deployment, inside the javascript-functionality branch. It logs the current state of an array with your list items, which is nice.
So far, a user is able to create a new task, and delete a new task.
Areas of improvement and comments
Maybe instead of logging a whole array, log only the new item.
There is no clear indication that you can delete a task by clicking on the list item.
- Add a visual indication that a task can be deleted.
- The css design looks very neat.
File structure looks perfectly fine, aside from one minor detail.
- If there is more than one of the same type file, combine them into a single folder.
Code Flow
The flow of the code seems a bit weird.
list.addEventListener("click", (x) => {
let idSome = x.target.getAttribute("data-id");
if (!idSome) return; // user clicked in something else
// passing id to function
taskFromDOM(idSome);
taskFromArray(idSome);
});
Seems to be calling functions from below in the code flow. Wouldn't it make more sense to create those functions before you need to use them?
- Move functions above when you need to call them.
Readability
- Rename (x) in event listeners to (e) or (event) for readability.
Code is a little bit hard to understand at this point. Maybe taskTo and taskFrom function could be condensed into one? Not sure.
- Combine task functions.