• Najnowsze pytania
  • Bez odpowiedzi
  • Zadaj pytanie
  • Kategorie
  • Tagi
  • Zdobyte punkty
  • Ekipa ninja
  • IRC
  • FAQ
  • Regulamin
  • Książki warte uwagi

Prośba o code review strony

VPS Starter Arubacloud
+2 głosów
482 wizyt
pytanie zadane 28 sierpnia 2018 w Nasze projekty przez mokebe Nowicjusz (210 p.)

Witam, prosiłbym o sprawdzenie kodu html, css i js wykonanej przeze mnie strony. Jest to moja pierwsza w pełni zakodowana strona psd to html, dlatego zdaję sobie sprawę, że kod zawiera dużo błędów, o których nie mam pojęcia. Z tego powodu proszę o wszelkie wskazówki, co robić, czego nie robić, o czym poczytać. Projekt pochodzi z grupy na facebooku, lecz z tego co zaobserwowałem, grupa chyba umiera i jest tam raczej mało aktywnych osób, dlatego postanowiłem napisać tutaj. Bardzo dziękuję za poświęcony czas.

Live: https://m-o-k-e-b-e.github.io/WWE8/
Kod: https://github.com/m-o-k-e-b-e/WWE8

Prawidłowa kategoria? Czy może powinna być "HTML i CSS"? 

1
komentarz 28 sierpnia 2018 przez User007 Bywalec (2,400 p.)

Sprawdź sobie stronę w walidatorze html: https://validator.w3.org/ Już te walidator wyrzuca parę błędów które popełniłeś.

Jeśli chodzi o wygląd, to mi się podoba. Można by było pewnie coś tam poprawić i dodać jakieś drobne animacje tu i tam ale jest przyjemna dla oka.

 

1
komentarz 28 sierpnia 2018 przez ScriptyChris Mędrzec (190,190 p.)

Jeśli dobrze zauważyłem, to tutaj:

      next = document.querySelector('.right-arrow');
      prev = document.querySelector('.left-arrow');
      images = document.querySelectorAll('.portfolio-img');
      modal = document.querySelector('.modal');
      gallery_img = document.querySelector('.gallery-img');

zadeklarowałeś kilka zmiennych jako globalne. Albo chciałeś użyć przecinków, zamiast średników, albo zapomniałeś dopisać const, tak jak linijkę wyżej.

1
komentarz 29 sierpnia 2018 przez Tomek Sochacki Ekspert (227,510 p.)
Prawdopodobnie miały być przecinki, ale lepiej stosować dla jednej zmiennej jedno const/let.

2 odpowiedzi

+4 głosów
odpowiedź 28 sierpnia 2018 przez Tomek Sochacki Ekspert (227,510 p.)
wybrane 30 sierpnia 2018 przez mokebe
 
Najlepsza
  1. hamburger.addEventListener('click', function(){

    Dlaczego nie arrow function? Nie jest to żaden błąd, ot tak po prostu gdybym dostał taki kod np. jako zadanie rekrutacyjne to dałbym właśnie pytanko dlaczego nie użyłeś arrow function i czy wiesz w ogóle co to itp.?

  2. const all_circles

    W JavaScript raczej przyjęło się stosować zapis camelCase, ale Twoja wersja nie jest żadnym błędem, kwestia uzgodnienia zasad w danym projekcie i zespole.

  3. Wszedzie tam, gdzie w JS przypisujesz style inline, czyli każdy zapis element.style.coś.... warto pomyśleć czy nie zamienić na dodanie/usunięcie klasy CSS. Nawet jeśli jakaś klasa miałaby tylko jedną regułę, to masz wtedy ładne rozdzielenie w JS na jakieś interakcje itp. a cały wygląd w css. W razie potrzeby np. dodania kolejnej reguły to nie zmieniasz już JS dodając kolejne element.style tylko po prostu w jednym miejscu w css uzupełniasz reguły w danej klasie.

  4. modal.style.display = "none";

    Odnośnie pkt. 3, to jest np. dobre miejsce na stworzenie uniwersalnej klasy "hide" i "show", w których odpowiednio ustawisz sobie display.

  5. function checkPosition() {
        if(position < 0) {
            position = images.length - 1;
        } else if(position > images.length -1) {
            position = 0;
        }
    }

    a co z zakresem od 0 do images.length -1 ? Być może w tym przypadku takie wartości nie wystąpią, ale staraj się pisać funkcje bardziej czytelnie i możliwie bardziej unwersalnie. Pomyśl nad wszystkimi przypadkami i ponad to warto wg mnie tutaj zastosować operator:
     

    position = (warunek) ? wartość-dla-true : wartość-dla-false;
    
  6. Dlaczego nagle w smoothScroll zaczynasz korzystać z jQuery? Calego jQuery używasz tylko dla metody animate, może lepiej napisać to samemu skoro tylko dla tej jednej funkcjonalności mielibśmy ładować userowi cała bibliotekę?

  7. for(let i = 1; i <= $('.menu-el').length; i++) {

    Staraj się unikać wewnątrz if, for itp. pobierania referencji i operowania na niej. Są różne szkoły, ja raczej bylem uczony i obecnie też się tego trzymam, że czytelniej jest najpierw pobrac referencję do elementów o klasie .menu_el i potem w for sprawdzić sobie ich ilość. Drugi plus poza czytelnością to fakt, że za chwilę możesz potrzebować znowu te elementy i już odwołasz się do zapisanej pseudotablicy, bez ponownego pobierania referencji.

  8. $('.arrow').click(function() {
        smoothScroll('.charts');
    })

    A dlaczego znowu jQuery, a nie czysty JS jak wcześniej? Trzymaj raczej konsekwencję w swoim kodzie.

  9. const header = $('.page-header');
    if(pageYOffset > header.height()) {
       header.addClass('scrolled');
    } else {
       header.removeClass('scrolled');
    }

    Poczytaj o drugim argumencie dla metody toggle dla classList:
     

    const header = document.querySelector('.page-header');
    
    // Pobieramy w czystym JS wartość obliczonego height:
    const headerHeight = +window.getComputedStyle(header).getPropertyValue("height");
    
    // Ustawiamy klasę w zależności od warunku:
    header.classList.toggle('scrolled', pageYOffset > headerHeight);
    
  10. A w socials media używasz już classList :) wybieraj zawsze jedną metodę i staraj się jej trzymać w całym projekcie, znacznie to ułatwia czytanie i utrzymanie kodu.

Co do html i css się nie wypowiem bo nie jestem w tym specem. Wygląd to jak napisałeś wzorowany na psd więc nie oceniam bo to nie Twój layout :)

 

Tak generalnie to jeśli to Twoje początki to jest wg mnie na prawdę bardzo dobrze. Pisz kolejne stronki, popracuj nad jakimiś bardziej złozonymi logicznie stronami, np. jakieś formularze z walidacją, jakieś generowanie dynamicznie tabel, wykresów itp. Jesteś na dobrej drodze, tak trzymaj i szczerze życzę powodzenia i co najważniejsze w tej branży - wytrwałości i cierpliwości do kodu :)

