Wątek przeniesiony 2020-09-27 21:43 z JavaScript przez cerrato.

Code Review - React

0

Cześć!
Proszę o code review. Strona napisana przy użyciu Reacta, Boostrapa i w myśl BEM.
https://github.com/Dariuszb94

2

Zamień link do profilu na link do projektu.

3

Nie mam dużego doświadczenia z Reactem, ale na pierwszy rzut oka już się rzuca kilka rzeczy.

Po pierwsze, commit message o treści... "commit message"... - nie rób tak, staraj się od początku korzystać poprawie z gita

Po drugie, brak testów.

A co do samego kodu:

if (donation===50){

      this.setState({ button50Active: true });
      this.setState({ button70Active: false });
      this.setState({ button100Active: false });
    }
    
    if (donation===70){
      this.setState({ button50Active: false });
      this.setState({ button70Active: true });
      this.setState({ button100Active: false });
    }
    if (donation===100){
      this.setState({ button50Active: false});
      this.setState({ button70Active: false });
      this.setState({ button100Active: true });
    }

W wielu miejscach, również tutaj, brakuje poprawnego formatowania kodu.
No i button50Active - taka nazwa nic nie mówi, staraj się lepiej dobierać nazwy dla zmiennych, obiektów, funkcji, generalnie wszystkiego, tak by można było po nich wywnioskować o co chodzi.

Poza tym, całość tych ifów można uprościć

this.setState({ button50Active: donation === 50});
this.setState({ button70Active: donation === 70});
this.setState({ button100Active: donation === 100});

Dalej,

<article className="steps-container__content d-flex flex-column flex-lg-row align-items-center align-items-lg-start col-12">
    <Step1 />
    <Step2 />
    <Step3 />
    <Step4 />
</article>

Każdy komponent Step1-4 to kopia poprzedniego z innym numerkiem i chyba innym tekstem

<div className="step__numer d-flex justify-content-center align-items-center">
        2
 </div>

Czemu nie zrobić 1 komponentu, który numerek / tekst przyjmuje jakos props?

let random = Math.floor(Math.random() * (999999 - 111111 + 1) + 111111);

Używaj const gdy to możliwe

...
    WSPIERAM
</button>

Poczytaj o internationalization

1

Te opcje z poprzedniego posta w ogóle mogłyby być w <input type="radio">:

W Readme nie ma screenshota, czyli randomowa osoba, która mogłaby być potencjalnie zainteresowana taką apką, oleje to (przynajmniej ja bym tak zrobił).

import formBg from "../../../../Assets/formBg.png";

bardzo kruchy taki import, bo jak przeniesiesz folder to będziesz musiał zmieniać importy. Ja bym przemyślał, gdzie trzymasz grafikę. Może lepiej trzymać grafikę w tym samym folderze, w którym masz moduł? Webpack można skonfigurować, żeby to zamieniał sobie po drodze:
https://webpack.js.org/guides/asset-management/

plus np. apki stworzone z użyciem create-react-app mają to z automatu.

No i masz wiele komponentów bezstanowych, nie rozumiem czemu je robisz na klasach, np.:

class Socials extends Component {
    render() {

zamiast po prostu

function Socials() {

komponenty stanowe też można już na funkcjach robić (z użyciem hooks), ale niektórzy nie lubią hooków i robią na klasach (ja jestem po stronie hooków, ale rozumiem, że ktoś może woleć klasy). No ale tutaj nie masz i tak stanu, żadnych metod, więc żadna potencjalna korzyść, a tylko więcej pisania.

1 użytkowników online, w tym zalogowanych: 0, gości: 1