Formularz do zmiany widoku w React

0

Cześć, czy ktoś by mógł spojrzeć na mój projekt i podać kilka wskazówek? https://github.com/discoStar711/react-app

Wiem, że jest tutaj wielu doświadczonych i pomocnych programistów. Aplikacja pobiera wpisy z 3 kont na twitterze i wyświetla w 3 kolumnach. Za pomocą formularza można zmieniać układ kolumn i wyświetlać wpisy z danych dat oraz wybrać liczbę tweetów do wyświetlnia.

Teraz nie jestem pewien co dalej. Stworzyłem formularz client/src/component/layoutForm.js za pomocą, którego chcę zmieniać widok na stronie głównej aplikacji (zmieniać układ kolumn, filtrować tweety z danych dni i max. liczbę tweetów).

Jeśli nie ma żadnych ustawień w localStorage to chcę wyrenderować DefaultView jeśli są to zgodnie z tymi ustawieniami chciałbym wyrenderować CustomView z różnymi ustawieniami.

  1. Na razie logikę do wyrenderowania widoku trzymam w TwitterList, ale docelowo chciałbym dać do LayoutViewResolver, który by wyrenderował albo DefaultView albo CustomView. Czy to jest dobre posunięcie i prosiłbym o wskazówki jak to zrobić.

  2. Jak przekazać te 4 parametry do CustomView i jak napisać logikę by nie róbić if'a na każdy przypadek? <CustomView order={passOrder} startDate={passStartDate} endDate={passEndDate} tweetsAmount={passTweetsAmount} /> coś takiego? Czy źlę myślę?

  3. Nie wiem też jak sprawdzić czy localStorage jest pusty. twitterList.js linia 25 wydaję się nie działać, próbowałem też layoutSettings = null i layoutSettings === null i zawsze zwraca <h1>Custom</h1> w LayoutViewResolver. Nie wiem też czy to jest dobry sposób by zdecydować czy wyrenderować DefaultView czy CustomView.

  4. Czy mój kod nie jest zbyt proceduralny? Dotychczas programowałem w Javie i ciężko mi się przestawić na JS

Componenty w aplikacji:
App wyświetla formularz i widok z tweetami.
LayoutForm formularz z ustawieniami, zapisuje ustawienia do localStorage
TwitterList pobiera ustawienia z localStorage i wyświetla widok defaultowy lub customowy
LayoutViewResolver chciałbym przenieść logikę wybierania widoku do wyświetlenia tutaj i przesłać ustawienia do CustomView

Przepraszam za bazgroły w kodzie i za wiadomości do commitów.

1

Metoda render nie powinna zawierać efektów ubocznych, a ty się odwołujesz do zewnętrznego API (localStorage):
https://github.com/discoStar711/react-app/blob/master/client/src/component/twitterList.js#L20

 const layoutSettings = JSON.parse(localStorage.getItem('layoutSettings'));

To powinieneś sobie ciągnąć w jakimś componentDidMount (o ile moja wiedza jest wciąż aktualna, bo w ostatnich miesiącach twórcy Reacta tyle kombinują z tym, w której metodzie co powinno się znajdować, że chyba już nikt do końca nie wie, może poza pracownikami FB i osobami, które piszą artykuły techniczne na Medium o zmianach w React)

 console.log('local storage content:', layoutSettings);

do debugowania ok, ale takie rzeczy się oczywiście wywala na produkcji. Szczególnie w metodzie, która ma coś renderować (wywołania console.log potrafią zamulić przeglądarkę). Swoją drogą co zwraca ten console.log? Mam wrażenie, że problem może być właśnie w tym, że tych danych nie ma w localStorage. Ale być może te dane jednak są.

function CustomView() {
    return <CustomView />;
}

Tu masz wieczną rekurencję (swoją drogą ciekawe jak na to React zareaguje, czy po prostu zawiesi stronę, czy może wykryje, że jest wieczna rekurencja i rzuci błędem?)

Jak przekazać te 4 parametry do CustomView i jak napisać logikę by nie róbić if'a na każdy przypadek?

jakie 4 parametry z jakiego komponentu do którego. i co to znaczy nazewnictwo "pass" przed nazwą zmiennych?

Nie wiem też jak sprawdzić czy localStorage jest pusty. twitterList.js linia 25 wydaję się nie działać

Teraz masz przypisanie = zamiast sprawdzania równości (== albo ===), więc jak ma działać. Jeśli ustawiasz coś na null, to zawsze będzie to fałszywe i wejdzie w else.

