Skip to content

Comments

Develop#19

Open
kubcio1906 wants to merge 5 commits intoD0man:masterfrom
kubcio1906:develop
Open

Develop#19
kubcio1906 wants to merge 5 commits intoD0man:masterfrom
kubcio1906:develop

Conversation

@kubcio1906
Copy link

test

D0man and others added 5 commits January 13, 2019 22:50
fix typo in fix typo
fix typo in fix typo in fix typo lol
margin: 0 auto;
}

#layout{
Copy link

Choose a reason for hiding this comment

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

Średnio mi się podoba stylowanie po id. Zwykło się widzieć stylowanie po klasach


}

h3{
Copy link

Choose a reason for hiding this comment

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

To samo mogę powiedzieć o stylowaniu po znacznikach :)

font-size: 24px;
line-height: 28px;
}
.sexiflexi{
Copy link

Choose a reason for hiding this comment

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

Za nic nie mogę się domyśleć co to jest... Pisz nazwy klas bardziej mówiące o tym, co jest czym ;)

border-style: solid;
border-color: black;
border-width: thin;

Copy link

@IamMK IamMK Jan 25, 2019

Choose a reason for hiding this comment

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

Nie lepiej użyć

border: thin solid black;

?

top:-35px;
right: 90px;
transition: all 200ms;
opacity: 0.5;
Copy link

Choose a reason for hiding this comment

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

Tutaj to nie jest błąd, lecz podpowiedź. Możesz pisać po prostu .5 ;)

<section id="layout" >
<img src="coming-soon-ribbon.png" id="coming_soon">
<img src="undraw_developer_activity_bv83.svg" id="guy" >
<div id="code" style="color: white;">CODE ACADEMY</div>
Copy link

Choose a reason for hiding this comment

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

  1. Nie stosuj styli inline. To nie jest dobre.
  2. Code Academy. Plik HTML ma być pisany bez najmniejszych oznak "pokemona". Zamiast wielkich liter użyj w css
text-transform: uppercase;

<img src="coming-soon-ribbon.png" id="coming_soon">
<img src="undraw_developer_activity_bv83.svg" id="guy" >
<div id="code" style="color: white;">CODE ACADEMY</div>
<h3>Leave your email</h3>
Copy link

Choose a reason for hiding this comment

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

A H1 i H2 gdzie są?

Choose a reason for hiding this comment

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

tak się z ciekawości spytam, a muszą być? ;> nie można sobie użyć h3 jak nigdzie nie było h1, ani h2? przecież to pełnoprawny element HTML'a

Copy link

Choose a reason for hiding this comment

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

@mateuszlittwin-gf, jeśli masz gdzieś semantykę, to nie muszą ;) Najlepiej jest dać tytuł główny dokumentu w H1, H2 jako tytuły sekcji, H3 jako artykułów itd ;) Choćby ze względu na czytelność kodu i stworzenie swoistego "drzewka" strony.

Choose a reason for hiding this comment

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

to tym bardziej tutaj nie pasuje h1 ani h2, bo to też nie są tytuły sekcji ;) bardziej tu pasuje p

Copy link

Choose a reason for hiding this comment

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

Myślę, że użycie nagłówka tutaj jest do przyjęcia. Choćby ze względu na fakt, że element powinien zwrócić uwagę odwiedzającego. Jednak po drodze zabrakło właśnie header tagów poprzednich ;)

Choose a reason for hiding this comment

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

Teraz tak patrze szerzej i coming-soon-ribbon mozna wywalic gdzies wyzej, no on chyba i tak jest z position absolute (zgaduje).
Ten obrazek undraw_developer_activity pasuje bardziej do jakiegos znacznika header, albo innej sekcji
Pozostałe elementy faktycznie mogą miec h1 i h2, czyli

<h1>Code Academy</h1>
<h2>Leave you email</h2>

Copy link

Choose a reason for hiding this comment

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

O coś takiego mi dokładnie chodzi ;) Gdyby dodatkowo ładnie rozdzielić plik na sekcje header, main i footer byłoby niemal idealnie :)

<div id="get_mail">
<div class="form-group">
<div class="sexiflexi"> <input required type="email" name="email" id="email_field" class="form-control">
<label for="email" class="form-control-placeholder" width="30%">E-mail</label>
Copy link

Choose a reason for hiding this comment

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

Znowu inline. Synu. Nie wolno tak.

<P id="foot">User &copy; 2019<br>
Made with <img src="love.png"> in Poland<br>
We're using cookies to improve your experience.
</P>
Copy link

Choose a reason for hiding this comment

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

Yyy? To jest ble. Bardzo ble. Jeśli chcesz przejść do następnej linii to tworzysz p. Te <br> są tu potrzebne jak sanki w lato...

</P>


</section>
Copy link

@IamMK IamMK Jan 25, 2019

Choose a reason for hiding this comment

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

Co do całości. Zabrakło tu m.in tagów <header> <main> <footer> .

Copy link

Choose a reason for hiding this comment

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

Dodatkowo. Tagi <img> nie mają atrybutu alt.
Dokument nie ma tytułu, tzn <title> w sekcji <head>
I w sumie nie ma zdefiniowanego języka jako atrybut <html>

@kubcio1906
Copy link
Author

Dzięki za wszystkie wskazówki i porady :) jest to mój pierszy projekt w html i css... :D jest nad czym pracować mam nadzieję że z czasem ogarne clean code :D

@kubcio1906
Copy link
Author

kubcio1906 commented Jan 25, 2019 via email

@IamMK
Copy link

IamMK commented Jan 25, 2019

Pierwszy taki projekt :) mam też drugi layout wykonany ale w nim pewnie też dużo do poprawy jest :D czyli mówisz że lepiej nie stylować znaczników typu h3,h2 tylko dawać im klasy ? jak po rodzicu styluje w css to po klasie od razu wystyluje pisząc np: .nagłówek , czy po rodzicu .contener .content .nagłówek . 

Pisząc "styować po rodzicu" miałem tutaj na myśli, że rzeczy dziedziczone przez elementy typu krój czcionki, wielkość, kolor itd. powinny być zdefiniowane w rodzicu jak największej liczby sekcji, które daną właściwość mają ;)
I .contener .content .nagłówek nie jest dobrym nawykiem. Max na co można się pokusić to .content .nagłówek jeśli owa klasa .nagłówek ma wyglądać inaczej np. jako dziecko klasy .content i jako dziecko np. klasy .section

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.

5 participants