Ok 497 tabs #10
Ok 497 tabs #10
Conversation
| lineHeight: '1rem', | ||
| }, | ||
| '.hakukohdeLabel': { | ||
| fontWeight: 500, |
There was a problem hiding this comment.
Tyylioppaassa sallitut lihavoinnit on 400, 600 ja 700
| <h2 className="organisaatioLabel" title={hakukohde.organisaatioOid}> | ||
| {getTranslation(hakukohde.jarjestyspaikkaHierarkiaNimi)} | ||
| </h2> | ||
| <h2 className="hakukohdeLabel" title={hakukohde.oid}> | ||
| {getTranslation(hakukohde.nimi)} | ||
| </h2> |
There was a problem hiding this comment.
Tässä on kaksi h2 tason otsikkoa peräkkäin, mikä ei taida semanttisesti olla ihan oikein. Toinen vois ehkä olla h3? Tuo title-attribuuttiin laitettu oid hämmentää myös hieman.
There was a problem hiding this comment.
Näyttäisi erikoiselta jos laittaisin vahvemmin näkyvän tekstin h3:na ja sitä ei voi myös käyttää ennen h2:sta.
Muutan tämän niin, että tuossa on vain yksi h2 tagi ja spaneilla + br:llä erotan nuo (class)tyylit.
| {Tabs.map((tab, index) => ( | ||
| <div | ||
| key={'hakukohde-tab-' + index} |
There was a problem hiding this comment.
Tässä ei suuremmin väliä, mutta miksei käyttäisi suoraan jo olemassaolevaa yksilöivää tunnistetta key-attribuuttina, kun sellainen kerran on?
| {Tabs.map((tab, index) => ( | |
| <div | |
| key={'hakukohde-tab-' + index} | |
| {Tabs.map((tab) => ( | |
| <div | |
| key={tab.title} |
tests/e2e/hakukohde.spec.ts
Outdated
| const ToinenAsteTabs: BasicTab[] = [ | ||
| { title: 'Hakeneet', route: 'hakeneet' }, | ||
| { title: 'Valinnan hallinta', route: 'valinnan-hallinta' }, | ||
| { title: 'Valintakoekutsut', route: 'valintakoekutsut' }, | ||
| { title: 'Pistesyöttö', route: 'pistesyotto' }, | ||
| { title: 'Harkinnanvaraiset', route: 'harkinnanvaraiset' }, | ||
| { title: 'Hakijaryhmät', route: 'hakijaryhmat' }, | ||
| { title: 'Valintalaskennan tulos', route: 'valintalaskennan-tulos' }, | ||
| { title: 'Sijoittelun tulokset', route: 'sijoittelun-tulokset' }, | ||
| ]; |
There was a problem hiding this comment.
Nämä voisi olla vakiona jossain constants.ts:ssä tms. kun samat oli komponentissakin käytössä.
| const ToinenAsteTabs: BasicTab[] = [ | |
| { title: 'Hakeneet', route: 'hakeneet' }, | |
| { title: 'Valinnan hallinta', route: 'valinnan-hallinta' }, | |
| { title: 'Valintakoekutsut', route: 'valintakoekutsut' }, | |
| { title: 'Pistesyöttö', route: 'pistesyotto' }, | |
| { title: 'Harkinnanvaraiset', route: 'harkinnanvaraiset' }, | |
| { title: 'Hakijaryhmät', route: 'hakijaryhmat' }, | |
| { title: 'Valintalaskennan tulos', route: 'valintalaskennan-tulos' }, | |
| { title: 'Sijoittelun tulokset', route: 'sijoittelun-tulokset' }, | |
| ]; | |
| const TOINEN_ASTE_TABS: BasicTab[] = [ | |
| { title: 'Hakeneet', route: 'hakeneet' }, | |
| { title: 'Valinnan hallinta', route: 'valinnan-hallinta' }, | |
| { title: 'Valintakoekutsut', route: 'valintakoekutsut' }, | |
| { title: 'Pistesyöttö', route: 'pistesyotto' }, | |
| { title: 'Harkinnanvaraiset', route: 'harkinnanvaraiset' }, | |
| { title: 'Hakijaryhmät', route: 'hakijaryhmat' }, | |
| { title: 'Valintalaskennan tulos', route: 'valintalaskennan-tulos' }, | |
| { title: 'Sijoittelun tulokset', route: 'sijoittelun-tulokset' }, | |
| ] as const; |
There was a problem hiding this comment.
Nimesin tuon nyt uudelleen, mutta viimeistään OK-536:n yhteydessä logiikka miten nuo tabit muodostuu muuttuu paljon dynaamisemmaksi, joten mielestäni ei kannata laittaa tässäkään vaiheessa samaan paikkaan noita. Ne ei myös ole ihan yksi yhteen samat.
Oletko lisännyt tarvittavat yksikkö- tai ui-testit toiminnallisuudelle? Kyllä
Oletko tarkistanut ja päivittänyt riippuvuudet? Kyllä
Oletko kokeillut toimiiko käyttöliittymä mobiilissa landscape moodissa? En