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

Portfolio - ocena

+10 głosów
442 wizyt
pytanie zadane 15 marca 2018 w Nasze projekty przez gremlin Dyskutant (7,680 p.)

Muszę przyznać, że Już raz wrzucałem tu moje portfolio, lecz od tego czasu poddane zostało wielu zmianom.

Dlatego postanowiłem poprosić Was o ocenę/ feedback/ wrażenia raz jeszcze.

live: https://nilmergomino.github.io/portfolio/

kod: https://github.com/NilmergOmino/portfolio

Pozdrawiam wszystkich serdecznie :)

5 odpowiedzi

+5 głosów
odpowiedź 15 marca 2018 przez Tomek Sochacki Ekspert (228,960 p.)

Wizualnie się nie wypowiem, bo do grafika to mi baaardzo daleko :) ale mogę dać parę drobnych wskazówek odnośnie JS:

  1. proponuję pomyśleć nad zamianą var na const, ewentualnie let jeśli potrzeba,
  2. dużo używasz innerHTML... nie analizowałem dokładnie gdzie i czy jest to faktycznie potrzebne, ale rozważ czy nie lepsze będzie w wielu miejscach textContent (poczytaj sobie o różnicy między nimi, nawet chyba tu na forum kiedyś było o tym),
  3. setTimeout nie wymaga jawnego odniesienia się do obiektu window. Nie jest to oczywiście błąd, ale tak tylko piszę gdyby trafiło Ci się np. takie pytanko na rozmowie,
  4. zawsze po IF i ELSE stosuj klamerki (https://github.com/NilmergOmino/portfolio/blob/master/js/concat.js#L13). Teoretycznie jedna instrukcja nie wymaga klamerek, ale w praktyce jest to uważane za złą praktykę i lepiej je dawać. Pomyśl np. nad eslint do analizy kodu to Ci wyłapie część błędów.
  5. Poczytaj o arrow function (https://github.com/NilmergOmino/portfolio/blob/master/js/concat.js#L38), co jest często stosowane m.in. jako callback function w metodach Array.prototype. Oczywiście Twoje rozwiązanie nie jest błędem, ale jakby ktoś analizował ten kod to może Cię zapytać czy wiesz co to arrow function (jak już będziesz o tym czytał to poczytaj również o wskaźniku this).
  6. komentarze staraj się pisać po angielsku, takie są raczej standardy.
  7. Nie musisz wszędzie robić tego DomContentLoaded, wystarczy tutaj że dasz js przed zamknięcie </body>
  8. dlaczego raz deklarujesz funkcje jako:
    function fn() {};
    
    //a raz jako:
    
    var fn = function() {};

    obie formy są oczywiście poprawne, choć to var bym raczej zamienił na const, ale to w tym momencie jest mniej istotne. Jeśli nie masz konkretnego powodu by tak robić to lepiej zostać przy jednej formie.

  9. Plusik za zminifikowanie kodu JS, niewiele osób robi to w portfoliach.

To co napisałem to nie są jednak jakieś wielkie błędy, raczej pewne wskazówki, które myślę, że warto poprawić, tym bardziej, że JS określasz na poziomie 1.

Wizualnie jest całkiem spoko.

Życzę zatem powodzenia i szybkiego znalezienia fajnej roboty!

komentarz 15 marca 2018 przez gremlin Dyskutant (7,680 p.)
O, dzięki za wnikliwą analizę!

Przyznam szczerze, że większość kodu js napisałem dużo wcześniej (cały smooth scroll jest raczej do przepisania, tylko nie mogę się za to zabrać). Teraz natomiast pisałem animację tego oka i używałem funkcji strzałkowych oraz let i const tylko przy minifikacji mi grunt krzyczał błędami, a że nie chciało mi się ładować webpacka to po prostu zamieniłem na var i zwykłe funkcje (to lenistwo mnie wykończy).

Z tym if/else to mnie zaskoczyłeś. Sądziłem, że to normalna praktyka bo wygląda prościej. Ale wierzę na słowo i postaram się unikać takiego zapisu!

Dzięki raz jeszcze za wskazówki! Są dla mnie zawsze cenne :)
1
komentarz 15 marca 2018 przez Tomek Sochacki Ekspert (228,960 p.)
Poczytaj sobie o eslint i chociaż tak ogólnie przejrzyj sobie dokumentację to pewnie sam wyłapiesz parę dobrych praktyk, do tego ewentualnie poczytaj o airbnb.

Z tym gruntem to problem wydaje mi się, że leży gdzie indziej, bowiem uglify minifikuje kod zgodny z ES5, więc musiałbyś najpierw przerobić go na ES5, na przykład podciągając babela ale nie wiem dokładnie jak to zrobić w grunt bo raczej korzystam z webpacka na codzień, ale wydaje mi się, że tu będzie źródło problemu.

Na szybko znalaezłem coś takiego, może się przyda:

https://github.com/babel/grunt-babel
+2 głosów
odpowiedź 16 marca 2018 przez ShiroUmizake Nałogowiec (46,390 p.)

Graficznie nie powala jest ok.

    <link rel="icon" type="image/png" sizes="16x16" href="img/favicon-16x16.png">

Tym sposobem nie na wszystkich urządzeniach favicon będzie się osadzała. Między np: iOS. Poszukaj na internetcie jak to się prawidłowo robi.

 <header class="header eye-container">
        <h1 class="header__heading"><a href="" class="logo-link eye-focus_on">
            <svg xmlns="http://www.w3.org/2000/svg" width="21.49716mm" height="14.82321mm" viewBox="0 0 76.171041 52.523184">
                <title>Nilmerg Omino</title>
                <g transform="translate(-116.178 -281.61)" stroke="#f2f2f2" stroke-linejoin="bevel">
                    <circle class="logo_circle" cx="139.40106" cy="308.41641" r="21.718279" fill="#04aa2a" stroke-width="2"/>
                    <path d="M130.94142 324.01607l.004-29.8007 17.85715 29.14151v-29.72147" fill="none" stroke-width="2.5"/>
                    <path d="M163.20965 327.93615l.029-21.18083 12.86613 10.7346 11.96614-10.78629.00001 21.2324" fill="none" stroke-width="2.500001"/>
                </g>
            </svg>
        </a></h1>

To nie BEM.

Podziełiłbym tą strukturę jest duża i sporo powtarzalna.

CSS:


@media (max-width: 450px) {
    .knowledge-legend_active{
        top: 0;
        left: 0;
        width: 100%;
    }
    .nav{
        &__list{
            flex-direction: column;
        }
        &__list-item{
            margin-right: 0;
        }
    }
}
@media (max-width: 400px) {
    .article-skills{
        .section-skills{
            min-height: 150px;
            width: 90%;
            &_second{
                margin-top: 20px;
            }
        }
    }
}
@media (max-width: 380px) {
    .article-projects{
        .project{
            &-figcaption{
                flex-direction: row;
                padding-left: 0px;
            }
            &__link{
                margin: 5px 10px 10px 0;
                min-width: 50px;
            }
        }
    }
}

 

Nie jestem fanem AWD, myślę że dało by to ograć na 3 standardowych breakpointach. Tylko pewnie musiałbyś większość skierować na rysowanej przeglądarce.

Nie wykorzystujesz do końca możliwości preprocesorów. Wydzieliłbym komponenciki scss i wszystko ładował do style.css. Zmienne wyrzuciłbym do jakiegoś defaultowego pliku.

Błędy typowo optymalizacyjne.

Ale jedna rzecz mnie zastanawia.. chwalisz się Reactem a nigdzie Reacta nie ma. Ani wordpressa. Jeżeli już czymś się chwalisz to wypadałoby to potwierdzić, nawet na poziomie befginer.

Podobało mi się bardzo mocno... oczko jeżeli to ty robiłeś (nie użyłeś gotowca) to bije pokłony :).

Co do reszty, bardzo proste projekty ale są jak najbardziej na ok. Czekam co z Reacta wydłubiesz :).

 

 

 

 

komentarz 16 marca 2018 przez gremlin Dyskutant (7,680 p.)
Dzięki za wskazówki i opinię :)

Z reactem dopiero zaczynam naukę dlatego jest najniżej. W php robiłem jakiś prosty system logowania i rejestracji i dodawania słówek (taka strona do nauki słówek) ale jednak z tego zrezygnowałem bo miałem braki w podstawach html/css/js i postanowiłem najpierw na tym się skupić i wrócić do tego projektu później. Dlatego php i mysql również jest najniżej, coś tam umiem, łączyć się z bazą, podstawowe zapytania itp. ale szału nie ma.

Popracuję nad optymalizacją.

A oko robiłem sam :)
+2 głosów
odpowiedź 16 marca 2018 przez imklau Nałogowiec (42,480 p.)

Takie pytanko, bo w kodzie niby trochę jakby BEM jest ale też nie tak do końca. Czyli używasz go? Bo jeśli tak to:

<nav class="nav">
            <ul class="nav__list">
                <li class="nav__list-item"><a href="#o-mnie" class="nav__link scroll-link eye-focus_on">O mnie</a></li>
                <li class="nav__list-item"><a href="#umiejetnosci" class="nav__link scroll-link eye-focus_on">Umiejętności</a></li>
                <li class="nav__list-item"><a href="#projekty" class="nav__link scroll-link eye-focus_on">Projekty</a></li>
                <li class="nav__list-item"><a href="#kontakt" class="nav__link scroll-link eye-focus_on">Kontakt</a></li>
            </ul>
