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

IS-2795: Add sok in oversikten #549

Merged
merged 2 commits into from
Nov 27, 2024
Merged

IS-2795: Add sok in oversikten #549

merged 2 commits into from
Nov 27, 2024

Conversation

eirikdahlen
Copy link
Contributor

@eirikdahlen eirikdahlen commented Nov 21, 2024

Hva har blitt lagt til✨🌈

Legger til en v1 der vi har søk i modal. Den tar inn initialer og fødselsdato, og viser en liste i tabellformat som ligner litt på oversikten, bare uten checkboks og sortering.

Tenker å flytte det til tab/link i videre iterasjoner.

Screenshots

Knapp
image

Modal
image

Resultat
image

Validering
image

Ingen treff
image

@vetlesolgaard
Copy link
Contributor

Selv om det ikke koster oss noe utviklingsmessig å ha flere kolonner, så tror jeg det blir enklere for brukeren å få oversikt over søkeresultatet om vi for eks. bare har navn og personnummer (eller kun navn? 🤔). Vi unngår også bloat ved å bare inkludere det vi tror kan fungere som minimum av informasjon i første omgang. Flere kolonner synes jeg bør legges på som en reaksjon på noe vi får tilbakemelding om i produksjon at de savner. Synes det uansett er god praksis å skjære(🔪) ned løsninger til det absolutt minste, slik at dette er grunnlaget for videre læring. Da unngår vi å ha noe mer i løsning som vi antar gir verdi, og som vi kanskje aldri får svaret på om egentlig er nyttig for brukeren. (Føler dette er status på at vi viser innbygger sitt svar ute i oversikt på sen oppfølging)
Hva tenker dere? 😊

<TextField
label="Fødselsdato"
description="ddmmyy"
htmlSize={14}
Copy link
Contributor

Choose a reason for hiding this comment

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

Trenger vi å bruke htmlSize her? 🤔 Alle disse kunne kanskje også vært "small"? 😊

Suggested change
htmlSize={14}
size="small"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

htmlSize er bare hvor stor den skal være i boksstørrelse horisontalt. Så hvor mange tegn med tekst vi legger opp til, på en måte 🤔 Jeg prøvde med small, men det så litt pinglete ut haha, og så var skissen med vanlig størrelse så da bare gikk jeg for det. Kan diskutere det med Peter.

/>
<TextField
label="Fødselsdato"
description="ddmmyy"
Copy link
Contributor

Choose a reason for hiding this comment

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

Skulle til å kommentere at vi kunne ha dette i placeholder, men vi bør jo følge aksel 😊 Men vi bør kanskje tydeliggjøre hva "ddmmyy" betyr? Så bør vi kanskje også ha ddmmyyyy? 😊

@eirikdahlen
Copy link
Contributor Author

Selv om det ikke koster oss noe utviklingsmessig å ha flere kolonner, så tror jeg det blir enklere for brukeren å få oversikt over søkeresultatet om vi for eks. bare har navn og personnummer (eller kun navn? 🤔). Vi unngår også bloat ved å bare inkludere det vi tror kan fungere som minimum av informasjon i første omgang. Flere kolonner synes jeg bør legges på som en reaksjon på noe vi får tilbakemelding om i produksjon at de savner. Synes det uansett er god praksis å skjære(🔪) ned løsninger til det absolutt minste, slik at dette er grunnlaget for videre læring. Da unngår vi å ha noe mer i løsning som vi antar gir verdi, og som vi kanskje aldri får svaret på om egentlig er nyttig for brukeren. (Føler dette er status på at vi viser innbygger sitt svar ute i oversikt på sen oppfølging) Hva tenker dere? 😊

Anders og jeg snakket litt om det samme. Vi har vel allerede i brukertestene fått høre at virksomhet og veileder er nyttig informasjon å ha der. Vi snakket litt om at den enkleste å fjerne er kanskje frist-kolonnen, og evt varighet. Hendelse kan være fin å ha for å skille mellom de som er i oversikten og de som ikke har en hendelse på seg, men det er også bare en antakelse. Jeg er med på å kutte den ned der vi kan, men tenker vi kan diskutere det med design og fag også 👍🏼

