Oppgrader nav-pakker #1095
Oppgrader nav-pakker #1095
Conversation
|
|
||
| expect(forrigeDag?.getAttribute('disabled')).not.toBeNull(); | ||
| const [tilOgMedKalender] = getAllByRole('dialog'); |
There was a problem hiding this comment.
Tidligere gikk vi ut fra at container.querySelector('[aria-label="torsdag 9"]'); kun ville finne datoen i kalender for til-og-med, men nå som Datepicker rendrer kalenderen til DOM uansett om den vises for brukeren vil det første treffet være fra fra-og-med-kalenderen, der datoen er tillatt å velge.
Skriver om til at vi henter ut til-og-med-kalenderen ved å se etter dialog-elementer som er tilgjengelige for brukeren (i dette caset er fra-og-med-kalenderen aria-hidden og treffer derfor ikke på getAllByRole('dialog'))
|
|
||
| const forrigeDag = tilOgMedKalender.querySelector('[aria-label="torsdag 9"]'); | ||
|
|
||
| expect(forrigeDag).toBeDisabled(); |
There was a problem hiding this comment.
Dette gir en bedre feilmelding:
expect(element).toBeDisabled()
Received element is not disabled:
<button aria-label="torsdag 9" class="rdp-button_reset rdp-button rdp-day" name="day" role="gridcell" tabindex="-1" type="button" />
Heller enn før:
expect(received).not.toBeNull()
Received: null
|
Skulle det ha stått react 18 i PR beskrivelsen? 😄 |
package.json
Outdated
| @@ -110,7 +110,7 @@ | |||
| "constate": "^3.3.2", | |||
| "cookie-parser": "^1.4.6", | |||
| "cors": "^2.8.5", | |||
| "date-fns": "^2.30.0", | |||
| "date-fns": "^3.0.0", | |||
There was a problem hiding this comment.
Det finnes en nyere versjon (3.3.1) enn dette også, skulle vi bare ha oppgradert dit med en gang?
There was a problem hiding this comment.
Ja, ser at det er den som ble installert også. Kan endre til det!
| @@ -71,9 +76,12 @@ describe('Datovelger', () => { | |||
| </TestProvidere> | |||
| ); | |||
|
|
|||
| const tilOgMedÅpneknapp = getAllByRole('button')[1]; | |||
| const [, tilOgMedÅpneknapp] = getAllByRole('button'); | |||
There was a problem hiding this comment.
Av nysgjerrighet: Hvordan funker denne måten å skrive det på/hva betyr det? [, tilOgMedÅpneknapp]
There was a problem hiding this comment.
Dette er array destructuring, der jeg ser bort fra første (nullte) element. Det er mest digg når du skal lage konstanter ut fra flere elementer i et array, f.eks. const [idag, imorgen] = listeMedDager, men jeg synes det også er fint som her når vi vil hoppe over første element. Liker det litt bedre enn å lese indekslesing etter et funksjonskall 😇
Men det krever at det er lesbart for andre, da. Så om det ikke gir mening trenger vi ikke å gjøre det sånn
Se MDN: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment
@idaame ja 🙈 |
Har trykket meg gjennom hele, minus kvittering siden jeg ikke kjørte familie-dokument. Burde jeg gå helt til kvittering, synes du? |
💰 Hva forsøker du å løse i denne PR'en
Favro: NAV-18496 Teknisk vedlikehold for å gjøre klart til videreutvikling 👷♀️ 🚧
Endringer vi tar med oss:
🔎️ Er det noe spesielt du ønsker å fremheve?
Er det noe du er bekymret eller usikker på? Beskriv det gjerne her.
Ifølge readme skal man kunne kjøre med node >=18.13.0, men med node 20 fant ikke dev-server filene den forventet. Kunne se at filene lå der, tipper det har med file resolutions og antatt file extension. Men legger inn en kommentar i readme om at v20 ikke funker ennå.
Pre-commit krasjet også, den fant ikke
lint-stagedglobalt installert hos meg. Men vi installerer den pakken lokalt, den er definert som devDependency, så det ryddigste blir å kjøre den mednpxså den hentes fra/node_modules.💬 Ser at
concurrentlysom vi bruker for å kjøre scripts i parallell bruker forrige versjon avdate-fnsslik at vi ender opp med å ha begge lokalt. Men her tar jeg gjerne en diskusjon om hvorvidt vi trenger den pakken for å kjøre to scripts i parallell. Testet å kjøre build serielt, det tar 0.6s lengre enn medconcurrently. Vi må i så fall skrive om scriptet forstart:devlitt men det tenker jeg er mulig å få til✅ Checklist
Har du husket alle punktene i listen?
Jeg har ikke skrevet tester fordi:
Har smoke testet manuelt