Skip to content
This repository has been archived by the owner on Feb 5, 2023. It is now read-only.

change style for table__active and table__archived in manage-users an… #152

Merged
merged 5 commits into from
Jul 26, 2021

Conversation

AnastassiaVarabei
Copy link
Contributor

…d department-card

@tsinevik tsinevik self-requested a review July 25, 2021 19:33
Comment on lines 46 to 63
&__active {
width: 67px;
height: 24px;

text-align: center;

background-color: #00e676;
border-radius: 10px;
}

&__archived {
background-color: #eb5757;
border-radius: 5%;
width: 67px;
height: 24px;

text-align: center;

background-color: #f5f5f5;
border-radius: 10px;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Здесь дублируются стили, лучше вынеси повторяющиеся и цвет, например, активного статуса в отдельный класс типа __status, а для "в архиве" сделай булевый класс-модификатор по БЭМу типа __status_archived.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Сделала отдельный класс

Comment on lines 44 to 61
&__active {
width: 67px;
height: 24px;

text-align: center;

background-color: #00e676;
border-radius: 10px;
}

&__archived {
background-color: #eb5757;
border-radius: 5%;
width: 67px;
height: 24px;

text-align: center;

background-color: #f5f5f5;
border-radius: 10px;
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
Contributor Author

Choose a reason for hiding this comment

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

Сделала отдельный класс

Comment on lines 81 to 84
<p class="table__status table__status__active text_color_white small_text">Активен</p>
</ng-container>
<ng-template #archived>
<p class="table__archived text_color_gray1 small_text">В архиве</p>
<p class="table__status table__status__archived text_color_gray1 small_text">В архиве</p>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Смотри, мы называем классы по методологии БЭМ, по этой методологии все компоненты состоят из трёх составляющих: блок, элемент и модификатор. Более подробно про это рекомендую здесь почитать: https://ru.bem.info/methodology/quick-start/. Если коротко, то классы мы пишем в следующем формате:

  1. имя-блока__имя-элемента_имя-модификатора_значение-модификатора
  2. имя-блока__имя-элемента_значение-модификатора

В твоём случае table – это имя блока, status – имя элемента, active и archived – значения модификаторов, то есть они должны писаться через нижнее подчеркивание. Более того, раз у тебя два модификатора отвечают за цвет фона одного и того же элемента, то есть нельзя применить одновременно оба стиля, то в таком варианте нужно ещё перед active и archived прописать какое-то имя модификатора, например, table__status__bg__active или &__bg__archived.
Но мне это нагромождение не очень нравится, поэтому переходим к следующему комментарию.

Comment on lines 46 to 60
&__active {
&__status {
width: 67px;
height: 24px;

text-align: center;

background-color: #00e676;
border-radius: 10px;
}

&__archived {
width: 67px;
height: 24px;

text-align: center;
&__active {
background-color: #00e676;
}

background-color: #f5f5f5;
border-radius: 10px;
&__archived {
background-color: #f5f5f5;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Мне кажется, лучше сделать так: убрать класс __active и перенести его значение в __status, т.е. для значение "активен" мы применяем только класс __status. А __archived оставить и для значения "в архиве" будем применять уже два класса: класс элемента __status и класс с модификатором этого элемента __status_archived – таким образом, код станет чище, я считаю.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

исправила

Comment on lines 63 to 66
<p class="table__active text_color_white small_text">Активен</p>
<p class="table__status table__status__active text_color_white small_text">Активен</p>
</ng-container>
<ng-template #archived>
<p class="table__archived text_color_gray1 small_text" >В архиве</p>
<p class="table__status table__status__archived text_color_gray1 small_text" >В архиве</p>
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
Contributor Author

Choose a reason for hiding this comment

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

исправила

Comment on lines 44 to 58
&__active {
&__status {
width: 67px;
height: 24px;

text-align: center;

background-color: #00e676;
border-radius: 10px;
}

&__archived {
width: 67px;
height: 24px;

text-align: center;
&__active {
background-color: #00e676;
}

background-color: #f5f5f5;
border-radius: 10px;
&__archived {
background-color: #f5f5f5;
}
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
Contributor Author

Choose a reason for hiding this comment

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

исправила

@tsinevik tsinevik merged commit 9b54ddc into develop Jul 26, 2021
@tsinevik tsinevik deleted the improvement/style-status-value branch July 26, 2021 16:53
@tsinevik tsinevik added this to the v0.3.0 milestone Aug 31, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants