W tym artykule zbieramy do kupy rzeczy, na które powinniśmy zwracać uwagę przed oddaniem review / w trakcie robienia komuś review.
Umieszczam tutaj kwestie, które pojawiły się już kilka razy, abyśmy mogli im łatwiej zapobiegać na przyszłość.
Póki co lista jest niewielka, ale będzie rosła z czasem.
Każdy z punktów jest nagłówkiem, do którego można bezpośrednio linkować - zachęcam Reviewersów do linkowania do konkretnych sekcji w swoich komentarzach do diffów.
Standardy odnośnie metadanych rewizji są opisane w [osobnym dokumencie](https://hub.sealcode.org/w/sealhub_workflow/standardy/metadane-rewizji/)
# 0. Komunikacja w zespole
## 0.1 Komentarze do review
### 0.1.1 Starannie wybieraj słownictwo i formy gramatyczne
Zadaniem reviewera jest krytykować kod, a nie jego autora. Warto wyrażać to także przy wyborze słów i form gramatycznych w trakcie pisania komentarzy.
Lepiej jest napisać "X jest źle zrobione" niż "źle zrobiłeś X".
### 0.1.2 Daj znać, gdy jakiś fragment kodu jest niezrozumiały
Jeżeli nie rozumiesz jakiegoś fragmentu kodu, napisz o tym w komentarzu - możesz poprosić autora kodu o to, aby (i/lub):
* zrefaktorował kod do bardziej deklaratywnej postaci (patrz zasada 1.3);
* zmienił nazwy zmiennych i funkcji aby skuteczniej wyrazić swoją intencję;
* opatrzył kod komentarzem;
* dodał lub poszerzył dokumentację kodu;
* odpowiedział na pytania recenzenta w komentarzu do diffa;
# 1. Ogólne
## 1.1 Formatuj kod za pomocą Prettiera
Prettier pozwala na automatyczne formatowanie wielu rodzajów plików - w tym html, js, jsx, css, scss. Jeżeli piszesz kod w języku wspieranym przez prettiera, to zadbaj o to, aby był nim sformatowany.
Gdy tworzysz HTML za pomocą JS, pamiętaj o tym, aby HTML zamykać w ``` (zamiast `'` lub `"`) i poprzedzać każdy string z HTML-em komentarzem `/* HTML */`. Dzięki temu Prettier prawidłowo sformatuje zawarty w stringu HTML:
```
name="przed formatowaniem prettierem", lang=js
a = {
content: /* HTML */ `<p>foo bar</p><p>bar</p><p>baz</p>`
};
```
```
name="po formatowaniu prettierem", lang=js
a = {
content: /* HTML */ `
<p>foo bar</p>
<p>bar</p>
<p>baz</p>
`
};
```
Więcej informacji o Prettierze znajdziesz [tutaj](https://hub.sealcode.org/w/sealhub_workflow/narzedzia/prettier/)
## 1.2 DRY - don't repeat yourself
Co do zasady nie powinniśmy się powtarzać. Jeżeli jakiś fragment kodu się powtarza to warto zastanowić się, czy każdy z powtarzanych segmentów nie realizuje czasem tej samej intencji. Jeżeli tak, to rodzi to problem - jeżeli w przyszłości nasza intencja lub sposób jej realizacji się zmieni, to będziemy musieli pamiętać aby nanieść odpowiednie zmiany w każdym powtarzającym się miejscu. Aby temu zapobiec należy wyciągnąć ten kod do osobnego modułu/mixina/funkcji.
### 1.2.1 Not too DRY
Czasem widzimy dwa bardzo podobne fragmenty kodu i korci nas, aby je wpiąć w jeden moduł/funkcję/selektor. Jednak jeżeli te dwa fragmenty realizują inną intencję, a ich podobieństwo jest zwykłym zbiegiem okoliczności, to lepiej nie łączyć ich w jedną jednostkę - bo gdy intencja któregoś z nich się zmieni, trzeba będzie dokonywać ponownego rozdzielenia.
## 1.3 Wyrażaj swoją intencję wprost
To jest bardzo miękka zasada, ale zachęcam aby myśleć o niej często przy pisaniu kodu. Zilustruję ją prostym przykładem:
```
lang=html, name=complicated.html
<style>
.wrapper:before,
.wrapper:after {
display: inline-block;
content: " ";
width: 1rem;
}
</style>
<div class="wrapper"><span class="content">Ala ma kota</span></div>
```
```
lang=html, name=direct.html
<style>
.content {
margin: 1rem;
}
</style>
<div class="wrapper"><span class="content">Ala ma kota</span></div>
```
Obydwa powyższe przykłady dają taki sam efekt wizualny: napis "ala ma kota" ma oddech z prawej i lewej strony. Ale w `direct.html` kod znacznie klarowniej wyraża naszą intencję: właściwie wprost mówi nam, co chcieliśmy osiągnąć.
Oczywiście czasem technologia, z której korzystamy nie pozwala nam ująć naszych intencji w bezpośredni sposób - ale jeżeli tylko mamy taką możliwość, powinniśmy zawsze sięgać po bardziej czytelne rozwiązanie.
### 1.3.1 Deklaratywny kod pomaga jaśniej wyrażać swoją intencję
Kiedy wyrażamy swoją intencję w kodzie, często pierwszym narzędziem, po jakie sięgamy są pętle, wyrażenia if/else i inne podstawowe składowe języka. Tak bardzo skupiamy się na tym, *jak kod ma działać*, że osoba czytająca go może mieć problem ze zrozumieniem, *co ten kod robi*. Warto wtedy dodać warstwę abstrakcji, która uczytelni kod i ułatwi jego zmianę w przyszłości.
Często można uczynić kod bardziej deklaratywnym zmieniając logikę na dane. Przykład kodu niedeklaratywnego:
```
name=niedeklaratywny, lang=html
<ul>
<li><span class="name">Luka</span>, pies</li>
<li><span class="name">Piki</span>, kot</li>
<li><span class="name">Cytrus</span>, kot</li>
</ul>
```
```
name=deklaratywny, lang=js
const animals = [
{ name: "Luka", type: "pies" },
{ name: "Piki", type: "kot" },
{ name: "Cytrus", type: "kot" }
]/* HTML */ `
<ul>
${animals.map(
({ name, type }) => /* HTML */ `
<li><span class="name">${name}</span>, ${type}</li>
`
)}
</ul>
`;
```
Charakterystyczną cechą niedeklaratywnego kodu jest obecność wyrażeń if/else. Na szczęście bardzo często można przepisać kod tak, aby nie korzystał if/else i w większości wypadków kod zyska wtedy na czytelności:
```
name="z if/else", lang=js
if (animal.type === "dog") {
console.log("this is a dog");
console.log("it has four legs");
console.log("demands belly rubs");
} else if (animal.type === "cat") {
console.log("this is a cat");
console.log("it has four legs and each of them is plotting against you");
} else {
console.log("unknown animal!");
}
```
```
name="bez if/else", lang=js
animal_handlers = {
dog: () => {
console.log("this is a dog");
console.log("it has four legs");
console.log("demands belly rubs");
},
cat: () => {
console.log("this is a cat");
console.log("it has four legs and each of them is plotting against you");
},
unknown: () => {
console.log("unknown animal!");
}
};
(animal_handlers[animal.type] || animal_handlers.unknown)();
```
### 1.3.2 Unikaj if-ów z negacją
Dodanie negacji do dowolnego zdania sprawia, że jego zrozumienie wymaga więcej wysiłku. "Skłamałbym gdybym powiedział, że nie wiem co to zdanie znaczy" jest trudniejsze do sparsowania, niż "Wiem co to zdanie znaczy".
Podobnie
```
lang=js
if( i_can_pet_seal() ) {
pet_seal();
} else {
be_disappointed();
}
```
Jest bardziej klarowne niż
```
lang=js
if( !i_can_pet_seal() ){
be_disappointed();
} else {
pet_seal();
}
```
### 1.3.3 Unikaj nadmiarowego zagnieżdżania ścieżki pozytywnej w `else`
Czasem sprawdzamy, czy nastąpił błąd za pomocą if/else. W przypadku braku błędu wykonuje się kod z else:
```
name="nadmiarowe-zagnieżdżenie", lang=js
if(response.error){
alert("Wystąpił błąd!");
} else {
document.write(response.data[0].title);
// dalszy kod ścieżki pozytywnej
}
```
Powoduje to nadmiarowe zagnieżdżenie całej ścieżki pozytywnej, któremu można zapobiec dodając strategicznie `return`:
```
name="poprawnie", lang=js
if(response.error){
alert("Wystąpił błąd");
return;
}
document.write(response.data[0].title);
// dalszy kod ścieżki pozytywnej
```
# 2. HTML
(jeżeli zawierasz w HTML pełne zdania, to pamiętaj także o zasadach z sekcji "Proza" (poniżej)
## 2.1 Obrazki
### 2.1.1 Atrybut `alt`
Każdy tag `img` musi zawierać atrybut `alt`.
Jeżeli obrazek zawiera treść, a nie jest tylko ozdobą strony, to należy jako wartość atrybutu `alt` podać słowny opis tego obrazka - kierowany do osób nie widomych. Co widać na obrazku? Jakie treści się w nim znajdują? Jeżeli obrazek jest po prostu ozdobnym tekstem, możemy po prostu przepisać ten tekst do atrybutu `alt`
Jeżeli obrazek jest tylko ozdobą/ornamentem strony, ustawiamy `alt=""`.
### 2.1.2 Responsywność
Zadbaj o to, aby obrazki były responsywne - zob. https://blog.sealcode.org/post/12/responsywne_obrazki_w_html/
## 2.2 Klikalność
Elementy klikalne powinny:
* mieć duży obszar, który reaguje na klikanie. Małe ikonki powinny reagować na kliknięcie nawet, gdy klikniemy trochę obok samej ikony. Linki stylowane na buttony powinny reagować na kliknięcie nie tylko na tekst, ale także na całe tło buttona (albo nawet na obszar dookoła);
* zmieniać kursor na "pointer" po najechaniu na nie;
* po najechaniu kursorem zmieniać swój kolor, rozmiar, lub w jakikolwiek inny sposób zapraszać do kliknięcia.
## 2.2 Nazwy klas (BEM)
Nazywamy klasy zgodnie z nomenklaturą [BEM](http://getbem.com/)
### 2.2.1 Prawidłowe stosowanie podziału na Block, Element i Modifier
Tworzymy nowy Block, gdy napotykamy komponent który jest semantycznie samodzielny.
Element jest zawsze powiązany semantycznie z Blockiem - np. "element **paginacji**", "opcja w **menu**"
Modifier jest dodawany, aby zmienić stan/wyróżnienie danego elementu.
### 2.2.2 Unikanie nadmiarowego zagnieżdżania
Każdy Block jest samodzielny, nawet jeżeli akurat znajduje się wewnątrz innego Blocku.
Rozpatrzmy to na przykładzie paska nawigacji, na którym widnieje zdjęcie profilowe aktualnie zalogowanego użytkownika.
Zdjęcie użytkownika nie jest semantycznie powiązane z paskiem nawigacji, więc jest Blokiem - a nie Elementem wewnątrz navbara. W tym wypadku lepiej utworzyć osobne bloki "navbar" i "profile-picture", zamiast tworzyć nadmiarowo zagnieżdżoną klasę "navbar__profile-picture".
### 2.2.3 Dodawanie wszystkich potrzebnych klas
Jeżeli stosujemy modifier do jakiegoś elementu, w jego tagu `class` powinny znaleźć się zarówno klasa bazowa, jak i ta opatrzona modyfikatorem:
```
lang=html
<div class="navbar__item"></div>
<div class="navbar__item navbar__item--selected"></div>
```
## 2.3 Layout
### 2.3.1 Tekst ma jak oddychać
Tekst prezentuje się najlepiej, gdy ma oddech - czyli nie przykleja się do krawędzi ekranu/swojego kontenera.
Warto zwrócić na to uwagę w szczególności przy projektach, które powinny być responsywne. Sprawdź, czy dla którejś szerokości ekranu nie dzieje się coś takiego:
{F45497}
Czasem dodanie zwykłego paddingu lub marginesu w odpowiednim miejscu rozwiązuje problem. Pamiętaj przy tym o [wyrażaniu swoich intencji wprost](https://hub.sealcode.org/w/sealhub_workflow/standardy/#1-3-wyra-aj-swoj-intencj).
### 2.3.2 Rytm pionowy
Jeżeli wytyczne projektu na to pozwalają, interfejs powinien utrzymywać [rytm pionowy](https://hub.sealcode.org/w/sealhub_workflow/standardy_wizualne/vertical-rhythm-tutorial/)
### 2.3.3 Ergonomia czytania
Krótkie teksty nie mogą być za wąskie ani za szerokie - [powinny trzymać szerokość około 70 znaków](http://webtypography.net/2.1.2).
Dłuższe teksty muszą spełniać następujące wytyczne:
* wąskie linie - bliżej 36 znaków na linię niż 66,
* font - slab lub sans, o dużym świetle,
* tekst wyrównany do lewej,
* krótkie paragrafy,
* częste i proste nagłówki.
### 2.3.4 Prawidłowy kontrast
Kontrast pomiędzy tłem a kolorem tekstu powinien być odpowiedniu duży, aby ułatwić czytanie tekstu.
Do badania kontrastu mogą posłużyć następujące narzędzia:
* https://webaim.org/resources/contrastchecker/
* https://contrast-ratio.com/
* https://addons.mozilla.org/en-US/firefox/addon/wcag-contrast-checker/
## 2.4 Obfuskacja
Aby utrudnić życie spam-botom, warto poddać obfuskacji adresy email umieszczone w publicznie dostępnych dokumentach html. Jedną ze skuteczniejszych metod obfuskacji jest [zamiana każdej litery adresu na jej html-owy odpowiednik](https://stackoverflow.com/a/748804/1467284)
## 2.5 Semantyka
Jest wiele rodzajów elementów w HTML - `a`, `div`, `p`, `span`, `article`, `link`, etc. O ile często wybranie dla danego elementu tagu X zamiast Y nie powoduje zmian w wizualnym wyglądzie strony, o tyle może wpłynąć na jej czytelność dla wyszukiwarek internetowych oraz dla osób korzystających ze screen readerów (np. osób niewidomych).
Dlatego warto zapoznać się z przeznaczeniem każdego z rodzajów elementów i odpowiednio używać każdego z nich.
Paragrafy tekstu zawijamy w osobne `<p>` zamiast oddzielać je za pomocą `<br>`. Tekst wewnątrz buttona lepiej aby był `span`-em zamiast `div`-em. Główny kontener artykułu na blogu powinien być `article`-em zamiast zwykłym `div`-em.
W szczególności trzeba też zwrócić uwagę na przyciski. Niewskazane jest używanie ostylowanych div-ów jako przycisków, gdyż tracimy wtedy dużo ułatwień (np. div-ów domyślnie nie można focusować za pomocą klawisza `tab`). Jeżeli to są przyciski nawigacyjne, niech będą linkami (`<a>`). Przyciski potwierdzające wysłanie formularza powinny być `<input type="submit">. Przyciski wywołujące jakąś inną akcję, niech będą... przyciskami (`<button>`).
Na stronie powinien z reguły znajdować się tylko jeden nagłówek `h1`. Nagłówki niższego stopnia (`h2`-`h6`) powinny logicznie odzwierciedlać drzewiastą strukturę artykułu czy innego contentu, jaki prezentuje strona.
Polecane źródła do wzbogacenia swojej intuicji dot. semantyki HTML:
* https://developer.mozilla.org/en-US/docs/Web/HTML/Element
* https://htmlreference.io/
## 2.6 Fonty
### 2.6.1 Generyczne często wystarczają
Dodawanie customowych fontów do strony zwiększa jej wagę i wydłuża czas wczytywania. Jeżeli to możliwe, korzystaj z tzw. "generycznych" fontów:
* `sans-serif`,
* `serif`,
* `monospace`,
* `cursive`,
* `fantasy`,
* `system-ui`.
Wybranie generycznych fontów powoduje, że przeglądarka wybiera któryś z już zainstalowanych w systemie fontów o zadanej cesze i nie pobiera ich z Internetu. Na każdym systemie mogą zostać wybrane inne fonty (zależnie od wersji/rodzaju systemu i preferencji użytkownika).
### 2.6.2 Używaj generycznych fontów jako fallbacku
Jeżeli zdecydujesz się korzystać z customowych fontów, dodaj fallback, który jest możliwie podobnym do zamierzonego fontem generycznym - jeżeli okaże się, że z jakiegoś fontu nie można pobrać, zostanie użyty podobny generyk i strona nie straci znacznie na wyglądzie.
```
lang=css
font-family: InterUI, sans-serif; /* 'sans-serif' jest tutaj fallbackiem */
```
## 2.7 Responsywność
### 2.7.1 Jak najmniej breakpointsów
(Powiązane z [1.3 Wyrażaj swoją intencję wprost](https://hub.sealcode.org/w/sealhub_workflow/standardy/#1-3-wyra-aj-swoj-intencj))
Dzięki rosnącej kompatybilności przeglądarek z `flex-box`-em i `css-grid`-em coraz rzadziej jesteśmy zmuszeni korzystać z breakpointsów, a zamiast tego możemy korzystać z bardziej deklaratywnych metod.
Przykład: załóżmy, że mamy dwa div-y z rożnymi informacjami. Chcemy, aby na dużych ekranach każdy z nich zajmował połowę szerokości ekranu i aby oba wyświetlały się obok siebie, w układzie horyzontalnym:
```
+-------------------+--------------------+
| | |
| div 1 | div 2 |
| | |
+-------------------+--------------------+
```
a na małych ekranach przechodziły w widok kolumnowy:
```
+---------+
| div 1 |
+---------+
| div 2 |
+---------+
```
O ile można to zaimplementować za pomocą breakpointsów:
```
lang=css, name=with-brakpoints.css
.container {
display: flex;
flex-flow: row nowrap;
}
@media (max-width: 500px){
.container {
flex-flow: column nowrap;
}
}
```
o tyle breakpointsy wymagają abyśmy określili sztywno granicę pomiędzy jednym layoutem a drugim i przy bardziej złożonych projektach powodują że wprowadzanie zmian jest uciążliwe. Podejście bardziej deklaratywne pomaga nam uniknąć tego kłopotu:
```
lang=css, name=declarative.scss
.contaier {
display: flex;
flex-flow: row wrap;
& > * { //div1 i div2
flex-basis: 50%; //niech zajmują połowę ekranu...
min-width: 250px; //ale niech nie będą węższe niż 250px. Wtedy zamiast ściskać je flexbox się zawinie i przejdziemy do de-facto widoku kolumnowego
}
}
```
## 2.8 Jednostki
### 2.8.1 Unikaj używania `100vw`.
Używaj `100vw` tylko, jeżeli `100%` nie działa tak, jak oczekujesz. `100vw` robi się kłopotliwe, kiedy dodajemy np. padding dla rodzica danego elementu i nagle strona nie mieści nam się na ekranie w poziomie...
## 2.9 Bezpieczeństwo
### 2.9.1 Linki
W elementach `a` powinniśmy zawsze (z dokładnością co do uzasadnionego wyjątku) ustawiać `rel=noopener`, aby zapobiec podatności na [ten atak](https://mathiasbynens.github.io/rel-noopener/)
# 3. Proza
## 3.1 Unikaj wiszących spójników
Zadbaj, aby tekst był złożony tak, aby nie zawierał [wiszących spójników](https://pl.wikipedia.org/wiki/Sierotka_(typografia)). Możesz to osiągnąć za pomocą twardych spacji (HTML: ` `) lub odpowiedniej konfiguracji programu do składu tekstu.
## 3.2 Używaj prawidłowych znaków typograficznych
### 3.2.1 Apostrof
Znak `'`, o ile powszechnie używany w roli apostrofu, nie jest apostrofem. Prawidłowym znakiem typograficznym reprezentującym apostrof jest znak `RIGHT SINGLE QUOTATION MARK` (HEX: 2019, DEC: 8217; zob. http://unicode.org/Public/UNIDATA/NamesList.txt), który wyglada tak: ’
Uwaga na znacznik `'` w html, gdyż odpowiada on znakowi `'`, a nie apostrofowi. Prawidłowym znacznikiem apostrofu w html jest `’` (można go też zapisać jako ’)
### 3.2.2 Ampersand
Znak `&` ma szczególne znaczenie, bo służy do tworzenia encji jak np. ` `. Chcąc wyświetlić ampersand na stronie należy użyć znacznika `&`
# 4. UI
## 4.1 Asynchroniczne operacje (czekanie)
Nikt nie lubi czekać. Gdy program wykonuje jakąś czasochłonną operację to załóż, w niektórych sytuacjach może ona trwać 10, a nawet 100 razy dłużej. Może to być spowodowane trudnymi warunkami sieciowymi, starszym sprzętem lub ponadprzeciętnie dużą ilością danych do przetworzenia.
Jeżeli to możliwe, wyświetl w interfejsie:
* informację o aktualnym stadium procesu (np. progress bar, ew. spinner)
* informację o przewidywanym czasie zakończenia
W wypadku błędu:
* wyświetl informację o niepowodzeniu. Postaraj się, aby była czytelna i żeby użytkownik mógł stwierdzić, co jest przyczyną błędu
* jeżeli operacja zawiodła z przyczyn niezależnych od użytkownika, automatycznie spróbuj ponowić operację (jeżeli to możliwe i stosowne w danej sytuacji)
## 4.2 Struktura informacji z perspektywy użytkownika
O ile strukturę backendu kierujemy bazą danych, [na froncie kierujemy się potrzebami użytkowników](https://twitter.com/steveschoger/status/997125312411570176)
# 5. Aspekty wizualne
## 5.1 Wyrównanie geometryczne vs optyczne
Czasem wycentrowanie czegoś w poziomie z idealną matematyczną precyzją nie daje wizualnie satysfakcjonujących efektów:
{F64058, layout=inline} {F64060, layout=inline}
Biorąc pod uwagę kształt, środek ciężkości i inne aspekty wizualne centrowanego elementu możemy wtedy pozwolić sobie na zrezygnowanie z idealnego matematycznie centrowania i zamiast tego wybrać ręcznie najprzyjemniejsza dla oka wersję:
{F64062, layout=inline} {F64063, layout=inline}
# 6. Javascript
## 6.1 Używaj async/await lub .then, ale nie mieszaj ich
W ramach jednej funkcji zdecyduj się na używanie albo async/await, albo notacji z `.then`. Async/await jest generalnie preferowany, `.then` może przydać się w krtókich funkcjach.
# 7. CSS
## 7.1. Określaj animowane atrybuty w `transition`
```
lang=css, name="A"
.button {
transition: 200ms;
background-color: green;
}
.button:hover {
background-color: red;
}
```
```
lang=css, name="B"
.button {
transition: background-color 200ms;
background-color: green;
}
.button:hover {
background-color: red;
}
```
Hover na buttonie w przypadku A i B da taki sam efekt, ale w B przeglądarka wie że ma się spodziewać animacji tylko samego koloru tła i będzie w stanie tę animację lepiej zoptymalizować
# 8. Dokumentacja
## 8.1 Używaj placeholder tekst w dokumentacji komponentów
Dokumentację komponentu tworzonego na podstawie layoutu wypełnionego konkretnymi danymi warto wypełnić placeholderowym tekstem (np. "Lorem ipsum") i placeholderowymi obrazkami zamiast tekstem wyciągniętym z layoutu. Pozwoli to zadbać o elastyczność komponentu przy jego tworzeniu i zwiększy jego reużywalność.