Skip to content
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

dev-35: сделал основную логику чатов на фронте, связал с бэкендом #25

Open
wants to merge 21 commits into
base: dev
Choose a base branch
from

Conversation

AlexNov03
Copy link
Collaborator

No description provided.

@AlexNov03 AlexNov03 changed the title сделал основную логику чатов на фронте, связал с бэкендом dev-35: сделал основную логику чатов на фронте, связал с бэкендом Nov 18, 2024
@AlexNov03 AlexNov03 requested a review from Gallaann November 18, 2024 14:25
Copy link
Collaborator

@Gallaann Gallaann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Много лишних консоль логов, а так же нужно выносить логику из компонента, когда это возможно (Сетевые запросы, например. Их код не должен находиться внутри компонента, достаточно просто вызова 1 функции, в идеале вообще без параметров)

public/components/complex/messagelist/messagelist.css Outdated Show resolved Hide resolved
{{#each Messages}}
{{{this}}}
{{/each}}
</div>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Очень нелюблю значок в конце файла в гит диффе, добавь плиз перевод строки в конец файла

public/components/dialog-preview/dialog-preview.hbs Outdated Show resolved Hide resolved
public/components/dialog-preview/dialog-preview.hbs Outdated Show resolved Hide resolved
public/components/message/message.css Outdated Show resolved Hide resolved
public/pages/chat/chat.js Outdated Show resolved Hide resolved
public/pages/editPin/editPin.js Outdated Show resolved Hide resolved
public/pages/editPin/editPin.js Outdated Show resolved Hide resolved
public/pages/editPin/editPin.js Show resolved Hide resolved
public/pages/main/main.js Outdated Show resolved Hide resolved
Copy link
Collaborator

@Gallaann Gallaann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Почти у цели. Ты же понимаешь, что localhost:8000 не будет работать на устройствах пользователей, потому что они не запустили на своих устройствах сервер на ноде с вашим приложением. Вот поэтому добавь нормальные урлы в public/constants/api.js и используй их, там есть функция определения текущего айпишника (там это BASE часть урла). Надеюсь, что ты поправишь это небольшое замечание сам, поэтому сразу ставлю аппрув

Comment on lines +39 to +41
const response = await postMethod('http://localhost:8080/users/by/params', {
nick_name: nickNameVal,
});
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ну это ерунда какая-то. что за локальный адрес. B добавь в api consts путь к своему методу.

const template = Handlebars.templates['chat.hbs'];

const userIDResponseData = await getMethod(
'http://localhost:8080/is_authorized'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Опять. Некрасиво все это как-то. Смотри файл public/constants/api.js, там легко добавить ручки

inputImageRight: svgIcon,
});

const userChats = await getMethod('http://localhost:8080/mychats');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

public/constants/api.js

async AddUserChat(event) {
const companionID = event.currentTarget.dataset.companionId;
const response = await postMethod(
`http://localhost:8080/create/chat/${companionID}`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

public/constants/api.js

dialog.addEventListener('click', this.getChatMessages);
});

this.socket = new WebSocket('ws://localhost:8080/handshake');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Добавить в public/constants/api.js вебсокет урл


this.currentChatID = event.currentTarget.dataset.chatId;
const urlPath =
'http://localhost:8080/chat/' +
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

public/constants/api.js

body: requestBody,
});

console.log(requestBody);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Убрать

description: DescriptionValue,
title: TitleValue,
media_url: ImageUrl,
});
console.log(requestBody);

await fetch('http://localhost:8080/create-pin', {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

network.js + public/constants/api.js

Comment on lines 53 to 56
}).catch(() => {
return undefined
});

return response;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Не могу нормально выделить. В общем, не .then((response, shouldLog) => { а .then((response) => {
Потому что в круглых скобочках стрелочной функции в конструкци then( () => {} ) мы указали какие параметры она должна принять из промиса. НО, промис не отдает фторым параметром булевое значение, мы его сами передаем в более верхнеуровневую функцию getMethod и его-то нам и нужно уже передать дальше в handleResponse

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants