Skip to content

Comments

My attempt#17

Open
airfortech wants to merge 16 commits intoD0man:developfrom
airfortech:master
Open

My attempt#17
airfortech wants to merge 16 commits intoD0man:developfrom
airfortech:master

Conversation

@airfortech
Copy link

@airfortech airfortech commented Jan 22, 2019

@@ -0,0 +1,226 @@
@import url('https://fonts.googleapis.com/css?family=Lato:300,400,700&subset=latin-ext');

Choose a reason for hiding this comment

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

Lepiej jest załadować fonty w w HTML.

Copy link

Choose a reason for hiding this comment

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

Ze względu na fakt, że czcionki ile by nie ważyły to dodatkowe dane, które muszą być zaciągnięte zanim cały plik css zostanie wczytany. Lepiej jest zrobić to w html ;)

a{
text-decoration: none;
}
button{

Choose a reason for hiding this comment

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

Staraj się unikać stylowania po tagach.

Copy link
Author

Choose a reason for hiding this comment

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

tutaj akurat reset robilem

Copy link

Choose a reason for hiding this comment

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

nie lepiej reset w oddzielnym pliku?

border: 2px solid red;
}

.contact__input:invalid ~ .contact__input-warning--error{

Choose a reason for hiding this comment

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

Z tą częścią musisz pokombinować. Nie zadziałało tak jak miało. Od wejścia na stronę wyświetla napis invalid e-mail. Ale idziesz w dobrym kierunku. To może Ci się przydać: https://css-tricks.com/float-labels-css/

text-decoration: none;
}
button{
font-family: roboto, sans-serif;

Choose a reason for hiding this comment

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

Ustawiasz fonta, którego nie załadowałeś do projektu. Chciałeś chyba Lato wrzucić :)

Copy link
Author

@airfortech airfortech Jan 22, 2019

Choose a reason for hiding this comment

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

No tak, to copy/paste z resetu mojego jak zaczynam każdy projekt ;) Czcionkę przy okazji zmienia na bardziej przyjazną. Będę musiał pamiętać by to zmianiać. Swoją drogą, inherit zadziała dla buttona przy dziedziczeniu z body?

Copy link
Author

Choose a reason for hiding this comment

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

Sam sobie odpowiem, inherit lepiej się w tym przypadku sprawdzi:)

Copy link

@IamMK IamMK left a comment

Choose a reason for hiding this comment

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

Podsumowując. Nazwy klas są dość ok. Idzie się połapać co jest czym bez patrzenia w plik html. Co do jakiś błędów - może mam tylko takie wrażenie, ale Twój kod wydaje mi się dłuższy niż powinien. Można go śmiało skrócić ;) Mimo to generalnie jest dobrze.

README.md Outdated


Jak ktoś ma ochotę zrobić lepszy readme, to niech to zrobi :D
demo - http://vizman.ayz.pl/01-Landing-Page/
Copy link

Choose a reason for hiding this comment

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

Cały README usunąłeś... Unikaj tego, mało ko to lubi

Copy link

Choose a reason for hiding this comment

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

obraz
Musisz poprawić ;)

Copy link
Author

Choose a reason for hiding this comment

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

Poprawione, jednak dalej chyba jest konflikt? Czyli nic do readme nie mogę dodać, nawet linka do wersji podglądowej?

font-size: 3rem;
line-height: 5rem;
}

Copy link

Choose a reason for hiding this comment

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

nie wiem po co tutaj ta zabawa z minusowymi marginami. Niekoniecznie jest to dobre ;)

Copy link
Author

Choose a reason for hiding this comment

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

Też już nie pamiętam po co:)

@@ -0,0 +1,226 @@
@import url('https://fonts.googleapis.com/css?family=Lato:300,400,700&subset=latin-ext');
Copy link

Choose a reason for hiding this comment

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

Ze względu na fakt, że czcionki ile by nie ważyły to dodatkowe dane, które muszą być zaciągnięte zanim cały plik css zostanie wczytany. Lepiej jest zrobić to w html ;)

a{
text-decoration: none;
}
button{
Copy link

Choose a reason for hiding this comment

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

nie lepiej reset w oddzielnym pliku?

main{
flex-grow: 10;
}

Copy link

Choose a reason for hiding this comment

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

tu też zerowanie?

Copy link
Author

Choose a reason for hiding this comment

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

Tutaj nie.

<h1 class="header__title" >Code Academy</h1>
</header>
<main>
<form id="contact__submit--send" action="" class="contact">
Copy link

Choose a reason for hiding this comment

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

coś tu zmieniasz, że dodajesz modyfikator?
samo contact__submit wystarcza

Copy link
Author

Choose a reason for hiding this comment

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

pod jsa, dwa przyciski są tak samo ostylowane a musiałem je jakoś wyselekcjonować w jsie. Jeden --send odpala dodanie maila do tablicy, a drugi --close zamyka okno z listą adresów.

</div>
<input id="contact__submit--close" type="submit" class="contact__submit" value="ok">
</div>
</aside>
Copy link

Choose a reason for hiding this comment

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

rozumiem wstawka własna :)

e.preventDefault();
tab.push(input.value);
tab.reverse();
console.log(tab);
Copy link

Choose a reason for hiding this comment

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

Jeśli udostępniasz, usuwaj debugi i console.log'i. Mnie to osobiście drażni. Chyba że jest ku temu jakiś powód

Copy link
Author

Choose a reason for hiding this comment

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

ok, będę pamiętał.

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