Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Peer-to-peer Code Review #5

Open
mengiefen opened this issue Feb 24, 2022 · 0 comments
Open

Peer-to-peer Code Review #5

mengiefen opened this issue Feb 24, 2022 · 0 comments

Comments

@mengiefen
Copy link

Dear @omar25ahmed ,

Congratulations. 馃挴 馃挜

  • Your code is readable
  • You code structure is well organized
  • All functionalities are working as expected

Optional Suggestions 鈾伙笍

DOM manipulation using element.innerHTML has security concern. I think it is better to modify it using JavaScript function like createElement() , append(), appendChild().

ToDo-List/src/index.js

Lines 41 to 50 in 1965b75

li.innerHTML = `
<div class="task" data-index="${task.index}">
<input ${task.completed ? 'checked' : ''} type="checkbox" id='${task.index}'>
<p class="task-list ${task.completed ? 'done' : ''}">${task.description}</p>
<div class="btns">
<button type="button" class="close-button scroll">+</button>
<button type="button" class="fas fa-ellipsis-v scroll" id="edit-${task.index}"></button>
</div>
</div>
`;

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

No branches or pull requests

1 participant