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 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>

(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.

👍

(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.

👍

*/
/*??,??*/
.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.

👍

(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.

👍

</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 ;)

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

<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 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

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