Start pull requests #1
Conversation
Methods for open ContactDisplayForm, ContactEditForm on ContactListExplorer, some refactoring
There was a problem hiding this comment.
Код просмотрел. Мелочи указал. Общая картина будет с нарастанием функционала. Лучше делать более частые и мелкие пул реквесты.
Из общих рекомендаций:
Весь закомиченный код должен не кидать ворнингов. Так как это значит с большой вероятностью, что где-то ошибка.
| "version": "0.1.0", | ||
| "private": true, | ||
| "dependencies": { | ||
| "axios": "^0.18.0", |
There was a problem hiding this comment.
axios отлично справляется с задачей, но нативный fetch ничем не хуже и ничего не надо подключать, просто на будущее
| <div> | ||
| <h2>Home</h2> | ||
| </div> | ||
| ) |
There was a problem hiding this comment.
Понятно что это всё ещё будешь перелопачивать), но просто напоминание, что для чистоты только один компонент в одном файле
| about: undefined, | ||
| image: undefined, | ||
| }, | ||
| } |
There was a problem hiding this comment.
Не вижу смысла инициализировать переменные undefined. То же самое что не инициализировать
| this.handleInputChange = this.handleInputChange.bind(this) | ||
| this.handleSubmit = this.handleSubmit.bind(this) | ||
| this.closeCreate = this.closeCreate.bind(this) | ||
| } |
There was a problem hiding this comment.
facebook/react#9851
В кратце есть bind или arrow functions. Второй подход предпочитаю больше так как меньше писать => меньше багов
| const target = event.target | ||
| const value = target.value | ||
| const name = target.name | ||
| if (value !== '') { |
There was a problem hiding this comment.
если нет чёткой необходимости можно заменить на if (value) {
| } | ||
|
|
||
| handleUploadFile () { | ||
| const fileInput = document.getElementById('file') |
There was a problem hiding this comment.
использование id опасно, а если надо 2 одинаковых компонента нарисовать и у обоих одна id?
| } | ||
|
|
||
| handleSubmit () { | ||
| if ((this.state.contact.name === undefined || '') || (this.state.contact.surname === undefined || '')) { |
There was a problem hiding this comment.
тоже самое, необходима ли тут явнfя проверка на пусткю строку и undefined?
| display: flex; | ||
| } | ||
|
|
||
| @media screen and (max-width: 600px) { |
There was a problem hiding this comment.
Лучше юзать mobile first https://zellwk.com/blog/how-to-write-mobile-first-css/ Так как на мобилках открывают больше и они слабее. А медиа квери для больших размеров, а не наоборот
| Personal information: | ||
| </div> | ||
| <div className="contact-display__container__close-button"> | ||
| <img alt="img" src={require('../Popup/icons/close.png')} onClick={this.closeDisplay}></img> |
| import Toast from '../Popup/toast' | ||
| import popup from '../Popup/popup.css' | ||
|
|
||
| class ContactEditForm extends React.Component { |
There was a problem hiding this comment.
Edit и CreateForm оч похожи, возможно можно реюзать
No description provided.