Feature/#90 add different colors of usernames#108
Merged
Conversation
EvgeniyLyapunov
previously approved these changes
Sep 29, 2021
KlinskiyDD
reviewed
Sep 30, 2021
Collaborator
KlinskiyDD
left a comment
There was a problem hiding this comment.
Все класно работает. Есть пару замечаний в коде. Я думаю, что Паша сможет более точно внести правки.
Правильно ли я понял, что цвета у пользователей должны быть разными при каждом новом запуске? Если да, то этот эффект не достигнут.
b38a795 to
cbb122a
Compare
KlinskiyD
previously approved these changes
Oct 2, 2021
Kibnet
requested changes
Oct 2, 2021
Contributor
There was a problem hiding this comment.
Да, фича работает в основных сценариях. Но нужно исправить:
- Если выбрать сообщение для цитирования, над полем ввода сообщения отображается блок, где есть имя пользователя. Цвет там остался черным, нужно тоже раскрасить.
- Когда я делал коммит с прототипом, там было правильно разнесено по проектам, ты фишку не понял и всё засунул во вьюмодель, надо поправить, чтобы авалонии и следа не было там.
- Некоторые выбранные цвета кажутся слишком светлыми для белого фона. Надо заменить.
0348517 to
6bdeb7c
Compare
Kibnet
requested changes
Oct 4, 2021
Contributor
Kibnet
left a comment
There was a problem hiding this comment.
Я понял что мы пошли неправильным путём. Нужно сделать конвертер, который на входе будет получать через биндинг идентификатор пользователя, а в разметку возвращать цвет. Большая часть проблем сама уйдёт при таком подходе. Соответсвенно текущий код надо переместить в конвертер.
3794cff to
cf927c2
Compare
cf927c2 to
b56e0fd
Compare
KlinskiyDD
previously approved these changes
Oct 5, 2021
Mileeena
previously approved these changes
Oct 6, 2021
EvgeniyLyapunov
previously approved these changes
Oct 6, 2021
Kibnet
requested changes
Oct 6, 2021
Contributor
Kibnet
left a comment
There was a problem hiding this comment.
Выглядит понятно и изящно. Есть пару косячков, лучше исправить.
058ad88
b56e0fd to
058ad88
Compare
058ad88 to
e5d62c5
Compare
Kibnet
approved these changes
Dec 8, 2021
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.
No description provided.