Skip to content

napisanie artykułu#206

Open
Amanitus wants to merge 5 commits intodevmentor-pl:masterfrom
Amanitus:master
Open

napisanie artykułu#206
Amanitus wants to merge 5 commits intodevmentor-pl:masterfrom
Amanitus:master

Conversation

@Amanitus
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown
Owner

@devmentor-pl devmentor-pl left a comment

Choose a reason for hiding this comment

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

Piotrze,

Ogólnie wygląda to dobrze! 👍
Zostawiłem parę drobnych uwag do wdrożenia :)

Comment thread 01/index.html
Comment on lines +15 to +16
<li> <a href="#">rodzaje kursów</a> </li>
<ul>
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Zwróć uwagę, że </li> ma być przed </ul>. Ten element zawiera menu podrzędne tj.

<li> 
    <a href="#">rodzaje kursów</a>
    <ul>
    ...
    </ul>
</li>

Comment thread 02/styles/index.css
(0, 1, 1)
*/
?? {
.ac-container label {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

👍

Comment thread 02/styles/index.css
(0, 2, 1)
*/
?? {
.ac-container label:hover {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

👍

Comment thread 02/styles/index.css
*/
/*??,??*/
.ac-container input:checked + label,
.ac-container input:checked + label:hover {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

👍

Comment thread 02/styles/index.css
(0, 3, 3)
*/
?? {
/*??*/ .ac-container input:checked + label:hover::after {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

👍

Comment thread 04/assets/index.html
</ul>
</li>
<li><a href="#">Item 2 ></a> <!-- Strzałka w prawo -->
<ul class="submenu">
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Zwróć uwagę, że to menu pokazuje się na tej samej linii co to wyżej - tak nie powinno być.
Powinno wyświetlać się na tym samym poziomie co "item 2" i iść w dół.
Wystarczy dla elementu li przypisać position: relative i po problemie ;)

Comment thread 04/style/style.css
margin-right: 20px;
}

.menu li a {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Zdecydowanie łatwiej jest przypisać każdemu elementowi klasę i ją stylować, zamiast używać nazw znaczników.
Czyli zamiast pisać .menu li a lepiej będzie napisać .link

Comment thread 05/index.html
<div></div>
</footer>
</body>
</html> No newline at end of file
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

👍

Comment thread 05/style/style.css
Comment on lines +18 to +29
.logo {
width: 100px;
height: 50px;
position: absolute;
left: 0;
}
.menu {
width: 200px;
height: 50px;
position: absolute;
right: 0;
}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Wykorzystywanie "absolute" do ustawiania elementów nie jest dobrym pomysłem bo ustawiamy ich pozycję na sztywno i nie wpływa to na "rodzica". To rozwiązanie wybieramy w ostateczności - w skrajnych przypadkach. Tutaj zdecydowanie lepiej wybrać "flex-a", aby ustawić elementy na obu bokach - wystarczy ustawić space-between dla space-between

Comment thread 05/style/style.css
right: 0;
}

.dwie-kolumny {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Raczej staramy się używać określeń angielskich w nazwach klas

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