Skip to content

Козлова Валерия#13

Open
OnaHochetNaMars wants to merge 21 commits into
urfu-2016:masterfrom
OnaHochetNaMars:master
Open

Козлова Валерия#13
OnaHochetNaMars wants to merge 21 commits into
urfu-2016:masterfrom
OnaHochetNaMars:master

Conversation

@OnaHochetNaMars

@OnaHochetNaMars OnaHochetNaMars commented Nov 28, 2016

Copy link
Copy Markdown

@honest-hrundel

Copy link
Copy Markdown

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel

Copy link
Copy Markdown

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel

Copy link
Copy Markdown

🍏 Пройден линтинг и базовые тесты

@honest-hrundel

Copy link
Copy Markdown

🍏 Пройден линтинг и базовые тесты

Comment thread index.html Outdated
<div class="content">
<div class="img">
<label for="card-2">
<img src="images/img2.jpg" alt="">

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

❗️ alt не должен быть пустым. Изображение можно назвать понятней)

Comment thread index.html Outdated
<input type="radio" name="gallery" id="card-6" class="card-6">
<input type="radio" name="gallery" id="card-7" class="card-7">
<br>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

❗️ Хочется семантичных тегов

Comment thread index.html Outdated
<div class="card card-6">
<input type="checkbox" id="show-recipe-6">
<div class="content">
<div class="img">

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

❗️ Можно придумать более конкретные названия для классов, чем content и img.

Comment thread index.html Outdated
<h1>Название рецепта</h1>
<div>
<div class="ingredients">
Ингридиенты

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ингредиенты

Comment thread index.html Outdated
</div>
<div class="recipe-description">
<label class="close" for="show-recipe-5">X</label>
<h1>Название рецепта</h1>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

❗️ h1 может быть на странице только 1 и должен быть заголовком всей страницы

Comment thread index.css Outdated
transition: all .4s ease-in-out;
box-sizing: border-box;
padding-top: 30%;
z-index: -1;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

А зачем z-index, если и так opacity: 0?

Comment thread index.css Outdated
z-index: 10;
}

body

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

❗️ Оправдано ли использование position: relative здесь?

Comment thread index.css Outdated
z-index: 11;
}

.recipe-description

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

❗️ Повторяющийся селектор

Comment thread index.css Outdated

.recipe-description
{
display: flex;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

❗️ Хм, зачем ты используешь flex, если сам крестик направо перемещаешь с помощью text-align?

Comment thread index.css
.ingredients,
.steps
{
width: 50%;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

❗️ Если ты используешь флекс у родительского элемента, то располагать элементы внутри лучше с помощью него

@Dodo888

Dodo888 commented Nov 30, 2016

Copy link
Copy Markdown

❗️ По условию задачи, чем дальше фотография от активной - тем меньше должен быть ее поворот. Сейчас у всех неактивных изображений поворот одинаковый
image

@Dodo888

Dodo888 commented Nov 30, 2016

Copy link
Copy Markdown

❗️ Рецепт должен открываться при нажатии на активное изображение, а не только на надпись "РЕЦЕПТ" внутри него

@Dodo888

Dodo888 commented Nov 30, 2016

Copy link
Copy Markdown

❗️ Отсутствует анимация закрытия карточки

@Dodo888

Dodo888 commented Nov 30, 2016

Copy link
Copy Markdown

❗️ Одна карточка превращается в 3. Одна осталась на месте, остальные 2 (с изображением и рецептом) начали переворачиваться, но видно, что они разные по размерам. Необходимо, чтобы визуально карточка всегда была одна
image

@Dodo888

Dodo888 commented Nov 30, 2016

Copy link
Copy Markdown

❗️ Давай карточку рецепта тоже красиво сверстаем, сейчас она выглядит нерепрезентативно. Не хватает отступов, красивого крестика, каких-нибудь разделителей, шрифтов (например, как в макете из условия)

@Dodo888

Dodo888 commented Nov 30, 2016

Copy link
Copy Markdown

В целом здорово, но есть недочеты

@Dodo888

Dodo888 commented Nov 30, 2016

Copy link
Copy Markdown

🍅

@honest-hrundel

Copy link
Copy Markdown

🍏 Пройден линтинг и базовые тесты

@honest-hrundel

Copy link
Copy Markdown

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel

Copy link
Copy Markdown

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel

Copy link
Copy Markdown

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel

Copy link
Copy Markdown

🍏 Пройден линтинг и базовые тесты

@honest-hrundel

Copy link
Copy Markdown

🍏 Пройден линтинг и базовые тесты

@honest-hrundel

Copy link
Copy Markdown

🍏 Пройден линтинг и базовые тесты

@honest-hrundel

Copy link
Copy Markdown

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel

Copy link
Copy Markdown

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel

Copy link
Copy Markdown

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel

Copy link
Copy Markdown

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel

Copy link
Copy Markdown

🍏 Пройден линтинг и базовые тесты

@OnaHochetNaMars

Copy link
Copy Markdown
Author

🍏

Comment thread index.html
<input class="show-recipe" type="checkbox" id="show-recipe-5">
<input class="show-recipe" type="checkbox" id="show-recipe-6">
<input class="show-recipe" type="checkbox" id="show-recipe-7">
<div class="card card-1">

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

❗️ Можно все же использовать семантичные теги main, article, section

Comment thread index.html Outdated
<input type="checkbox" id="show-recipe-3">
<div class="content">
<div class="img">
<label for="card-3">

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

И все-таки?

@Dodo888

Dodo888 commented Dec 3, 2016

Copy link
Copy Markdown

❗️ Иногда при наведении появляется затемнение, но не появляется кнопка "Рецепт"
image

@Dodo888

Dodo888 commented Dec 3, 2016

Copy link
Copy Markdown

❗️ При анимации закрытия карточка мгновенно становится маленькой, потом переворачивается. Уменьшение тоже должно происходить плавно

@Dodo888

Dodo888 commented Dec 3, 2016

Copy link
Copy Markdown

❗️ При открытой карточке должно появляться затемнение, задний фон должен быть неактивным.
image

@Dodo888

Dodo888 commented Dec 3, 2016

Copy link
Copy Markdown

❗️ В Firefox карточка не открывается
image

@Dodo888

Dodo888 commented Dec 3, 2016

Copy link
Copy Markdown

❗️ В Edge при открытой карточке появляется вертикальный скролл
image

@Dodo888

Dodo888 commented Dec 3, 2016

Copy link
Copy Markdown

В Edge проблема со шрифтом (в Chrome все хорошо)
image

@Dodo888

Dodo888 commented Dec 3, 2016

Copy link
Copy Markdown

Карточка теперь выглядит очень классно) Оставил замечания

@Dodo888

Dodo888 commented Dec 3, 2016

Copy link
Copy Markdown

🚀

@honest-hrundel honest-hrundel assigned msmirnov and unassigned Dodo888 Dec 3, 2016
@msmirnov

msmirnov commented Dec 5, 2016

Copy link
Copy Markdown

Когда карточка закрывается, то происходят какие-то рывки и анимация совсем не похожа на анимацию открытия.

@msmirnov

msmirnov commented Dec 5, 2016

Copy link
Copy Markdown

При переключении центральная карточка зачем-то получает hover, хотя я мышку на неё не наводил. Вводит в заблуждение.
screencast 2016-12-05 23-23-23

@msmirnov

msmirnov commented Dec 5, 2016

Copy link
Copy Markdown

🍅

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.

4 participants