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

Шмонина Ирина #44

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

Conversation

IrinaShmonina
Copy link

@IrinaShmonina IrinaShmonina commented Nov 14, 2016

@honest-hrundel honest-hrundel changed the title IrinaShmonina Шмонина Ирина Nov 14, 2016
@honest-hrundel
Copy link

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

@honest-hrundel
Copy link

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

@dotokoto
Copy link

Общие замечания:

  1. Увеличение картинки не работает в firefox
  2. Нет закрывающего крестика
  3. Курсор в виде лупы - прикольно, но он остается, даже если картинку уже увеличили (
  4. Будет лучше, если картинка не будет исчезать с заднего плана при увеличении

display: block;
}

.page-3:checked ~ .section1,

Choose a reason for hiding this comment

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

У тебя много где повторяется display: none и display: block. Лучше будет их все объединить

}


img[tabindex='0']
Copy link

@dotokoto dotokoto Nov 16, 2016

Choose a reason for hiding this comment

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

Какой-то странный селектор по tabindex. Почему именно по нему? И для чего тут вообще нужен tabindex?

<header>
<h1>
Бабушкин огородик!
</h1>

Choose a reason for hiding this comment

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

Можно в одну строку написать

@dotokoto
Copy link

На маленьком экране картинки налезают друг на друга:
slide show 2016-11-16 20-59-29
При открытой картинке можно кликнуть на другую картинку, и она откроется. Мне кажется, так быть не должно

@dotokoto
Copy link

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

@dotokoto
Copy link

🍅

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.

3 participants