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

fix(): mark battle comments as new #1084

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

zyuhel
Copy link
Contributor

@zyuhel zyuhel commented Apr 11, 2023

Родительский коммент aka мнение в батлах, тоже должен выделяться. Из минусов текущего pr'ы, он ломает цвет этого самого мнения, нужно либо поправить стили тогда выходит, либо добавлять не comment-is-new а другой класс.
@vas3k как лучше сделать? потому что серые мнения в батлах это какой то минус к ux'у

@zyuhel zyuhel requested a review from vas3k as a code owner April 11, 2023 16:43
@zyuhel
Copy link
Contributor Author

zyuhel commented Apr 11, 2023

This pr is for #1070

@vas3k
Copy link
Owner

vas3k commented Apr 11, 2023

Да хз, цветом он уже выделяется, а ломать стандартные цвета как-то не хочется

@zyuhel
Copy link
Contributor Author

zyuhel commented Apr 11, 2023

Ну там раньше он был либо зеленым, либо синим, теперь для новых мнений становится серым, нормальным цветом для непрочитаного, при этом по цвету заголовка (синего, зеленого) понятно в какую сторону мнение.

можно сломать для батлов это выделение серым для нового, благо его и не было раньше, и тогда я назову новый класс например comment-type-battle-is-new и поправлю arrowscroll и тогда arrowscroll будет переходить на новые мнения, но при этом внешне они будут выглядеть как раньше.

а можно выделять серым как и все новые комменты, и тогда можно принять pr как есть.

@vas3k
Copy link
Owner

vas3k commented Apr 11, 2023

Без скриншотов непонятно :(

@zyuhel
Copy link
Contributor Author

zyuhel commented Apr 11, 2023

Screenshot 2023-04-11 at 19 44 39

@vas3k
Copy link
Owner

vas3k commented Apr 11, 2023

Ну вот примерно такого я и ожидал. Некрасиво ломать первичную логику ради вторичной, не нравится :(

@zyuhel
Copy link
Contributor Author

zyuhel commented Apr 11, 2023

Ну значит поправил, теперь визуально все останется так как и было.
у выделения серым была своя логика, но хозяин барин

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