Cześć!
Proszę o code review. Strona napisana przy użyciu Reacta, Boostrapa i w myśl BEM.
https://github.com/Dariuszb94
Zamień link do profilu na link do projektu.
Poprawny link: https://github.com/Dariuszb94/agRekrut
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
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.