Sprint 3 #6
Merged
Sprint 3 #6
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ Deploy Preview for legendary-cat-a62c0e ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Здравствуйте. (Нужно развернуть общий комментарий ↓)
Посмотрите на гитхабе все комментарии к коду (нужно прокрутить вниз страницу там)
Работа проделана огромная:
Readmeхорошо оформлен- Проект задеплоен и отлично работает
- Отлично, что нет
EOFошибок в гите - Хорошая структура папок и файлов
- Отлично, что указываете понятный текст ошибки под инпутом
- Отлично, что базовый урл вынесен отдельно, чтобы не дублировать его
- Отлично, что ловите возможные ошибки в конце каждого запроса к серверу
- Хороший роутинг
- Хорошая логика сокетов
но есть некоторые недочеты:
- Меню в чате не работает https://disk.yandex.ru/i/e2zepyxSTyvp3Q . Не могу добавить, удалить пользователя из чата или же удалить сам чат. Ничего не происходит при клике на пункты меню чата. Оказалось, что это из-за зависания сайта, так как постоянные запросы и перерисовки чатов происходят каждую секунду. Все-таки работает меню, но очень долго открываются попапы. А через 2-3 минуты совсем все зависает и не реагирует
- Потом вроде открылся попап добавления пользователя, но не могу добавить пользователя в чат. Все время ошибка в консоли вылезает https://disk.yandex.ru/i/LfKzB7JvzDiR6w
- Каждую секунду у Вас подгружаются все чаты. Это тормозит весь сайт. Нужно через сокеты получать новые сообщения, а не путем запроса всех чатов каждую секунду https://disk.yandex.ru/i/2F9BFhgFDETbrg
JSON.parseможет выкинуть исключение, если данные невалидные. Стоит добавить блокtry/catchдля отлова возможной ошибки
Можно лучше
- Попробуйте максимально избавиться от типов
anyв коде. Хотя бы часть из них можно заменить наunknown, если совсем не получается. Чаще всего помогает использование дженериков. Подробнее тут https://habr.com/ru/companies/tbank/articles/588655/ - В моих чатах есть картинка чата, но она не отображается сейчас у Вас. Нужно правильно подключать картинки. Скрин https://disk.yandex.ru/i/IsQAAMYil1_OoA
- Нужна возможность менять картинку чата
| constructor(apiPath: string) { | ||
| this.apiUrl = `${HOST}${apiPath}`; | ||
| } | ||
|
|
||
| get: HTTP = (url = '', options = {}) => { |
| @@ -0,0 +1,9 @@ | |||
| const HOST = 'https://ya-praktikum.tech/api/v2'; | |||
There was a problem hiding this comment.
- Отлично, что базовый урл вынесен отдельно, чтобы не дублировать его
|
|
||
| this.navigate('ChatPage'); | ||
| await AuthController.login(userData); | ||
| } catch (error) { |
There was a problem hiding this comment.
- Отлично, что ловите возможные ошибки в конце каждого запроса к серверу
src/core/Socket.ts
Outdated
| } | ||
|
|
||
| public message(event: MessageEvent) { | ||
| const data = JSON.parse(event.data); |
There was a problem hiding this comment.
JSON.parse может выкинуть исключение, если данные невалидные. Стоит добавить блок try/catch для отлова возможной ошибки
gennady-bars
approved these changes
Aug 25, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.