@eirikdahlen eirikdahlen force-pushed the IS-2795-sok branch 2 times, most recently from 521f290 to 9421f1c Compare November 22, 2024 15:14
const [isModalOpen, setIsModalOpen] = useState<boolean>(false);
const [nameInitials, setNameInitials] = useState<string>('');
const [birthdate, setBirthdate] = useState<string>('');
const {
Copy link
Contributor

Choose a reason for hiding this comment

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

Hadde nok gått an å bruke query, som er mest "riktig", i stedet for mutation (som er ment for operasjoner som har side-effekter på serveren). F.eks. ved å ha en sokDto state her som settes i handleSubmit og som sendes inn i query og brukes til queryKey og om query er enabled.

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, men trodde vi ville unngå en egen state for kallet? Blir kanskje litt det samme nå som man belager seg på isSuccess for å bestemme om man skal vise resultater eller ikke 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmja, det blir kanskje litt knot å gjøre det på query-måten også så vi kan godt ha det sånn det er 👍

@eirikdahlen eirikdahlen force-pushed the IS-2795-sok branch 3 times, most recently from 00a7391 to 8427dc4 Compare November 26, 2024 13:54
'isOppfolgingISenFaseEnabled',
context
),
isSokEnabled: unleash.isEnabled('isSokEnabled', context),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fjerner gamle toggles i samme slengen

@@ -42,6 +42,7 @@ export interface PersonOversiktStatusDTO
extends PersonOversiktUbehandletStatusDTO {
fnr: string;
navn: string;
fodselsdato: Date;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Denne manglet, så det fører til en del endringer i mock-dataen også.

Comment on lines +25 to +27
const columns = allColumns.filter(
(column) => column.sortKey === 'NAME' || column.sortKey === 'FNR'
);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Har foreløpig løst kolonnene på denne måten, men kunne godt laget en egen liste med kolonner her hvis dere synes det er bedre.

@@ -64,3 +64,26 @@ export const getWeeksBetween = (date1: Date, date2: Date): number => {
export const addWeeks = (date: Date, numberOfWeeks: number): Date => {
return dayjs(date).add(numberOfWeeks, 'weeks').toDate();
};

export function parseDateString(dateString: string): Date | null {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Litt snacks her:
Forventer at man har "renset" stringen (tillater 101010, 10102010, 10.10.10 og 10-10-2010 etc) som gjøres via removePunctuation.
Har 100-års intervaller, slik at ddmm24 er 1924 og ddmm23 er 2023. Kan være utsatt for feil her, men tviler på at vi har noen på over 100 år eller på 0 år i vår db.

Siste if-sjekken hindrer også at man sender inn 10-80-90 der 80 er ugyldig måned, men slicingen av stringen tar ikke høyde for det i første omgang, så man må dobbeltsjekke etter opprettelsen av Date-objektet at man fortsatt har den samme datoen.

Comment on lines +21 to +23
export function isNumeric(str: string): boolean {
return /^\d+$/.test(str);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gir feil hvis noen skriver 10a188 feks

@eirikdahlen eirikdahlen merged commit 8217cda into master Nov 27, 2024
3 checks passed
@eirikdahlen eirikdahlen deleted the IS-2795-sok branch November 27, 2024 08:42
@@ -0,0 +1,4 @@
export interface SokDTO {
initials: string;
birthdate: Date;
Copy link
Contributor

@geir-waagboe geir-waagboe Nov 27, 2024

Choose a reason for hiding this comment

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

Gjerne norsk (synes jeg)

En litt annen sak: Skulle vi støtte søk med 6 første siffer i D-nummer? I så fall bør typen være string kanskje?

Copy link
Contributor Author

@eirikdahlen eirikdahlen Nov 27, 2024

Choose a reason for hiding this comment

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

Godt poeng 🤔 Vi kan se jo litt hvordan dette utfolder seg. I så fall burde vi bare sende string og så kan backenden gjøre det om til LocalDate.

Når det gjelder DTOen så kan vi jo endre i både APIet og her hvis vi ønsker det.

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.

5 participants