Skip to content

Code to review#1

Open
2bleO wants to merge 3 commits into
mainfrom
review
Open

Code to review#1
2bleO wants to merge 3 commits into
mainfrom
review

Conversation

@2bleO
Copy link
Copy Markdown
Owner

@2bleO 2bleO commented Jul 27, 2021

In this PR we:

Commit to-do-list exercise as one big chunk of code in a feature branch for JS best practice review.

@2bleO 2bleO requested a review from marijanbrvar July 27, 2021 07:25
Copy link
Copy Markdown
Collaborator

@marijanbrvar marijanbrvar left a comment

Choose a reason for hiding this comment

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

Great Work. I found your code readable, and there are no many things I would suggest to change, anyways my suggestion is, it will improve code readability a lot, which is the basic principle of code duty separation. Just group DOM manipulation in function and not mix logic and dom manipulation.

Comment thread src/index.js
const list = [];
let displayedList;

const todoList = (arr) => {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Great Work. I found your code readable, and there are no many things I would suggest changing, anyways. My suggestion is, it will improve code readability a lot, which is the basic principle of code duty separation. Just group DOM manipulation in function and not mix logic and dom manipulation.

You can use string interpolation or separate code related to dom element creation and styling in another function than the call function. Of course, this is just an optional suggestion.

Great work, my friend!

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Thank you!

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.

2 participants