if(layoutSettings = null) {

const getLayoutSettings = localStorage.getItem('layoutSettings');
console.log('after2: ', JSON.parse(getLayoutSettings));

Za bardzo na pałę to piszesz. Jeśli w kilku miejscach odwołujesz się do API (localStorage.getItem) i wykonujesz za każdym razem te same operacje dodatkowe (JSON.parse), to oznaka, że można ten kod albo wydzielić do osobnej funkcji i wywoływać tylko tę funkcję w kilku miejscach, albo iść jeszcze dalej i po prostu przestać się odwoływać z każdego komponentu do localStorage, tylko pobierać dane w jednym miejscu i podawać komponentom same dane (choćby przez props albo context).

Z drugiej strony w innych miejscach z kolei robisz abstrakcje, które nie są zbyt potrzebne, np. zamiast

if(layoutSettings = null) {
            return true;
        } else {
            return false;
        }

można by było napisać

return false // tego nie chcesz, ale to efektywnie napisałeś

albo:

return layoutSettings == null

i ogólnie cała koncepcja shouldRenderDefaultView wydaje się przerostem formy nad treścią, żeby robić z tego metodę, potem to przekazywać dalej do komponentu LayoutViewResolver zmieniając nazwę na viewToRender i podając do komponentu LayoutViewResolver

      let view = this.shouldRenderDefaultView(layoutSettings);

        return (
            <LayoutViewResolver viewToRender={view}/>

A w komponencie LayoutViewResolver brać skądś właściwość view

function LayoutViewResolver(props) {
    const shouldRenderDefaultView = props.view;

To dla mnie nawet prawa działać nie ma (bo podajesz viewToRender, a bierzesz view), ale z drugiej strony nie takie rzeczy się widziało w projektach, gdzie też się niby wydaje, że nie ma prawa działać, a okazuje się, że jakaś magia jest pod spodem, jakiś HOC, dependency injection i właściwości się magicznie biorą skądś.

Chociaż u ciebie nic takiego nie widziałem akurat. W każdym razie coś jest nie tak przemyślane z odpowiedzialnością poszczególnych komponentów oraz pewnie z nazewnictwem zmiennych (nazywanie zmiennych jest ważne, bo w zależności od tego, jak coś nazwiesz, to nadasz temu czemuś inne przeznaczenie w kodzie i inaczej będziesz o tym myślał). Wydaje mi się, że tworzysz za dużo tych "bytów" i nie możesz się nawet zdecydować na 1 konkretną nazwę (i jeden byt nazywasz w jednym miejscu "view" w drugim "viewToRender" w innym "shouldRenderDefaultView"... i potem ci się to myli wszystko).

0

@LukeJL: dzięki za odpowiedź

To powinieneś sobie ciągnąć w jakimś componentDidMount

Dzięki, poprawię.

Swoją drogą co zwraca ten console.log?

Czy w localStorage są jakieś dane.

Tu masz wieczną rekurencję

Dzięki. Powinienem dać komentarz TO DO po prostu.

jakie 4 parametry z jakiego komponentu do którego. i co to znaczy nazewnictwo "pass" przed nazwą zmiennych?

Chcę przekazać parametry z formularza https://github.com/discoStar711/react-app/blob/master/client/src/component/layoutForm.js do localStorage. Na starcie aplikacji w https://github.com/discoStar711/react-app/blob/master/client/src/component/twitterList.js chcę sprawdzić czy są jakieś ustawienia w localStorage potem odczytać dane z localStorage jeśli tam są i przekazać do CustomView co wyświetlić. Jeśli nie ma danych w localStorage to wyrenderować DefaultView

Teraz masz przypisanie = zamiast sprawdzania równości (== albo ===)

Sprawdzałem równość i robiłem jeszcze if(!layoutSettings) i dostawałem błąd Cannot read property of null. Nie wiem jak sprawdzić czy w localStorage są jakieś dane, bo chcę zdecydować jaki widok wyświetlić. Czy w źle się do tego zabieram i powinnienem użyć innego sposobu?

pobierać dane w jednym miejscu i podawać komponentom same dane

Dzięki, spróbuję to zrobić.

i ogólnie cała koncepcja shouldRenderDefaultView wydaje się przerostem formy nad treścią, żeby robić z tego metodę

To powinien robić LayoutViewResolver zgadza się?

To dla mnie nawet prawa działać nie ma

Przepraszam. Wychodzi nie znajomość JS i React.

Wydaje mi się, że tworzysz za dużo tych "bytów" i nie możesz się nawet zdecydować na 1 konkretną nazwę

Dzięki. Spróbuję to poprawić.

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