komentarz 28 sierpnia 2018 przez niezalogowany
Super CR! Nie wiedziałem, że toggle przyjmuje drugi argument : )
1
komentarz 28 sierpnia 2018 przez ScriptyChris Mędrzec (190,190 p.)

Też nie wiedziałem o drugim parametrze dla classList.toggle. Przydatne. :)

Co do code review, to radziłbym unikać dodawania listenerów w pętli (jak tutaj), a zamiast tego stosować event delegation, gdy jest taka możliwość.

komentarz 29 sierpnia 2018 przez mokebe Nowicjusz (210 p.)
Bardzo dziękuję! Naprawdę doceniam poświęcony czas. Na pewno wszystko sobie przeanalizuję i poprawię.
–12 głosów
odpowiedź 28 sierpnia 2018 przez voltex Obywatel (1,210 p.)
W sumie nie ma nic do sprawdzania za bardzo. Ot stronka do napisania w 3 wieczory jak się jest początkującym. Trochę htmla, kilka funkcji w js i ostylowanie tego.
1
komentarz 28 sierpnia 2018 przez Kamil Łydka Stary wyjadacz (13,600 p.)
O patrzcie, jaki mądry.
komentarz 28 sierpnia 2018 przez mokebe Nowicjusz (210 p.)
Zupełnie nie o to w tym wątku mi chodziło, ja nie proszę o ocenę samej strony, tylko o przejrzenie kodu i napisanie co jest z nim nie tak, ponieważ nie chcę tych samych błędów później powtarzać.
1
komentarz 28 sierpnia 2018 przez Tomek Sochacki Ekspert (227,510 p.)

@voltex, To daj Kolego swój kod z początków nauki, zobaczymy czy jest taki super i nie ma żadnych błędów... Rozumiem, skoro tak jedziesz po autorze posta, że gdy Ty zaczynałeś uczyć się JS to już pierwszego dnia ogarniałeś całą asynchroniczność i wszystkie aspekty JS?

PS. Jeden minus odemnie - nie chcesz robić cr to się nie wypowiadaj.

komentarz 28 sierpnia 2018 przez voltex Obywatel (1,210 p.)
edycja 28 sierpnia 2018 przez voltex

@Tomek Sochacki 

Noo, ale jade po autorze.. No zniszczyłem go xD Po prostu projekt jest mega mało ambitny i nigdzie nie napisałem, że jest zły kod czy coś. Jak zobaczyłem stronę to odechciało mi się nawet patrzeć w kod... I nie dlatego, że jest źle napisana, ale dlatego, że widziałem już takich stron miliony.

Tak przeglądam to forum i widzę ciągle powtarzające się "projekty" typu strona wizytowa/portfolio. Czy nikt z początkujących nie ma ambicji napisania czegoś choć odrobinę bardziej oryginalnego? 

