Skip to content

Comments

Moje wypociny#13

Open
EndiZja wants to merge 3 commits intoD0man:developfrom
EndiZja:master
Open

Moje wypociny#13
EndiZja wants to merge 3 commits intoD0man:developfrom
EndiZja:master

Conversation

@EndiZja
Copy link

@EndiZja EndiZja commented Jan 20, 2019

bądźcie łaskawi... ;)

<meta name="viewport" content="width=device-width, initial-scale=1.0">
<meta http-equiv="X-UA-Compatible" content="ie=edge">
<link url="../Academy/Lato/" rel="stylesheet">
<link href="https://fonts.googleapis.com/css?family=Lato:100,100i,300,300i,400,400i,700,700i,900,900i" rel="stylesheet">

Choose a reason for hiding this comment

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

W zupełności wystarczy link z Google Fonts. I nie potrzebnie ładujesz wszystkie style tego fonta skoro ich nie wykorzystujesz.

Copy link

Choose a reason for hiding this comment

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

Nazwa pliku. Narażasz bezpieczeństwo strony choćby faktem że nie masz pliku index i udostępniasz strukturę katalogów.

<div>
<form action="#" method="post" enctype="text/plain">
<p>Leave your E-mail</p>
<label for="email"></label>

Choose a reason for hiding this comment

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

To samo co u mnie. Label nie powinien być pusty, można go ukryć w CSSie

Copy link

Choose a reason for hiding this comment

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

Yyy? Po co Ci był plik php skoro go nie używasz?

Copy link

Choose a reason for hiding this comment

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

Musi tu być ten div?

<meta http-equiv="X-UA-Compatible" content="ie=edge">
<link url="../Academy/Lato/" rel="stylesheet">
<link href="https://fonts.googleapis.com/css?family=Lato:100,100i,300,300i,400,400i,700,700i,900,900i" rel="stylesheet">
<link href="../Academy/style1.css" rel="stylesheet">

Choose a reason for hiding this comment

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

W przypadku Twojej struktury plików, która jest w repozytorium, ten zapis jest błędny, bo po ściągnięciu szuka css w folderze wyżej. Pewnie lokalnie działało, bo wyżej w katalogu też miałaś pliki.

Copy link

Choose a reason for hiding this comment

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

W skrócie wraca do folderu wcześniejszego i... na serwerze jest zwykle public. Dajesz

<link href="style1.css" rel="stylesheet">

I nazwę pliku zmień na taką bez 1 :D


<header class="clearfix">
<img class="header_image" src="../Academy/assets/undraw_developer_activity_bv83.svg" alt="undraw_developer" />
<h1>CODE ACADEMY</h1>

Choose a reason for hiding this comment

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

Zdecydowanie lepiej napisać normalnie Code Academy, a wielkie litery zrobić za pomocą text-transform: uppercase.

Copy link

@mzastue mzastue Jan 22, 2019

Choose a reason for hiding this comment

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

ale wtedy trzeba skorelować dodatkowy styl z danym elementem, po co?

można wtedy też napisać CodE aCADemY, a to jest statyczny plik html, więc nie ma sensu dodawać text-transform; uppercase

Choose a reason for hiding this comment

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

Wydaje mi się, że to nie kwestia rozpatrywania tego jako dodatkowego stylowania, czy rozgraniczania co jest wydajniejsze, a co nie. Ale zastosowanie text-transform jest wygodne i estetyczniejsze, imho. O ile w tym przypadku to nie ma znaczenia, bo jest to małe, jest to statyczne, to chyba chodzi o to żeby się uczyć dobrych praktyk, które przeniesiemy do większych i bardziej zaawansowanych projektów. Nie wydaje mi się żebyś na jakimś dużym projekcie walił nagłówki capslockiem w HTMLu.


<body>
<div class="container">
<img class="coming-soon" src="../Academy/assets/coming-soon-ribbon.png" alt="coming soon" />

Choose a reason for hiding this comment

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

W przypadku zdjęć ta sama sytuacja co z ładowaniem styli. Też ich szuka w folderze wyżej.

