Merged
Conversation
8 tasks
f67a870 to
edc4ce9
Compare
edc4ce9 to
1d4684e
Compare
Mileeena
previously approved these changes
Sep 9, 2021
8fec19f to
ca653dc
Compare
EvgeniyLyapunov
previously approved these changes
Sep 12, 2021
Collaborator
EvgeniyLyapunov
left a comment
There was a problem hiding this comment.
Дима, привет. Всё работает. Ты молодец!
Есть мнение, что комментарии в коде не нужны, они просто захламляют, надо писать код так, что бы он был понятен без комментариев.
Мне не хватает комментариев. При обЪявлении переменной или свойства, при описание метода.
Kozlov-AE
previously approved these changes
Sep 13, 2021
Contributor
Kozlov-AE
left a comment
There was a problem hiding this comment.
В целом нормально. Работает вроде хорошо)
UnluckyD
previously approved these changes
Sep 13, 2021
899822d
5d4c197 to
c6cca86
Compare
Поправил разметку для всех сообщений. Сделал новое контекстное меню
Удалил лишние классы, а вместо них использовал ReactiveUI Также переписал логику получения комманд для ContextMenu
Переименовал свойства, чтобы их названия более наглядно отображили их суть. Добавил автомаппер для MessageMold и MessageViewModel
Визуально поправил код в Message, Messages и ReplyMessages, чтобы было легче читать код. Добавил оранжевую подсветку при редактировании сообщения. Добавил комментарии к коду. Сделал рефакторинг HubMessage, IChatHub, удалив ненужные свойства и строки.
4710125 to
7bcc568
Compare
Kibnet
requested changes
Sep 17, 2021
Contributor
Kibnet
left a comment
There was a problem hiding this comment.
Работает хорошо. Проделана очень большая и сложная работа.
- Есть пару багов, отметил в комментах знаком 🪲.
- В разных местах одно и тоже названо по разному, нужно привести к единому "Quoted".
- Есть несколько мест с откровенным копипастом, нужно вынести в методы для переиспользования.
8e77b02 to
1316d65
Compare
1316d65 to
5a2886f
Compare
9c23fa2 to
ddb50c7
Compare
Kibnet
approved these changes
Sep 22, 2021
| try | ||
| { | ||
| messageDictionary.Clear(); | ||
| Messages.Clear(); |
Contributor
There was a problem hiding this comment.
Тут нужно очищать цитируемое сообщение
Comment on lines
+28
to
+32
| <ItemGroup> | ||
| <Compile Update="Views\QuotedMessage.axaml.cs"> | ||
| <DependentUpon>QuotedMessage.axaml</DependentUpon> | ||
| </Compile> | ||
| </ItemGroup> |
Comment on lines
+201
to
+222
| /// <summary> | ||
| /// Получает список вложений | ||
| /// </summary> | ||
| /// <param name="message"></param> | ||
| /// <returns></returns> | ||
| private async Task<List<AttachmentHubMold>> GetAttachments(Message message) | ||
| { | ||
| var Attachments = new List<AttachmentHubMold>(); | ||
| if (message.Attachments != null) | ||
| { | ||
| var attach = await _ravenSession.LoadAsync<Attachment>(message.Attachments); | ||
|
|
||
| foreach (var id in message.Attachments) | ||
| { | ||
| if (attach.TryGetValue(id, out var attachment)) | ||
| { | ||
| Attachments.Add(Mapper.Map<AttachmentHubMold>(attachment)); | ||
| } | ||
| } | ||
| } | ||
| return Attachments; | ||
| } |
Contributor
There was a problem hiding this comment.
Почти полный копипаст из ChatService. Можно было бы вынести общий метод без финального маппинга, а маппинг у же по месту применять через Select
Comment on lines
+231
to
+233
|
|
||
|
|
||
|
|
| { | ||
| Id = quotedMessage.Id, | ||
| UserLogin = user.Login, | ||
| UserNickname = user.DisplayName, |
Contributor
There was a problem hiding this comment.
Рассинхрон, в одном месте UserNickname, в другом DisplayName. Нужно единообразие.
| <PackageReference Include="ServiceStack.Api.OpenApi" Version="5.11.0" /> | ||
| <PackageReference Include="ServiceStack.Api.Swagger" Version="5.11.0" /> | ||
| <PackageReference Include="SignalR.EasyUse.Server" Version="0.2.1" /> | ||
| <PackageReference Include="Splat" Version="13.1.10" /> |
Contributor
There was a problem hiding this comment.
Splat предназначен только для клиентских приложений из-за своего удобства. На сервере есть и так встроенные средства для инверсии зависимостей. Нужно выпилить.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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.
Добавил новую фичу "Цитирование сообщения"
Поправил дизайн для сообщений.
Изменил SendMessageControl.
Наконец добавил дизайн для ContextMenu и ItemMenu.