Skip to content

CODE REVIEW  #9

@ko-karol

Description

@ko-karol

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions