-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: исправление кнопки отправки формы на странице контакты #485
Conversation
src/pages/contacts/contacts.tsx
Outdated
@@ -17,6 +18,10 @@ import { validEmailRegexp } from 'shared/constants/regexps'; | |||
|
|||
import type { NextPage } from 'next'; | |||
|
|||
interface ErrorResponse { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
в fetcher уже есть такой тип, можно экспортировать
@@ -1,3 +1,4 @@ | |||
/* eslint-disable camelcase */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
переименуй ветку как bugfix/....., писал об этом в чатике
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Да, тут окей, думал нет соглашения в проекте, давай тогда ему придерживаться
src/pages/contacts/contacts.tsx
Outdated
@@ -162,10 +167,12 @@ const Contacts: NextPage = () => { | |||
}, | |||
body: JSON.stringify(data), | |||
}); | |||
} catch ([status, errors]) { | |||
} catch (err) { | |||
// TODO: добавить проверку типов выброшенного исключения, пока считаем, что всегда получаем ответ API |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
И давай раз уж тут есть TODO, сделаем ее
- Добавь тут обработку исключения
- И чтобы не использовать as давай будем проверять, что ошибка соответствует определенному типу (сделай это функцией чтобы мы использовали ее и в других компонентах), а такая уже есть... isHttpRequestError
src/services/fetcher/fetcher.ts
Outdated
@@ -27,7 +27,10 @@ const fetchResource = (httpClient: typeof fetch) => async <T>(path: string, opti | |||
export const fetcher = fetchResource(fetch); | |||
|
|||
async function handleResponse<T>(response: Response) { | |||
const payload = await response.json(); | |||
let payload; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
я так понимаю, что ты это добавил потому что бэк ответа не присылает?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ага. Бек присылет пустой body, поэтому без этой проверки возникает ошибка.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Так, ну тут бы я не стал завязываться на статус, сейчас в твоей условии если где-то придет статус 201 с телом, то он проигнорируется, плюс разработчикам не очевидно что это такое
Ломается у тебя тут на парсинге поэтому есть несколько вариантов это поправить, тебе тут надо добавить проверку или обработка исключений, попробуй подумать сам, если что приходи
src/services/fetcher/fetcher.ts
Outdated
} | ||
|
||
return payload as T; | ||
return response.json() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Да, тут все окей, только думаю лучше будет выглядеть с try catch и async/await чтобы все красиво было и придерживалось кодстайла
}, | ||
}); | ||
} | ||
if (statusCode === 400) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
а почему тут добавилась проверка на 400 статус?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Тут я разделил ошибки по статусу. В случае 400 с сервера приходят тексты ошибок и мы меняем сотояние формы, а если какая-либо другая будет, то нет.
…ровка функции handleResponse)
* fix: исправление поля текст сообщения на странице контакты * fix: исправление кнопки отправки формы на странице контакты (#485) * fix: исправление кнопки отправки формы на странице контакты * fix: исправление кнопки отправки формы на странице контакты (доработка по замечаниям ревью) * fix: исправление кнопки отправки формы на странице контакты (ревью) * fix: исправление кнопки отправки формы на странице контакты (корректировка функции handleResponse) * fix: удалены пропсы ограничивающие высоту и ширину в textarea * fix: исправление поля текст сообщения на странице контакты * fix: удалены пропсы ограничивающие высоту и ширину в textarea * fix: правки после ревью --------- Co-authored-by: Aleksandr Dronov <[email protected]>
Описание
Исправление кнопки отправки формы на странице контакты
Ссылка на задачу
На странице контакты. При нажатии кнопки "Отправить" ничего не происходит