Closed KamilZm closed 5 years ago
This pull request is automatically deployed with Now. To access deployments, click Details below or on the icon next to each push.
Latest deployment for this branch: https://volby2.hlidacstatu.cz
V commitu 5e21455 přidáno scrollování na prvek formuláře po stisknutí tlačítka, aby se uživatel (hlavně v mobilní verzi) nedostal vždy na začátek stránky, protože to na mě působili strašně zmatečně (stiskl tlačítko a viděl zase od začátku to, co předtím).
V commitu 50f1a6f kompletně předělány validace. Bohužel se mi zatím nepodařilo udělat rozumnou validaci na autocomplete PSČ - když se uživatel vrátí zpět z náhledu to blbě vypadá, protože se to tváří, že není položka vybraná, ale je, jen to autocomplete nezobrazuje, protože tam nemá žádný přiřazení objekt. Ale data jsou OK.
Je to teď nachystané tak aby šel přidat i validační sumář. Ale úplně čisté řešení to není, lepší by bylo, kdyby se např. property v userdata nastavovaly přes setter a přímo se prováděla validace. Ale do toho jsem zatím nešel.
Kamile, build je rozbitý protože je potřeba přidat nové soubory appconfig.js. appconfig.txt do now.json konfigurace, např. takhle:
"builds": [
{ "src": "index.html", "use": "@now/static" },
{ "src": "appconfig.js", "use": "@now/static" },
{ "src": "appversion.txt", "use": "@now/static" },
{ "src": "favicon.ico", "use": "@now/static" },
{ "src": "assets/**", "use": "@now/static" }
],
Když mi ten pullrequest upravíte (netřeba zavírat, stačí pushnout další commit do vašeho repa https://github.com/KamilZm/Volicsky-Prukaz/tree/euro2019/) tak já budu moct potom zkontrolovat tu výslednou deploynutou verzi.
@zzen OK, omlouvám se a díky za vysvětlení, přidáno.
Vrtalo mi teď hlavou, že jsem to nemusel dělat dříve, když jsem přidával soubory do /assets/
, ale teď tam vidím { "src": "assets/**", "use": "@now/static" }
, čímž je to jasné.
Ke shrnutí:
Úvodem - nepovažuji to samozřejmě za celý refaktor a konečné řešení. Dělal jsem to, co mě nejvíc pálilo, na co jsem sahal kvůli validacím a na co byl čas.
ad 1 (pull request) - souhlas, chtěl jsem to, ale problém byl, že jsem nepřišel na to, jak udělat další PR, protože dokud jsem měl otevřený jeden, tak jsem neuměl udělat druhý. Commity se automaticky přidávaly k tomu otevřenému.
ad 2) (formáty dat) - asi není větší problém opravit. Zatím jsem tam moc neřešil EN verzi (vzhledem k času), protože tu současnou stejně skoro nikdo používat nebude, viz diskuze ve Slacku. Ten, kdo ji použije, tak to snad nějak pochopí. Do budoucna jsem to plánoval předělat.
ad zbytek (validace, částečná lokalizace a předělání 'userData' ze skalárů na objekty s logikou): Částečnou lokalizaci jsem ze svého pohledu vysvětlil výše.
Tahle trojice problémů tvoří 95% tohoto PR. Jak jsem pochopil, tak mé řešení validací se Vám moc nezdá (nepřehledné) , lokalizaci nepreferujete (z Vašeho pohledu přijde výhodnější mít dvě paralelní verze) a změnu v userData zřejmě také ne. Z tohoto pohledu mi teď přijde nejjednodušší to celé, až na konfiguraci 79e260a a scrollování 5e21455 odrolovat, nebo PR nemergovat a zavřít, aby se nemusel mergovat s tím, co dělal @woodarius a ty první dvě změny udělám znovu.
Jak jsem již psal, dělal jsem to na své triko a zlobit se nebudu. Vy jste šéf, máte v hlavě nějakou vizi tohoto projektu a jste znalostmi asi tak 1000x přede mnou.
PR po dohodě s @woodarius uzavírám. Provedené úpravy budou znovu vloženy přímo do repo do zvláštní větve (ne přes fork).
@zzen @michalblaha Vyřešil jsem to tak, že se informaci o verzi bere ze souboru
/appversion.txt
. Je to pro generování jednodušší, než něco vyměňovat v HTML souboru a také je to bezpečnější, protože nehrozí, že by se mohl index.html tím zásahem díky nějakému problému rozdrbat. @zzen Pošlete prosím @michalblaha info, jak zjistit hash commitu, který jde do produkce @michalblaha Zjištěný hash prosím při publikování uložte do/appversion.txt
Jako výchozí je v/appversion.txt
verze000000100000000000000000000000000000000
, takže kdyby generování selhalo, zobrazí se ta (resp. 0000001) nebo žádná, pokud obsahem toho souboru nebude pouze SHA1).