</form>
<?php
}
?> No newline at end of file
Copy link

Choose a reason for hiding this comment

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

Zastanawia mnie, czy konieczne jest nazywanie inputa pole. To bez sensu, zwłaszcza zimą... Nazywaj elementy po imieniu, tzn. tak, że widząc stronę i w kod przynajmniej mniej więcej możesz określić co gdzie jest ;) W większych projektach się na 100% odwdzięczy takie podejście

Copy link

Choose a reason for hiding this comment

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

if (count($_POST))

nie mówię że źle, chociaż zazwyczaj ludzie używają isset :D przynajmniej za moich czasów tak było, czyt. 3-4 lata temu

Copy link

Choose a reason for hiding this comment

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

echo mail

tutaj się zgubiłem :D wysyłasz maila czy wyświetlasz wynik?

Copy link

Choose a reason for hiding this comment

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

<button>GET EARLY ACCESS</button>

Get early access ;) W pliku html zachowuj poprawną pisownię, w css użyj uppercase ;)

</form>
<?php
}
?> No newline at end of file
Copy link

Choose a reason for hiding this comment

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

if (count($_POST))

nie mówię że źle, chociaż zazwyczaj ludzie używają isset :D przynajmniej za moich czasów tak było, czyt. 3-4 lata temu

</form>
<?php
}
?> No newline at end of file
Copy link

Choose a reason for hiding this comment

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

echo mail

tutaj się zgubiłem :D wysyłasz maila czy wyświetlasz wynik?

</form>
<?php
}
?> No newline at end of file
Copy link

Choose a reason for hiding this comment

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

<button>GET EARLY ACCESS</button>

Get early access ;) W pliku html zachowuj poprawną pisownię, w css użyj uppercase ;)

<meta http-equiv="X-UA-Compatible" content="ie=edge">
<link url="../Academy/Lato/" rel="stylesheet">
<link href="https://fonts.googleapis.com/css?family=Lato:100,100i,300,300i,400,400i,700,700i,900,900i" rel="stylesheet">
<link href="../Academy/style1.css" rel="stylesheet">
Copy link

Choose a reason for hiding this comment

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

W skrócie wraca do folderu wcześniejszego i... na serwerze jest zwykle public. Dajesz

<link href="style1.css" rel="stylesheet">

I nazwę pliku zmień na taką bez 1 :D

<meta name="viewport" content="width=device-width, initial-scale=1.0">
<meta http-equiv="X-UA-Compatible" content="ie=edge">
<link url="../Academy/Lato/" rel="stylesheet">
<link href="https://fonts.googleapis.com/css?family=Lato:100,100i,300,300i,400,400i,700,700i,900,900i" rel="stylesheet">
Copy link

Choose a reason for hiding this comment

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

Nazwa pliku. Narażasz bezpieczeństwo strony choćby faktem że nie masz pliku index i udostępniasz strukturę katalogów.

right: 0%;
top: 0%;
bottom: 75.03%;

Copy link

Choose a reason for hiding this comment

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

Tutaj również to samo co wcześniej. Chociaż nw czy bez position: absolute tak chętnie Ci się to poustawia

color: #A0A0A0;
}


Copy link

Choose a reason for hiding this comment

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

Ty żywcem kopiowałaś kod z Figmy?

background: url(../Academy/assets/love.png) no-repeat;
}


Copy link

Choose a reason for hiding this comment

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

Wszystkie poprzednie będy się ciągle powtarzają. Nagminnie. Nie może tak być

Copy link
Author

Choose a reason for hiding this comment

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

Przynajmniej widać jakąś powtarzalność i że błędy nie są udawane.

}


.made {
Copy link

Choose a reason for hiding this comment

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

W końcu klasa 👯‍♂️


color: #FFFFFF;
}

Copy link

Choose a reason for hiding this comment

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

W roli podsumowania całego kodu... Myślę że sporo wytknąłem rzeczy podstawowych. W szczegóły nie patrzyłem, żeby nie zniechęcić Cię do dalszej nauki ;)

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.

4 participants