Skip to content

Html-css-basics-tasks#213

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

Html-css-basics-tasks#213
kacpermak1 wants to merge 5 commits intodevmentor-pl:masterfrom
kacpermak1:master

Conversation

@kacpermak1
Copy link
Copy Markdown

No description provided.

Task 01 completed
Task 02 completed
Task 03 completed
Task 04 completed
Task 04 completed
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.

Kacprze,

Oczywiście jest ok 👍
Jednak zostawiłem parę uwag w komentarzach ;)

</li>
</ul>
</nav>
</footer>
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, 2)
*/
?? {
.ac-container input:checked ~ article {
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.

👍

font-family: 'Source Sans Pro';
font-style: normal;
font-weight: normal;
src: url('/03/source-sans-pro/SourceSansPro-Regular.otf') format('opentype');
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 wstawiałbym ścieżki względne niż bezwzględne. W zależności od sposobu uruchamiania takiej strony ścieżka względna tj. ./03/source-sans-pro/SourceSansPro-Regular.otf - będzie częściej prawidłowa niż wzlędna tj. zaczynająca się od ukośnika tj. / - bo to oznacza katalog główny aplikacji, a nie zawsze jest on zgodny z adresem "plików statycznych".

content: "";
display: table;
clear: both;
} 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.

👍

<body>
<header>
<nav class="header-nav">
<ul class="nav-list">
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.

Nie chcesz spróbować używać BEM?
https://devmentor.pl/b/metodologia-bem-w-css-i-sass

list-style: none;
}

.nav-list > li {
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.

Jeśli używałbyś BEM to zawsze stylujemy po klasach

Comment on lines +47 to +48
.nav-list > li > a,
.nav-list > li > span {
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.

Lepiej w takich sytuacjach nadać odpowiednią klasę (oczywiście wyjątkiem jest :after czy :hover) i po niej stylować.
Zagnieżdżanie znaczników to tworzenie dodatkowych zależności, co niepotrzebnie komplikuje kod.

background-color: #555;
}

.nav-list li ul 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.

Ten zapis jest bardzo podatny na błędy i przeoczenia.
Co jeśli będzie kolejne zagnieżdżenie? Dodasz kolejne ul li?
Raczej utworzyłbym odpowiednią klasę i ją dodawał do odpowiednich elementów.

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

👍

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