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

Отправка сообщений #22

Merged
merged 21 commits into from
Apr 9, 2024
Merged

Отправка сообщений #22

merged 21 commits into from
Apr 9, 2024

Conversation

oFem1m
Copy link
Collaborator

@oFem1m oFem1m commented Apr 7, 2024

Отправка сообщений с помощью вебсокетов

@oFem1m oFem1m requested a review from johnSamilin April 7, 2024 18:01
@@ -14,7 +14,7 @@
placeholder='Сообщение...'
value='{{inputMessageValue}}'
/>
<button class='input_send chat-input' disabled>-></button>
<button class='input_send chat-input'>-></button>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

спасибо

@@ -1,3 +1,3 @@
<div class="message">
<div class="{{message_owner}}">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Вот тут нет ошибки?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Там теперь логика такая, что если id пользователя совпадает с id отправителя сообщения, то используется класс my_message, для которого уже заготовлены стили

Copy link
Collaborator

@johnSamilin johnSamilin Apr 8, 2024

Choose a reason for hiding this comment

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

так это можно прямо в шаблоне проверить типа через class="{{isCurrentUserOwner ? 'owner' : 'noOwner'}}"

@@ -73,6 +77,7 @@ export default class ChatPage {
throw new Error('Пришел не 200 статус');
}
const profile = response.body.user;
this.UserId = profile.id;
Copy link
Collaborator

Choose a reason for hiding this comment

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

А почему с большой?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

исправил на маленькую

@@ -84,6 +89,20 @@ export default class ChatPage {
this.#messageDrafts[this.#currentChatId] = event.target.value;
};

messageSendHandler = () => {
const inputMessage = this.#parent.querySelector('#input_message').value;
Copy link
Collaborator

Choose a reason for hiding this comment

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

.trim еще бы добавить и возможно удалить xss уже на этом этапе

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

сделал

// Проверяем, что есть сообщение и ID чата
websocketManager.sendMessage(chatId, inputMessage);
setTimeout(() => {
goToPage('/chat?id=' + chatId);
Copy link
Collaborator

Choose a reason for hiding this comment

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

А зачем тут переход?

export let baseUrl = 'localhost:8080';

if (protocol === 'https') {
baseUrl = 'chatme.site/api/v1';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Не согласен, почему от протокола зависит адрес?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

это для деплоя, там ngnix ругался

Copy link
Collaborator

Choose a reason for hiding this comment

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

тогда нужно сделать какой-то параметр, который будет определять, мы в тестовом окружении или в проде. условно, тут же переменная ENV_TYPE, которая в ветке master будет равна PROD, а в остальных - DEV

@@ -13,10 +13,7 @@ function handleRouting() {

export function goToPage(path) {
window.history.pushState({}, '', path);

Copy link
Collaborator

Choose a reason for hiding this comment

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

Почему эту правку убрали?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

некорректно работали стрелочки и некоторые переходы

const chatId = this.#currentChatId; // Получаем ID текущего чата
function escapeHTML(html) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

это должно быть методом класса, а еще лучше - функцией модуля utils, но точно не локальной функцией

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

исправил

(chat) => chat.id === message.chat_id,
);
if (chatIndex !== -1) {
const chat = this.#chats.splice(chatIndex, 1)[0];
Copy link
Collaborator

Choose a reason for hiding this comment

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

проще сделать сортировку через css (свойство order)

@oFem1m oFem1m merged commit 282572d into develop Apr 9, 2024
1 check passed
@oFem1m oFem1m deleted the sending_messages branch April 9, 2024 20:25
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.

2 participants