cesko-digital / app

Komunitní aplikace Česko.Digital
https://app.cesko.digital
BSD 3-Clause "New" or "Revised" License
23 stars 40 forks source link

feat: tab bar with hash navigation #970

Closed drahoja9 closed 7 months ago

drahoja9 commented 7 months ago

Closes #955

Prozatím jsem to udělal dle zadání jako hash router. Má to ale jednu nevýhodu ve spojení s SSR – jelikož se část za hashem nikdy neodesílá na server, vždy dojde nejdřív k vyrenderování výchozího (prvního) tabu a až při hydrataci na klientovi se to přerenderuje na správnou záložku. V reálnym nasazení to asi není moc velkej problém, protože to člověk většinu času ani nepostřehne (lze vidět např. zde). V lokálním setupu při vývoji je to ale dost markantní a pomalejší klienti by s tim možná mohli mít stejnej problém?

zoul commented 7 months ago

Super! Přijde mně to bezvadně použitelné i takhle, ale chvilku jsem nad tím dumal a došlo mně, že bychom asi mohli nějak použít router a udělat z tabů normální podstránky. Držel jsem se client-side tab baru ze setrvačnosti, ale když je to SPA a ta navigace mezi stránkami je stejně na klientovi, tak by rozdělení na samostatné stránky mohlo být přirozenější.

Chvilku jsem přemýšlel, jak to vůbec vygooglovat, a nakonec jsem narazil na tohle. To je přesně to, co chceme, ne? Renderuje se to na serveru a routuje se to pomocí cesty. Co si o tom myslíš?

(Za mě je klidně možno mergnout tohle, ale kdyby se ti chtělo to vyzkoušet s těmi paralelními routami, jsem pro. Ta organizace jednotlivých tabů do podstránek mně přijde pěkná.)

drahoja9 commented 7 months ago

Narazil jsem na ten totožnej odkaz a přesně mě taky napadlo, že by to asi mohly bejt samostatný stránky. Co se mi moc nelíbí na tom řešení je, že musíš definovat celou cestu u každýho odkazu (např. <Tab href="/previews/tab-next-router">Hot</Tab>), což je trochu duplicitní s tim, že momentálně tu URL path určuje hierarchie složek (takže se to pak může začít rozcházet, když někdo přesune složku a nezmění cestu v kódu nebo naopak). Nebo jsem něco přehlídl a chápu to špatně? :)

Jinak další nevýhoda je za mě nová závislost @ariakit/react, která je zatim asi zbytná.

zoul commented 7 months ago

Já jsem nad tím ještě teď přemýšlel v souvislosti se změnami v datovém dashboardu (viz novou verzi) a došla mně ještě jedna zásadní výhoda toho uspořádání do samostatných stránek – mohli bychom díky tomu výrazně posunout hranici mezi serverem a klientem. Když budou jednotlivé záložky (tabs) samostatné stránky, tak můžou dělat vlastní asynchronní operace na serveru – zatímco když jsou na klientovi, tak už musí data načítat z klienta nebo je dostávat „shora“ od nejbližší serverové komponenty. (Tak jsem na to narazil – musel jsem data metrik načítat v serverové stránce a předávat do záložky přes jednoho prostředníka, což není pěkné.)

To je pro mě jasný argument to udělat přes ty samostatné stránky. Tu @ariakit/react bych zatím klidně skočil a to routování se buď někde poddá :), anebo na to jde udělat nějaký helper à la src/routing.ts, který zařídí, aby se to nerozbíjelo.

Akorát mně to teď nepřijde jako férová nabídka z mé strany: „Pojďme to udělat úplně jinak, než jak to máš už hotové.“ :) Kdybys chtěl, můžem to klidně mergnout takhle a k tomu přepisu na paralelní routy se dostat později. (Stejně jsem ti mezitím ujel s masterem 🤦) TLDR: Řekni si!

drahoja9 commented 7 months ago

Za mě úplně v pohodě, pokud v tom vidíš ty benefity, tak to tak udělejme. Já na to mám zatim moc malej kontext a scope a vidim v tom jen ty lokální nevýhody.

Zkusim teda taby předělat na samostatný stránky. Jen jsem nepochopil, co přesně že bys udělal s tou závislostí @ariakit/react? 😀 Nainstalovat nebo zkusit poslepovat nějak po svym?

Co se týče tohodle PR, jsou pro mě naprosto v pohodě obě varianty. Pokud myslíš, že to někdo teď v nejbližší době nějak využije, klidně mergněme tohle a udělám nový PRko na samostatný taby. Pokud ne, udělám to tady. Ujetí masteru je ten nejmenší problém, jsem mistr rebasování. 😁 Ale koukám, že jsem úplně přímo neodpověděl, takže – za mě bych tohle mergnul, je to nějak funkční řešení, i když ne ideální a dodělávky bych dal do novýho PR (přechod na samostatný stránky už asi nebude tak triviální jako tohle). 🙂

zoul commented 7 months ago

Zkusil jsem to nahodit v #972. Pozitiva:

Negativa:

Ještě nevím, co si o tom myslím, nechám to odležet.

zoul commented 7 months ago

PS: Souhlasím, mergnul bych tohle a s tím serverovým tab barem se můžem podle času bavit dál.

martinwenisch commented 7 months ago

@zoul adresu a tedy i jaky tab je aktivni mas normalne na serveru, jenom se ta stranka musi fakt nacist (musis pouzit Nextovy Link).

Jinak me prijde pouziti router jako komponenty v Nextu 13+ obsolete. Je to zbytecna abstrakce (client router) nad abstrakci (path v URL adrese, kterou routujes file routerem v app directory).

drahoja9 commented 7 months ago

Tak to vypadá podstatně jednodušeji, než jsem myslel. Paráda! 👍

Klidně přesuňme případnou diskuzi do toho tvýho PRka, případně můžu ve volnym čase zkusit pofixovat problémy, který jsi nastínil a tohle mergněme.

martinwenisch commented 7 months ago

Tady je to uplne nadherne videt. Ty dve router abstrakce jsou na sebe vzajemne preveditelny (dukaz necham za domaci ukol).

  1. mam array s hash + content a renderuju podle keywordu hash v page.tsx /stats
  2. mam app router /stats/[hash]/page.tsx a renderuju podle hash promenne, co dostanu do page.
Screenshot 2024-02-02 at 1 27 04 PM
zoul commented 7 months ago

Martine, máš pravdu, díky. (Upravil jsem podle toho #972.) Ta komponenta s překlikávacími taby by pak ale logicky skončila ve společném layout.tsx, ne? Dumám nad tím, jak zařídit označení aktivního tabu tam, aniž by ten layout musel být klientská komponenta.