02-deep-in-components - ToDo list

[maplemap]

Есть вопросы касательно раздела 02-deep-in-components, а конкретно по to-do list и поиску по заметкам.
Код на jsfiddle.

1. При поиске или сортировке данных в приложении, происходит полное изменение состояния (state), в котором хранится массив данных, а также происходит переписывание localStorage. Поэтому на данном этапе необходимо сохранять (не перезаписывая при следующей сортировке) в промежуточной переменной весь массив данных (метод onToDoSort, строка 132). Более того, необходимо еще отслеживать (метод componentDidUpdate, строка 166) запись в localStorage, иначе, если этого не делать, в состоянии сортировки при добавлении новых элементов и следующей перезагрузки страницы, происходит затирание неотсортированных данных.
   Насколько такой подход верен, существует ли простой способ отслеживания этого момента? Интуитивно происходит стремление к более понятному и простому, может ли React это предоставить?
2. В to-do list приложении при сортировке происходит изменения цвета шрифта соответствующих пунктов сортировки All New Completed. Я задал изначальную сортировку (метод componentWillMount, строка 162) и потом производил изменение ее (метод onToDoSort, строка 150) с передачей в соответсвующий компонент (метод render, строка 171). И уже в компоненте, подставляя необходимый стиль (метод render, строка 83), руководил изменением цвета шрифта All New Completed.
   Этот способ также сработал как и решение в предыдущем абзаце, но опять же интуитивно кажется мне весьма запутанным. Существует ли другой подход к решению этой задачи?

[krambertech]

Есть несколько замечаний:

1. Необязательно писать закрывающийся тег: <span></span> --> <span />

2. По поводу форматирования кода рекомендую почитать Airbnb Javascript Style guide (https://github.com/airbnb/javascript). Там есть раздел о Реакт. В таком виде его сложно читать и не работает подсветка в редакторе.

3. В компоненте ToDoAdd лучше хранить текст в состоянии, а не пользоваться ref

4. По поводу sortCopyTasks. Смотри, можно в state хранить todos и filter. И сделать метод, который из двух этих значений будет отдавать тебе нужные задачи прямо в render. Без никаких переменных.

Типа такого:

getVisibleToDos(todos, filter) {
    if (filter === 'COMPLETED') {
       return todos.filter(todo => todo.isCompleted);
   }

   if (filter === 'NEW') {
       return todos.filter(todo => !todo.isCompleted);
   }

   return todos;
}

[krambertech]

А по форматированию кода почитай, это может казаться неважным, но на самом деле очень важно. Потому что я, когда открыла, сначала несколько минут просто пыталась разобрать, что там написано 😬

[maplemap]

Прошу прощения, что заставил страдать при взгляде на код. (-_-)
Спасибо за отличную идею об избавлении от ненужных переменных. Теперь выглядит замечательно!
Поправил код и перевел в ES6 в codepen.io
Мои благодарности за поддержку, Катерина!

[krambertech]

Круто 👍

Мне нравится :)

[trofivan]

Немного увлёкся и получился такой ToDo List:

• Github: https://github.com/trofivan/react-essential-course/tree/master/02-deep-in-components/homework/04
• Demo: https://jsfiddle.net/trofivan/4715a1rp/

[N4G1B4T0R]

@trofivan Ола, я посмотрел твой to-do-list, крутая реализация, только я вот заметил 1 багу. Это когда ты вводишь ничего, оно добавляет таск. Я вот бы добавил валидность для этого ввода или placeholder. Ну это уже твое дело. А так все супер.

[trofivan]

@N4G1B4T0R спасибо, поправил.

[krambertech]

@trofivan Еще у меня не работает фильтр "Активные"

Все задачи. Видно, что в списке 3 активные.

Но с фильтром ничего нет:

[krambertech]

По коду там есть проблемы

1. Использование const и let. Мне не совсем понятна логика. Много мест, где ты используешь let, а можно было бы const. Если значение переменной не меняется - используй const.

2. Выравнивание в JSX. Нужно стараться, чтобы выравнивание везде было одинаковым. Например, я обычно пишу так:

// short
<Cat>Fluffy</Cat>

// longer
<Cat color="grey" hungry>
    Fluffy
</Cat>

// if you have a lot of props and children
<Cat
    color="grey"
    hungry
    weight={3}
    mood="sleepy"
>
    Fluffy
</Cat>

// one component with a lot of props
<Cat
    color="grey"
    hungry
    weight={3}
    mood="sleepy"
/>

3. мне вообще не нравится как у тебя реализована фильтрация. выглядит неочевидно + ты все время передаешь объекты в props. а это все-таки деоптимизация. это можно сделать намного проще.

4. по code-style

почему тут tasks в кавычках?

можно просто setState({ filter }); (ES6 object property shorthand)

тут можно использовать оператор ... [ ...tasks, newTask ]

колбеки лучше называть с приставкой on

почему тут просто не использовать map?

тут лучше перенести текст в состояние, а не чистить его прямо в DOM элементе. (не сильно критично)

тут тело ф-ции можно не писать вобще

обработчики лучше называть с приставкой handle

принято ставить пробел перед />, так код намного лучше читается

колбеки лучше располагать в конце объявления props. так потом код намного проще читать. в тех же целях, многие любят сортировать props по алфавиту (но колбеки все так же в конце)

я сторонник все-таки ставить {} в if всегда, если не в одну строку

это то, что сразу заметила. внимательно не смотрела 😉

[trofivan]

@krambertech спасибо за развёрнутый ответ, поправлю.
И странно, что у тебя не работает фильтр, у меня в Chrome и IE на винде всё нормально отрабатывает.

[trofivan]

@krambertech а в чём преимущество использования .map перед .forEach в моём коде?
С .map получается такая же конструкция, что и с forEach, или я что-то не так понял?

[trofivan]

@krambertech ты в каком браузере тестировала и на какой ОС? Хочу проверить у себя почему переключение фильтра не работает. Проверил на винде в Хроме, IE, FireFox - всё работает.

[krambertech]

@trofivan почитай документацию map и forEach и используй их по назначению. map проходится по массиву и возвращает новый массив, forEach же просто проходится по массиву.

И ты сейчас мутируешь свой массив по ссылке, это всегда плохо :)

[krambertech]

@trofivan macOS, Chrome 54.0.2840.71 (64-bit)

[dmitry-shishkin]

У меня получился вот такой список задач: https://codepen.io/shugich/pen/NapxyZ
Он же на Гитхабе: https://github.com/shugich/react-todo

Я хотел повешать сначала добавление задачи на onsubmit для формы, так как мне это кажется правильным и логичным. Но в итоге я так и не разобрался как отменять стандартное действие. event.preventDefault() тут не срабатывает.

[trofivan]

@shugich не знаю, почему у Вас не работал вариант с .preventDefault(), может с аргументом ошиблись. Вот переписал: https://codepen.io/trofivan/pen/YrZbBm?editors=0010
И если вы используете контролируемую форму и храните текст в state, то при добавлении его в базу тоже берите его из state, а не завязывайтесь на элемент.

[dmitry-shishkin]

Странно, наверное я что-то не так делал. Спасибо большое. И за замечания про использование стейта, это так очевидно, а я почему-то упустил :-(