Авинас Мария by MaryaAvinas · Pull Request #44 · urfu-2016/markup-task-5

MaryaAvinas · 2016-11-07T17:09:40Z

Посмотреть решение

honest-hrundel · 2016-11-07T17:10:15Z

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

honest-hrundel · 2016-11-07T17:16:09Z

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

honest-hrundel · 2016-11-07T17:17:27Z

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

honest-hrundel · 2016-11-07T17:32:11Z

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

honest-hrundel · 2016-11-07T17:52:19Z

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

VasiliiKuznecov · 2016-11-07T19:58:32Z

насколько понимаю, когда в виде списка, описание кота должно уходить в отдельный столбик

VasiliiKuznecov · 2016-11-07T19:59:42Z

при наведении на картинку нет эффекта

VasiliiKuznecov · 2016-11-07T20:02:01Z

@@ -0,0 +1,92 @@
+.card-to-list:checked ~ main .catPhoto


list-view-checkbox

вместо card-to-list

не стоит называть классы в camelCase, css не регистрочувствителен
лучше разделять дефисом: cat-photo

VasiliiKuznecov · 2016-11-07T20:02:59Z

+    width: 100%;
+}
+
+header


Лучше не использовать в селекторе тег, который может быть испольован не один раз на странице

VasiliiKuznecov · 2016-11-07T20:03:58Z

+    border: 1px solid black;
+}
+
+img


Лучше не использовать в селекторе тег, который может быть испольован не один раз на странице
чтобы не получилось, что если у нас появится картинка, например, в хедере, к ней применился лишний стиль

VasiliiKuznecov · 2016-11-07T20:04:37Z

@@ -0,0 +1,92 @@
+.card-to-list:checked ~ main .catPhoto
+	{


лишний отступ при каждом селекторе

должно быть:

selector { rules }

VasiliiKuznecov · 2016-11-07T20:06:22Z

+    font-size: larger;
+}
+
+.image-for-list


list-icon

VasiliiKuznecov · 2016-11-07T20:06:42Z

+		<input type="checkbox" class="card-to-list">
+		<img alt="list" src="spisok.png" title="Список" class="image-for-list">
+	<main>
+		<div class="card">


VasiliiKuznecov · 2016-11-07T20:06:59Z

+		<span>Магазин добрых котиков</span>
+	</header>
+	<br>
+		<input type="checkbox" class="card-to-list">


лишний отступ

VasiliiKuznecov · 2016-11-07T20:07:30Z

+		Мяндекс.Муррркет<br>
+		<span>Магазин добрых котиков</span>
+	</header>
+	<br>


зачем <br>? Если нужен отступ, лучше делать margin-bottom

VasiliiKuznecov · 2016-11-07T20:07:49Z

+	</header>
+	<br>
+		<input type="checkbox" class="card-to-list">
+		<img alt="list" src="spisok.png" title="Список" class="image-for-list">


VasiliiKuznecov · 2016-11-07T20:08:32Z

+		<img alt="list" src="spisok.png" title="Список" class="image-for-list">
+	<main>
+		<div class="card">
+			<img class="catPhoto" alt="котик №1" src="cats_photos/cat1.jpg">


alt должен описывать содержание картинки, например, "фото полосатого кота"

VasiliiKuznecov · 2016-11-07T20:11:08Z

+					<span>&#9734;</span>
+				</div>
+				<div class="price">
+					<ins class="current">12 мурлей</ins>


VasiliiKuznecov · 2016-11-07T20:13:10Z

+				</div>
+				<div class="price">
+					<ins class="current">11 мурлей</ins>
+					<del class="old">13 мурлей</del>


лучше назвать old-price и current-price
иначе непонятно будет, когда встретим в css класс old
Или кто-то может создать класс old для отображения другого элемента и это повлияет на это место

honest-hrundel · 2016-11-09T10:58:44Z

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

honest-hrundel · 2016-11-09T10:59:46Z

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

honest-hrundel · 2016-11-09T11:04:24Z

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

honest-hrundel · 2016-11-09T11:16:01Z

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

honest-hrundel · 2016-11-09T11:21:20Z

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

honest-hrundel · 2016-11-09T11:27:04Z

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

honest-hrundel · 2016-11-09T11:27:59Z

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

MaryaAvinas · 2016-11-09T11:28:13Z

🍏

VasiliiKuznecov · 2016-11-09T18:02:11Z


