-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Tee nappulat hyväksymisvaiheen pdf:ien esikatselulle ja katselulle #345
feat: Tee nappulat hyväksymisvaiheen pdf:ien esikatselulle ja katselulle #345
Conversation
f624ae9
to
3c42b26
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Toi muutos hyvaksymispaatosKuulutus-schemaan ei ainakaan taida olla toivottu. Bäkkäri ei hyväksy tyhjiä stringejä.
Virkamiespuolella lukutilassa noiden PDF:ien linkeissä on enemmä välityksiä ja niissä on käytetty sitä
Yks havainto, minkä nyt tässä PR:ssä tein, vaikkei tässä PR:ssä sitä komponenttia muokattukaan... 'Hyväksy ja lähetä' -painike (ja todennäköisesti 'Palauta'-painike) submittaa html formin, eli event.preventDefaulttia ei tapahdu.
src/components/projekti/hyvaksyminen/kuulutuksenTiedot/Painikkeet.tsx
@@ -1,4 +1,4 @@ | |||
version: "3.8" | |||
version: "3.3" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Miksi päätit vaihtaa tän version tässä pr:ssä? Ei siis sinänsä väliä, mutta aiheutti ihmetystä, kun mulla oli säädetty skip-worktree gitissä tähän tiedostoon toistaseks ku käytän eri imagea localstackista.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tämä oli moka!
{addEmptyOption && <option />} | ||
{addEmptyOption && <option value="" />} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Miks teit tän muutoksen? Noh.. Ei tää käsittääkseni muuta toiminnallisuutta ollenkaan, eli ei väliä. Toi value defaulttaa tyhjäks stringiks kuitenkin. Kirjottelin ton valuen arvoksi tyhjän stringin ihan vaan selkeyden vuoksi, ettei kuvittele sen olevan undefined tai jotain muuta.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mulle tuli ongelmaa ja konsoli herjas, että muuttaa ei-kontrolloitua komponenttia kontrolloiduksi. Tämä ratkaisi sen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Juu tämä OK 👍
hallintoOikeus: Yup.string().required("Hallinto-oikeus on valittava"), | ||
hallintoOikeus: Yup.string(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Eikös tämä ole edelleen pakollinen kenttä? Käyttöliittymässä ainakin on asteriksi ton kentän kohdalla
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tippunut jostain syystä pois.
No description provided.