Conversation
|
This pull request is automatically deployed with Now. Latest deployment for this branch: https://task-5-git-fork-airosso-master.itmo-yandex.now.sh |
mokhov
left a comment
mokhov
left a comment
There was a problem hiding this comment.
Основное замечание – это выбор разбиения на компоненты (всё в App), нужно поправить
| } | ||
|
|
||
| export class App extends Component<{}, AppState> { | ||
| readonly state: AppState = { |
There was a problem hiding this comment.
Логику работы с письмами (выделение, удаление, открытие) нужно перенести из app в компонент «Список писем».
There was a problem hiding this comment.
Измененное задание нужно сдать очно?
|
|
||
| componentDidMount() { | ||
| const this2 = this; | ||
| (function sendEmails([time1, time2]: [number, number]) { |
There was a problem hiding this comment.
Это можно вынести в функцию вне компонента, т.к это синтетический код для задания
|
|
||
| deleteSelected = () => { | ||
| const deletedKeys = this.state.letters.filter(x => !!x.selected).map(x => x.key); | ||
| this.setState(({ letters }) => { |
There was a problem hiding this comment.
Для удобства обычно в setState называют prevState. По аналогии можно называть и прошлое состояние конкретного поля. Тогда не будет путанице в нейминге
| }); | ||
| }; | ||
|
|
||
| toggleLetter = (id: number) => { |
There was a problem hiding this comment.
Вот тут написано плохо. toggle должен быть методом письма. Сейчас и знание об зачекнутости должно быть внутри него, а наружу это должен быть state письма. Тогда не будет этого прохода по списку писем. Почему это плохо? Потому что чем больше писем, тем больше проходов по всему списку, тем медленнее работает приложение
2 participants
No description provided.