remake newGame notification, add join button #43
Conversation
src/background/browser-actions.js
Outdated
| const popupNotification = new Notification('Notification', notifications[notification]); | ||
| console.log('showNotification', storage); | ||
| const currentNotification = { ...notifications[notification], message }; | ||
| const popupNotification = new Notification(id.toString(), currentNotification); |
There was a problem hiding this comment.
Вот этот кусок кода я не понимаю. Из старых уведомлений взять сущность, добавить туда сообщение, и создать новое уведомление?
There was a problem hiding this comment.
Ну типа в дефолтном уведомление заменить нужное, как лучше сделать видишь?
There was a problem hiding this comment.
Ну вот смотри.
- Я специально создал файл с моделями, чтобы держать там все шаблоны, и т.п.
- Есть класс Уведомление, у которого есть дефолтный шаблон. Ты можешь поправить реализацию класса, чтобы он получал шаблон и данные которые надо отобразить.
- В идеале, надо чтобы у нас была очередь сообщений, не больше Н числа. И обновляются они по принципу стека.
There was a problem hiding this comment.
Что есть очередь сообщений? и зачем она? У нас уникальные ID
There was a problem hiding this comment.
очереди пока нет. Это была "преждевременная оптимизация"
| if (gameStatus === gameStatuses.waiting) { | ||
| animateBadge(); | ||
| showNotification('newGame'); | ||
| const message = `Player - ${action.payload.players[0].name} || game level - ${action.payload.level} || status - ${action.payload.state} `; |
There was a problem hiding this comment.
Вот эта строка мне кажется не должна зависеть от кол-ва игроков. Это больше придирки, но тут информация по игре, и отдельно - статус игры.
There was a problem hiding this comment.
Где- то про это общались в игре может быть только 2 игрока и players[0] всегда создатель
Если строку делать независимой я не узнаю имя создателя
|
Переделал на функцию, но возникли проблемы с addListener которые создавались каждый раз и множились. Пришлось сделать логику по их очищению. Есть сомнения что перемудрил |
src/background/browser-actions.js
Outdated
| const currentNotification = { ...notifications[notification], message }; | ||
| const popupNotification = new Notification(id.toString(), currentNotification); | ||
| popupNotification.addListener(); | ||
| createNotification(gameID.toString(), notifications[notification], message, gameID); |
There was a problem hiding this comment.
Может вместо notifications будет templates ?
There was a problem hiding this comment.
templates вроде как ничего не дает как информацию, и такие названия крайне не желательны типа const array =[] , нет?, templateNotifications но длинно получается
|
У меня вложенность из за того что внутренний обработчик должен имень доступ к переменным |
|
Переделал на советы сделать один/общий листенер, через хак передачи через ID |
|
Starting your workflow run... |
#19