-header
-	{
+.name-of-market


market-name, потому что of не несет смысловой нагрузки и лучше записать короче

VasiliiKuznecov · 2016-11-09T18:03:29Z

 .card:hover
-    {
+{
    background-color: lightgray;


Лучше не описывать цвет словом, а с помощью hex записи

VasiliiKuznecov · 2016-11-09T18:04:49Z

насколько понимаю, когда в виде списка, описание кота должно уходить в отдельный столбик

не увидел изменений

VasiliiKuznecov · 2016-11-09T18:04:56Z

🚀

trixartem · 2016-11-11T13:35:39Z

Замечания по верстке:

При наведении на элемент все скачет и прыгает
Нет внутренних отступов для текста(выглядит не очень красиво)
При наведении на карточку фон закрашивается только по верхней границе картинки, что не очень красиво. Сделай, пожалуйста так что бы карточки были все одинаковой высоты.
На чекбоксе картинка уехала вверх(смотрел в Яндекс.Браузере) + картинка не кликабельна
В целом далеко от основного макета
Звездочки не работаю. Было бы здорово добавить эффекты при наведении на звездочки.
"Картинки кликабельны" + "Реализовать эффекты при наведении на имя котика, фото, категорию, и плитку в целом" - часть задания, но я этого не увидел:(

trixartem · 2016-11-14T05:36:21Z

+
+.list-view-checkbox:checked ~ main .information .name
+{
+    width: 100%;


Точно такое же свойство ниже

Для чего ширину менять?

trixartem · 2016-11-14T05:36:44Z

+
+.list-view-checkbox:checked ~ main .card
+{
+    width: 100%;


Здесь тоже не понятно зачем устанавливать ширину 100%

trixartem · 2016-11-14T05:46:41Z

🍅 Жду исправлений

trixartem · 2016-11-14T05:38:17Z

+    margin: 5px 0;
+    display: inline-table;
+    width: 250px;
+    text-overflow: ellipsed;


Не правильное свойство. Оно не будет работать.

trixartem · 2016-11-14T05:39:12Z

+.card
+{
+    margin: 5px 0;
+    display: inline-table;


Почему inline-table?

trixartem · 2016-11-14T05:41:33Z

+
+.card:hover
+{
+    background-color: lightgray;


Лучше использовать hex запись, что бы запись была короче и этот цвет проще менять(делать светлее темнее и т. д.).

trixartem · 2016-11-14T05:41:47Z

+.card:hover
+{
+    background-color: lightgray;
+    border: 1px solid black;


#000 короче

trixartem · 2016-11-14T05:42:41Z

+	<input type="checkbox" class="list-view-checkbox">
+	<img alt="иконка для изображения списка" src="spisok.png" title="Список" class="list-icon">
+	<main>
+		<div class="card">


Где семантика?

trixartem · 2016-11-14T05:44:17Z

+			<img class="cat-photo" alt="фото спящего кота" src="cats_photos/cat4.jpg">
+			<div class="information">
+				<div class="name">
+					<a href="#">Кеша</a>


Лишняя вложенность, кажется что достаточно одной ссылки

MaryaAvinas added 3 commits November 7, 2016 23:06

разметка

9c6171d

Стили

beff64f

Доп.материалы

6bcb6be

Update index.html

f189222

Update index.css

babfa35

deleted extra spaces

c7824a6

remove spaces to Tab

09fb6e3

honest-hrundel assigned VasiliiKuznecov Nov 7, 2016

VasiliiKuznecov reviewed Nov 7, 2016

View reviewed changes

Comment thread index.css Outdated

font-size: larger;

}

.image-for-list

Copy link
Copy Markdown

VasiliiKuznecov Nov 7, 2016

Choose a reason for hiding this comment

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

list-icon

VasiliiKuznecov reviewed Nov 7, 2016

View reviewed changes

MaryaAvinas added 2 commits November 9, 2016 15:57

fix css

9b2ff1d

fix html

3880234

remove spaces

08ddc5e

remove spaces and fix classes of image with cat

9081570

add effect to hover cat's photo

9379acb

fix name of class

9343758

change name of class

1b0a7b1

VasiliiKuznecov reviewed Nov 9, 2016

View reviewed changes

honest-hrundel assigned trixartem and unassigned VasiliiKuznecov Nov 9, 2016

trixartem reviewed Nov 14, 2016

View reviewed changes

Conversation

MaryaAvinas commented Nov 7, 2016 • edited by honest-hrundel Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

honest-hrundel commented Nov 7, 2016

Uh oh!

honest-hrundel commented Nov 7, 2016

Uh oh!

honest-hrundel commented Nov 7, 2016

Uh oh!

honest-hrundel commented Nov 7, 2016

Uh oh!

honest-hrundel commented Nov 7, 2016

Uh oh!

VasiliiKuznecov commented Nov 7, 2016

Uh oh!

VasiliiKuznecov commented Nov 7, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

honest-hrundel commented Nov 9, 2016

Uh oh!

honest-hrundel commented Nov 9, 2016

Uh oh!

honest-hrundel commented Nov 9, 2016

Uh oh!

honest-hrundel commented Nov 9, 2016

Uh oh!

honest-hrundel commented Nov 9, 2016

Uh oh!

honest-hrundel commented Nov 9, 2016

Uh oh!

honest-hrundel commented Nov 9, 2016

Uh oh!

MaryaAvinas commented Nov 9, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VasiliiKuznecov commented Nov 9, 2016

Uh oh!

VasiliiKuznecov commented Nov 9, 2016

Uh oh!

trixartem commented Nov 11, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

trixartem Nov 14, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

MaryaAvinas commented Nov 7, 2016 •

edited by honest-hrundel

Loading

trixartem commented Nov 11, 2016 •

edited

Loading

trixartem Nov 14, 2016 •

edited

Loading