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

Dz 2 #3

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

Dz 2 #3

wants to merge 6 commits into from

Conversation

mpoplavkov
Copy link
Collaborator

No description provided.

Copy link

@DmitryMakhnev DmitryMakhnev left a comment

Choose a reason for hiding this comment

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

Переработать согласно разбору ДЗ

left: 7px;
}

.todo-item_checkbox input[type="checkbox"] {

Choose a reason for hiding this comment

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

Не нужно использовать селекторы по тегу. Используем класс

visibility: hidden;
}

.todo-item_checkbox input[type="checkbox"]::before {

Choose a reason for hiding this comment

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

Синхронизировать с разбором в ДЗ

}

.todo-item:hover .todo-item_remove {
opacity: 0.7;

Choose a reason for hiding this comment

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

Как навести на элемент который невидим?)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@DmitryMakhnev так opacity же не 0, значит он видим, разве нет?)
screenshot from 2017-05-13 21 51 18

overflow: hidden;
}

.__ready textarea {

Choose a reason for hiding this comment

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

Нельзя использовать селектор по модификатору вне контекста блока

@@ -0,0 +1,6 @@
.todo-item {
display: block;
height: 50px;

Choose a reason for hiding this comment

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

Высота должна быть динамической

@@ -0,0 +1,16 @@
.todos-filter {
background-color: inherit;

Choose a reason for hiding this comment

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

В таком случае непонятно откуда возьмётся цвет

}

.todos-filter:hover {
background-color: #fafafa;

Choose a reason for hiding this comment

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

Вроде бы на ховер не было каких-то стилей

@@ -0,0 +1,9 @@
.todos-add_input {
font-style: italic;

Choose a reason for hiding this comment

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

Italic только для плейсхолдера см https://css-tricks.com/almanac/selectors/p/placeholder/


.todos-step1 {
margin: 0 1%;
width: 98%;

Choose a reason for hiding this comment

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

Ширину указывать не надо
Отступы указать в пикселях

styles/reset.css Outdated
@@ -0,0 +1,11 @@
* {

Choose a reason for hiding this comment

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

Использовать reset из репозитория с препроцессорами

Copy link

@DmitryMakhnev DmitryMakhnev left a comment

Choose a reason for hiding this comment

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

Пофиксить замечания

<div class="todos-workspace">
<div class="todos-add">
<button class="todos-add_select-all" aria-label="Отметить как выполненные все"></button>
<input class="todos-add_input" placeholder="What needs to be done?" aria-label="Поле добавления todo">

Choose a reason for hiding this comment

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

Без обёртки будет криво, смотрите разбор дз

@@ -0,0 +1,90 @@
@import "../../config/globals";

Choose a reason for hiding this comment

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

todo-item не базовый блок

}

.todo-item_checkbox {
position: absolute;

Choose a reason for hiding this comment

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

todo item должен быть свёрстан на флоатах и флексах, см разбор дз

top: 0;
width: 100%;
height: 100%;
z-index: 1;

Choose a reason for hiding this comment

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

Эти стили должны быть в базовом блоке чекбокса

<div class="todo-item">
<div class="todo-item_checkbox">
<input type="checkbox"
class="todo-item_checkbox_target"

Choose a reason for hiding this comment

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

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

color: $c-gray;
border: none;
box-sizing: border-box;
width: 100%;

Choose a reason for hiding this comment

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

Тоже не правильно, решение должно быть на флоатах, проблемы решения на абсолютах, я раскрывал на лекциях

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