</nav>

przy takiej strukturze to poprawnym by było stworzenie np dodatkowego elementu:

<nav class="nav">
            <div class="nav__list-item">item</div>

            <ul class="nav__list">
                <li class="nav__list-item"><a href="#o-mnie" class="nav__link scroll-link eye-focus_on">O mnie</a></li>
                <li class="nav__list-item"><a href="#umiejetnosci" class="nav__link scroll-link eye-focus_on">Umiejętności</a></li>
                <li class="nav__list-item"><a href="#projekty" class="nav__link scroll-link eye-focus_on">Projekty</a></li>
                <li class="nav__list-item"><a href="#kontakt" class="nav__link scroll-link eye-focus_on">Kontakt</a></li>
            </ul>
</nav>

Bo nav__list-item jest w czymś, co powinno być w nav więc w tym przypadku to się zgadza. Chyba lepiej jest listę wydzielić jako osobny blok według BEM.

Coś w stylu:

<nav class="navbar">
            <ul class="nav">
                <li class="nav__item"><a href="#o-mnie" class="nav__link scroll-link eye-focus_on">O mnie</a></li>
                <li class="nav__item"><a href="#umiejetnosci" class="nav__link scroll-link eye-focus_on">Umiejętności</a></li>
                <li class="nav__item"><a href="#projekty" class="nav__link scroll-link eye-focus_on">Projekty</a></li>
                <li class="nav__item"><a href="#kontakt" class="nav__link scroll-link eye-focus_on">Kontakt</a></li>
            </ul>
</nav>

Przynajmniej według mnie tak wygląda to lepiej :)
Polecam poszukać przykładowych stron opartych o BEM i zobaczyć jak rozwiązują takie problemy, często też można sobie BEM połączyć np z OOCSS i wtedy przy pomocy Sassa to już w ogóle pięknie czytelny kod :P

Generalnie naprawdę kawał dobrej roboty odwaliłeś podczas nauki, co widać po Twoim kodzie.
Chyba jedna z lepszych prac tutaj laugh
Powodzenia dalej!

komentarz 16 marca 2018 przez gremlin Dyskutant (7,680 p.)
Dzięki za podpowiedzi! :)

Co do tego czy używam BEM to chyba najlepsza odpowiedź to "staram się".

Chyba faktycznie nazewnictwo klas muszę jeszcze dopracować.
+1 głos
odpowiedź 16 marca 2018 przez Beginer Pasjonat (22,110 p.)
Oko bardzo ładne i pomysłowe. (Z grafiką wcale nie jest źle - niepotrzebnie się sumitowałeś). O reszcie już kilkakrotnie się wypowiadałem.

Każdy pracodawca  w Polsce powinien Cię zatrudnić.
0 głosów
odpowiedź 15 marca 2018 przez Konginsan Początkujący (340 p.)
Witam, jestem nowy na tym forum. Przygodę z informatyką i programowaniem dopiero zaczynam, mogę więc tylko napisać że chciałbym już tyle umieć co Ty.

Pozdrawiam

Podobne pytania

0 głosów
1 odpowiedź 259 wizyt
pytanie zadane 25 maja 2017 w Nasze projekty przez Graatz Obywatel (1,490 p.)
0 głosów
1 odpowiedź 181 wizyt
pytanie zadane 28 października 2019 w Nasze projekty przez kowalskiwwwpl Nowicjusz (120 p.)
0 głosów
2 odpowiedzi 376 wizyt
pytanie zadane 22 marca 2019 w Nasze projekty przez Dawid_Predecki Obywatel (1,610 p.)

88,667 zapytań

137,276 odpowiedzi

306,615 komentarzy

58,867 pasjonatów

Motyw:

Akcja Pajacyk

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

Sklep oferujący ćwiczenia JavaScript, PHP, rozmowy rekrutacyjne dla programistów i inne materiały

Oto dwie polecane książki warte uwagi. Pełną listę znajdziesz tutaj.

...