EDIT: Zawsze mnie śmieszą komentarze typu "napisz lepiej", "pochwal się swoim kodem", "na pewno pisałeś gorszy kod". Takie komentarze wywołują u mnie tylko gromki śmiech.

PS. Daj mi jeszcze bana :)

1
komentarz 29 sierpnia 2018 przez Hiskiel Pasjonat (22,830 p.)

@voltex, napisałeś, że Cię śmieszą, ale nie napisałeś dlaczego. Boisz się pokazać kodu? Jeśli jesteś poważny, to przynajmniej uargumentujesz swoje zdanie.

Co do pisania oryginalnych rzeczy - ludzie często piszą wizytówki I portofolia, ponieważ strony tego typu są częstymi zgłoszeniami (u freelancerów najczęściej), oraz - taki WebMaster musi mieć jakąś swoją wizytówkę, a w ten sposób sprawdza, co podoba się ludziom, oraz jednocześnie się uczy, jakich błędów nie popełniać na przyszłość. Zobacz tylko portofolia jakichś WebMasterów. Ich strony to najczęściej strony reklamowe jakichś sklepów, wizytówki fotografów, etc.

 

A na forum chyba jesteś od paru dni. Tyle tu się przewijało projektów gierek czy oryginalnych serwisów, że nie sposób spamiętać.

 

Tak swoją drogą w oryginalności nie liczy się kategoria, a wykonanie. Jeśli artysta namaluje obraz, to powiesz mu, że nieambitne, ponieważ już miliony ludzi malowało obrazy i że ma wymyślić nowy rodzaj sztuki?

 

komentarz 29 sierpnia 2018 przez voltex Obywatel (1,210 p.)
Śmieszą mnie dlatego, że to takie podejście jak z gimnazjum... Nie muszę nic nikomu udowadniać. Jak ktoś daje swój kod publicznie to mam równe prawo go skrytykować jak ty pochwalić.

No jak ktoś ma ambicje pisać przez całe życie strony wizytowe to nawet w tych czasach nie trzeba być programistą. HTML, jakiś framework do CSS i dosłownie odrobinę JS, żeby coś ukryc, pokazać. A i to nawet nie bo teraz jest tyle gotowców, że wystarczy skopiowac i dać własne dane. Ale masz rację to są najczęsciej targety dla freelancerów, ale czy trzeba się chwalić na forum takimi projektami? Czy jest tu jakaś zawiła logika?
komentarz 29 sierpnia 2018 przez mokebe Nowicjusz (210 p.)
Naprawdę nie miałem zamiaru chwalić się taką stroną, prosiłem tylko o przejrzenie kodu. Ja doskonale zdaję sobie sprawę z tego, że to jest bardzo prosty projekt i nie mogę siebie nawet nazwać początkującym programistą ;) Na pewno chcę się dalej uczyć i będę robił coraz bardziej wymagające rzeczy. Tak jak pisałem - ten wątek utworzyłem po to, aby w przyszłości uniknąć takich samych błędów, jakie popełniłem tym razem, a nie byłem ich świadomy.

Podobne pytania

0 głosów
3 odpowiedzi 290 wizyt
pytanie zadane 3 stycznia 2018 w JavaScript przez AAwersja Nowicjusz (140 p.)
0 głosów
2 odpowiedzi 228 wizyt
pytanie zadane 10 stycznia 2019 w HTML i CSS przez smokolisz Mądrala (6,340 p.)
0 głosów
1 odpowiedź 95 wizyt
pytanie zadane 13 stycznia w SQL, bazy danych przez whiteman808 Obywatel (1,780 p.)

92,453 zapytań

141,262 odpowiedzi

319,088 komentarzy

61,854 pasjonatów

Motyw:

Akcja Pajacyk

Pajacyk od wielu lat dożywia dzieci. Pomóż klikając w zielony brzuszek na stronie. Dziękujemy! ♡

Oto polecana książka warta uwagi.
Pełną listę książek znajdziesz tutaj.

Akademia Sekuraka

Akademia Sekuraka 2024 zapewnia dostęp do minimum 15 szkoleń online z bezpieczeństwa IT oraz dostęp także do materiałów z edycji Sekurak Academy z roku 2023!

Przy zakupie możecie skorzystać z kodu: pasja-akademia - użyjcie go w koszyku, a uzyskacie rabat -30% na bilety w wersji "Standard"! Więcej informacji na temat akademii 2024 znajdziecie tutaj. Dziękujemy ekipie Sekuraka za taką fajną zniżkę dla wszystkich Pasjonatów!

Akademia Sekuraka

Niedawno wystartował dodruk tej świetnej, rozchwytywanej książki (około 940 stron). Mamy dla Was kod: pasja (wpiszcie go w koszyku), dzięki któremu otrzymujemy 10% zniżki - dziękujemy zaprzyjaźnionej ekipie Sekuraka za taki bonus dla Pasjonatów! Książka to pierwszy tom z serii o ITsec, który łagodnie wprowadzi w świat bezpieczeństwa IT każdą osobę - warto, polecamy!

...