• 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

VPS Starter Arubacloud
0 głosów
146 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 (599,730 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,773 wizyt
0 głosów
3 odpowiedzi 606 wizyt
+1 głos
0 odpowiedzi 177 wizyt
pytanie zadane 10 września 2020 w JavaScript przez creend Gaduła (4,700 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!

...