-
Notifications
You must be signed in to change notification settings - Fork 1
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
file renaming, removed some files, changes to types, add variant to inputs #313
base: main
Are you sure you want to change the base?
Conversation
Failer pga tester, ikke så rart. Fikser det før merge. |
cardWidth?: string | (number | string)[]; | ||
|
||
}; | ||
import { CardProps } from 'Types'; |
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.
Det som endres sammen, bør ligge sammen. Med mindre CardProps brukes noen andre steder, er det ikke hensiktsmessig å flytte det ut av denne fila. Det skaper lenger avstand
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.
Jeg syns det er fint at vi er konsekvente så i så fall så bør vi flytte typen inn til komponentens egen fil, men det kan vi jo ikke gjøre for alle. Syns det koster ekstremt lite å ctrl klikke på typen for å komme rett til definisjonen uansett hvor den ligger..
➿ Code coverage
|
Dette blir første runde med endringer siden jeg ikke får lov til å bruke så mye tid på dette foreløpig. Jeg er klar over at dette har breaket disse page-templatene deres (pga PageContainer ikke lenger eksisterer) - det overlever vi. Om dere vil se at ting fortsatt fungerer sånn tålig greit så er det fint. Om dere ikke vil bruke tid på det så er det også greit, regner med jeg merger dette inn ila noen dager.
Det er fortsatt ganske mye jobb som gjenstår her, vanskelig å si når jeg gjør noe mer med dette, men jeg har lyst til å kunne installere denne pakken i våre applikasjoner istedenfor å ha det her så forhåpentligvis ikke så altfor lenge til..