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

Code Review pierwszej aplikacji w JS

Object Storage Arubacloud
0 głosów
149 wizyt
pytanie zadane 9 kwietnia 2019 w JavaScript przez cylo24 Początkujący (300 p.)

Witam

napisałem pierwszy mini projekt w Html, css + JS czy mógłbym prosić o zrobienie Code review przez jakiegos bardziej doświadczonego programisty?

Z góry bardzo serdecznie dziekuję

kod zamieszczam na GitHubie

Repo na Githubie

Demo strony

1
komentarz 10 kwietnia 2019 przez DeBos123 Nałogowiec (44,950 p.)

W pliku js/main.js powinno być seconds, a nie secends.

Nie zajmuje się webdev'em, więc mogę tylko ocenić czytelność kodu, która jest na dobrym poziomie.

1 odpowiedź

+1 głos
odpowiedź 10 kwietnia 2019 przez ShiroUmizake Nałogowiec (46,300 p.)

Brak przycisku zatrzymania, startować mogę, ale zatrzymać nie mogę.

W pierszym ekranu nie mieści się aplikacja, duuuży minus.

Na mobile możnaby zrobić równo te checkboxy.

Akordeon mógłby się fajnie animować (ten zwijaczek).

Lista ci się nie mieści.

Zółty/czarny to jest słaby kontrast do czytania.

Tekst się zlewa, podbiłbym line-height. Wąziutka kolumna tekstu, cieżko się czyta.

Zdjęcia są mało czytelne, zobacz sobie jak zrobić resposywne i czytelne zdjęcia np w wymiarze 16:9 :). Link!

Duże odstępy między sekcjami. Najlepiej jakby 2 się mieściły w 1 oknie.

Favicona nie jest obsługiwana na wszystkich systemach i przeglądarkach. Zaciekawiony? Link!

Nav powinno być w header? Link!

 <header>
            <h3 class="section-title">Condition of boiled eggs</h3>
        </header>

A gdzie h2?

 

Section jest selektorem grupującym jakąś treść wiadomości np: rozdział. W tym przypadku div jest bardziej odpowiedni. 

<section class="box app-content">
        <header>
            <h3 class="section-title">Condition of boiled eggs</h3>
        </header>
        <div class="option">
            <input type="radio" name="radio" id="opt1" class="cookingDegree" />
            <label for="opt1">Soft</label>
            <input type="radio" name="radio" id="opt2" class="cookingDegree" />
            <label for="opt2">Half-medium</label>
            <input type="radio" name="radio" id="opt3" class="cookingDegree" />
            <label for="opt3">Medium</label>
            <input type="radio" name="radio" id="opt4" class="cookingDegree" />
            <label for="opt4">Half-hard</label>
            <input type="radio" name="radio" id="opt5" class="cookingDegree" />
            <label for="opt5">Hard</label>
        </div>
        <footer class="timer">
            <input type="text" id="values" name="showTime" disabled="disabled">
            <button>Start Cook</button>
        </footer>
    </section>

Kontrolerki zawsze są, opatulone form, inaczej gubi to treść semantyczną. To nie jest stopka semantycznego z punktu widzenia, powinen to być div. To nie jest input, gdyż nie wpisujemy tam żadnej treści. Powinen to być p lub div, flagę disabled nadajemy gdy nie chcemy pewnych danych wysłać w formularzu.

Gdzieś main został zjedzony po drodze.

const radios = [...document.querySelectorAll(".cookingDegree")];

Dlaczego tak? Przecież to Ci zwraca tablicę xp.

const btn = document.querySelector(".timer button");

Protip lepiej przeszukiwać tylko rodzica tych elementów, w tym wypadku form = szybsze działanie bo nie musi całego DOM przeszukać.

const options = [3, 4, 5, 7, 8];

Dlaczego nie wrzucisz w value inputów?
function showTime() {
    return (`0${hours}:${(minutes < 10 ? "0" + minutes : minutes)}:${(secends < 10 ? "0" + secends : secends)}`);
}

hmm.. nie łatwiej parsować milisekund ?:), i przeliczać czas, i najwyzej counterem odejmowac czas?
radios.forEach((radio, i) => {
    radio.addEventListener("click", () => {
        minutes = options[i];
        timeControl.value = showTime();
        timeControl.style.color = "white";
    })
})

Można łatwiej, od rodzica i potem e.target i dodajesz to co tam chciałeś. (Poczytaj o: Event Delegation)
unction time() {
    if (secends > 0) {
        secends--;
    } else if (minutes > 0) {
        minutes--;
        secends = 59;
    } else if (hours > 0) {
        hours--;
        minutes = 59;
        secends = 59;
    }
    timeControl.value = showTime();

    if (hours == 0 && minutes == 0 && secends == 0) {
        timeControl.style.color = "red";
        timeControl.value = `Eggs is done`;
        btn.disabled = "";
        radios.forEach((radio) => radio.disabled = "");
        return;
    }
    setTimeout(time, 1000);
};

Zobacz sobie przykłady parsowania Dat :) + zmiana stylistyki powinno być osobną metoda.

Ej nie ściągasz disabled jak jajka się ugotują.

komentarz 10 kwietnia 2019 przez Comandeer Guru (601,590 p.)

Nav powinno być w header? Link!

Nie ma takiego wymogu.

W tym przypadku div jest bardziej odpowiedni. 

A najbardziej odpowiedni będzie fieldset

komentarz 10 kwietnia 2019 przez ShiroUmizake Nałogowiec (46,300 p.)
W sumie racja! Zapomniałem o tym selektorze.

Podobne pytania

0 głosów
1 odpowiedź 1,804 wizyt
0 głosów
3 odpowiedzi 628 wizyt
+1 głos
0 odpowiedzi 180 wizyt
pytanie zadane 10 września 2020 w JavaScript przez creend Gaduła (4,700 p.)

92,579 zapytań

141,432 odpowiedzi

319,664 komentarzy

61,964 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

Kolejna edycja największej imprezy hakerskiej w Polsce, czyli Mega Sekurak Hacking Party odbędzie się już 20 maja 2024r. Z tej okazji mamy dla Was kod: pasjamshp - jeżeli wpiszecie go w koszyku, to wówczas otrzymacie 40% zniżki na bilet w wersji standard!

Więcej informacji na temat imprezy 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!

...