Conversation
vadim-ilchenko
left a comment
vadim-ilchenko
left a comment
There was a problem hiding this comment.
Принимаю. Оставил комментарии по возможным улучшениям
| <input | ||
| type="checkbox" | ||
| checked={checkedItems.length === filteredItems.length} | ||
| onChange={onMainCheck.bind(this)} |
There was a problem hiding this comment.
какой-то здесь странный bind. Оно без него не работает?
| <td> | ||
| <input | ||
| type="checkbox" | ||
| checked={checkedItems.includes(item.id)} |
There was a problem hiding this comment.
тут можно в item записывать item.checked
| <input | ||
| type="checkbox" | ||
| checked={checkedItems.includes(item.id)} | ||
| onChange={onHandleCheck.bind(this)} |
There was a problem hiding this comment.
здесь тоже не понятно, что за bind
| ← | ||
| </button> | ||
| )} | ||
| {pages.map(item => { |
There was a problem hiding this comment.
Не совсем оптимальное решение через map. Для того, чтобы показать 3 страницы, ты проходишь по массиву из всех страниц. Думаю можно оптимизировать
| if (checkedItems.includes(id)) { | ||
| newArr = checkedItems.filter(item => item !== id); | ||
| } else { | ||
| newArr = [...checkedItems, id]; |
There was a problem hiding this comment.
3 прохода по массиву за 3 строки.
id можно передавать при вызове функции, это избавит от event.target.value
Вместо создания массива checkedItems можно создать поле checked у каждого айтема и работать с ним. Все равно ты в componentDidMount записываешь в state список телефонов. Можно уже с ним работать, как хочешь
| checkedItems: newArr, | ||
| isMainChecked: event.target.checked | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Если использовать item.checked, то можно здесь обойтись одним mapом:
this.setState(items: items.map(item => ({...item, checked: !event.target.checked}))})
2 participants
Preview
Pull