Skip to content
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

Use Søknadvedlegginfo instead of Vedlegg in Søknad #72

Merged
merged 11 commits into from
May 31, 2018
Merged

Conversation

frodehansen2
Copy link
Contributor

No description provided.

@frodehansen2 frodehansen2 requested a review from a team May 31, 2018 10:52
@@ -1,21 +1,21 @@
export type UtenlandsoppholdPeriode = {
export interface UtenlandsoppholdPeriode {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pga. linting

Copy link
Contributor

@erlendev erlendev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ser bra ut dette her. Kun en kommentar på Søknadsvedlegg.

En annen litt mer generell ting: er ikke vedleggsoversikten(e) egentlig bolker, ikke spørsmål (ser i koden her at de blir sendt til render-funksjoner på Spørsmål og ikke Bolk)? Sånn jeg ser det er opplastning av vedlegg med visning av liste + mulighet for å fjerne elementer fra lista, konseptuelt likere en bolk enn et spørsmål.

type: SøknadsvedleggType;
}

const Søknadsvedlegg: React.StatelessComponent<Props> = ({ type }) => (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Er dette bare for typingens skyld? Foreslår å heller legge inn typen (evt. generic om det lar seg gjøre) i common enn å lage en wrapperkomponent i hvert prosjekt for dette behovet.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ja, det er i utgangspunktet kun for typing, og jeg tror det gir større verdi enn det koster å lage en egen for hvert prosjekt. Men skal se mer på generic

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Det blir vel omfattende å få inn typing på gruppen i hele attachments, så velger å holde det so det er enn så lenge

@frodehansen2
Copy link
Contributor Author

Jeg har ikke sett så mye på bruken av den, men ja det er nok kanskje mer som en bolk. Det får vi fikse etterhvert som vi lager sidene

@frodehansen2 frodehansen2 merged commit fe3faf9 into master May 31, 2018
@frodehansen2 frodehansen2 deleted the storage2 branch May 31, 2018 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants