Code Review

[armaaar]

• You didn't fork our repo
• You downloaded the project instead of cloning it
• You added all changes into a single commit
• You didn't pay attention to eslint
• You should have returned the initial state if no local storage found
  

  React-todos-list/src/store/index.jsx

  Line 18 in 38e2909

  if (serialisedState === null) return undefined;

• deconstruct props when possible
  

  React-todos-list/src/components/TodoList/index.jsx

  Line 7 in 38e2909

  <TodoItem todo={todo} index={index} key={index} removeToDo={props.removeToDo} handleEditSubmit={props.handleEditSubmit} handleCompleteTodo={props.handleCompleteTodo}/>

• Remove unused imports
  

  React-todos-list/src/pages/index.jsx

  Line 4 in 38e2909

  import TodoForm from '../components/TodoForm';

• Don't use event to bind data. use state instead
  

  React-todos-list/src/components/TodoItem/index.jsx

  Line 14 in 38e2909

  const task = event.target.value;

• Unnecessary hook, You already defined the same value as initial value for your state
  

  React-todos-list/src/components/TodoItem/index.jsx

  Line 10 in 38e2909

  setToDo(props.todo)

• Bad function name
  

  React-todos-list/src/components/TodoItem/index.jsx

  Line 20 in 38e2909

  function handleUpdateSubmit(event) {

• Unnecessary comment
  

  React-todos-list/src/components/TodoItem/index.jsx

  Line 30 in 38e2909

  //function to complete todo

• Never use inline styles without a proper reason. use class names instead
  

  React-todos-list/src/components/TodoItem/index.jsx

  Line 46 in 38e2909

  <h2 style={{ textDecoration:"line-through" }}

• Either remove unclickable call to actions or change their styles
  

  React-todos-list/src/components/TodoItem/index.jsx

  Line 48 in 38e2909

  <button className={styles.completeBtn} disabled={true}>Complete</button>

[emanelshawwa]

Dear Eng Ahmed, I hope this email finds you well Thank you for your comments , I will solve these issues and send email to you tomorrow Best regards Eman Elshawwa

…

On Sat, 17 Oct 2020, 2:16 pm Ahmed Rafik Ibrahim, ***@***.***> wrote: - You didn't fork our repo - You downloaded the project instead of cloning it - You added all changes into a single commit - You didn't pay attention to eslint - You should have returned the initial state if no local storage found https://github.com/emanelshawwa/React-todos-list/blob/38e290953673ba205f7fbceb2958f1c4721242ef/src/store/index.jsx#L18 - deconstruct props when possible https://github.com/emanelshawwa/React-todos-list/blob/38e290953673ba205f7fbceb2958f1c4721242ef/src/components/TodoList/index.jsx#L7 - Remove unused imports https://github.com/emanelshawwa/React-todos-list/blob/38e290953673ba205f7fbceb2958f1c4721242ef/src/pages/index.jsx#L4 - Don't use event to bind data. use state instead https://github.com/emanelshawwa/React-todos-list/blob/38e290953673ba205f7fbceb2958f1c4721242ef/src/components/TodoItem/index.jsx#L14 - Unnecessary hook, You already defined the same value as initial value for your state https://github.com/emanelshawwa/React-todos-list/blob/38e290953673ba205f7fbceb2958f1c4721242ef/src/components/TodoItem/index.jsx#L10 - Bad function name https://github.com/emanelshawwa/React-todos-list/blob/38e290953673ba205f7fbceb2958f1c4721242ef/src/components/TodoItem/index.jsx#L20 - Unnecessary comment https://github.com/emanelshawwa/React-todos-list/blob/38e290953673ba205f7fbceb2958f1c4721242ef/src/components/TodoItem/index.jsx#L30 - Never use inline styles without a proper reason. use class names instead https://github.com/emanelshawwa/React-todos-list/blob/38e290953673ba205f7fbceb2958f1c4721242ef/src/components/TodoItem/index.jsx#L46 - Either remove unclickable call to actions or change their styles https://github.com/emanelshawwa/React-todos-list/blob/38e290953673ba205f7fbceb2958f1c4721242ef/src/components/TodoItem/index.jsx#L48 — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AGK54E6WZZI234URGIKXZIDSLGDLRANCNFSM4SUKLNBA> .

[emanelshawwa]

Dear Eng Ahmed, I hope this email finds you well I solved the issues , I forked the repo and this is the link of forked repo : https://github.com/emanelshawwa/react-todos-code-challenge Best regards Eman Elshawwa On Sat, Oct 17, 2020 at 5:08 PM Eman Elshawwa <eman.elshawwa@gmail.com> wrote:

…

Dear Eng Ahmed, I hope this email finds you well Thank you for your comments , I will solve these issues and send email to you tomorrow Best regards Eman Elshawwa On Sat, 17 Oct 2020, 2:16 pm Ahmed Rafik Ibrahim, < ***@***.***> wrote: > > - You didn't fork our repo > - You downloaded the project instead of cloning it > - You added all changes into a single commit > - You didn't pay attention to eslint > - You should have returned the initial state if no local storage found > > https://github.com/emanelshawwa/React-todos-list/blob/38e290953673ba205f7fbceb2958f1c4721242ef/src/store/index.jsx#L18 > - deconstruct props when possible > > https://github.com/emanelshawwa/React-todos-list/blob/38e290953673ba205f7fbceb2958f1c4721242ef/src/components/TodoList/index.jsx#L7 > - Remove unused imports > > https://github.com/emanelshawwa/React-todos-list/blob/38e290953673ba205f7fbceb2958f1c4721242ef/src/pages/index.jsx#L4 > - Don't use event to bind data. use state instead > > https://github.com/emanelshawwa/React-todos-list/blob/38e290953673ba205f7fbceb2958f1c4721242ef/src/components/TodoItem/index.jsx#L14 > - Unnecessary hook, You already defined the same value as initial > value for your state > > https://github.com/emanelshawwa/React-todos-list/blob/38e290953673ba205f7fbceb2958f1c4721242ef/src/components/TodoItem/index.jsx#L10 > - Bad function name > > https://github.com/emanelshawwa/React-todos-list/blob/38e290953673ba205f7fbceb2958f1c4721242ef/src/components/TodoItem/index.jsx#L20 > - Unnecessary comment > > https://github.com/emanelshawwa/React-todos-list/blob/38e290953673ba205f7fbceb2958f1c4721242ef/src/components/TodoItem/index.jsx#L30 > - Never use inline styles without a proper reason. use class names > instead > > https://github.com/emanelshawwa/React-todos-list/blob/38e290953673ba205f7fbceb2958f1c4721242ef/src/components/TodoItem/index.jsx#L46 > - Either remove unclickable call to actions or change their styles > > https://github.com/emanelshawwa/React-todos-list/blob/38e290953673ba205f7fbceb2958f1c4721242ef/src/components/TodoItem/index.jsx#L48 > > — > You are receiving this because you are subscribed to this thread. > Reply to this email directly, view it on GitHub > <#1>, or > unsubscribe > <https://github.com/notifications/unsubscribe-auth/AGK54E6WZZI234URGIKXZIDSLGDLRANCNFSM4SUKLNBA> > . >