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

feat(error): updating error handling #1723

Merged
merged 17 commits into from
Dec 4, 2024
Merged

feat(error): updating error handling #1723

merged 17 commits into from
Dec 4, 2024

Conversation

adrianflatner
Copy link
Collaborator

@adrianflatner adrianflatner commented Nov 6, 2024

Error handling client og actions

Rydder opp i error handling for å gi riktig feedback til bruker ved bruk av forms. React 19 innfører useActionState som gjør det lettere å håndtere. Fikser opp i når det skal fyres av toasts.

Note

Sjekk ut docs på feilhåndtering for oversikt over hva som er nytt.

@adrianflatner adrianflatner marked this pull request as ready for review November 8, 2024 11:18
tavla/app/(admin)/utils/index.ts Outdated Show resolved Hide resolved
tavla/app/(admin)/edit/[id]/components/Footer/actions.ts Outdated Show resolved Hide resolved
tavla/app/(admin)/utils/index.ts Show resolved Hide resolved
tavla/app/(admin)/utils/index.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@emilielr emilielr left a comment

Choose a reason for hiding this comment

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

Vi har generelt en del forskjellige måter vi håndterer error på. Noen plasser henter vi ut formData i server-actionen, mens andre ganger sender vi det inn som en prop til server-actionen. Vi burde egentlig definere en standard her.

I tillegg er det ikke 100% etter retningslinjene til Entur å fyre av en toast når noe går galt. Nevnte det i en kommentar lengre nede her, men det har med brukeropplevelsen å gjøre. En toast varer bare i x sekunder, og den vises ikke så godt, og er heller ikke plassert der hvor brukeren interagerer med løsningen (om noe skulle gå galt). I utgangspunktet burde vi se på andre måter å vise frem at noe har fått galt for brukeren, slik at brukeren får mulighet til å prøve på nytt/endre. F. eks:
image

tavla/app/(admin)/edit/[id]/components/Footer/actions.ts Outdated Show resolved Hide resolved
tavla/app/(admin)/utils/index.ts Outdated Show resolved Hide resolved
tavla/app/(admin)/utils/handleError.ts Outdated Show resolved Hide resolved
tavla/app/(admin)/utils/handleError.ts Outdated Show resolved Hide resolved
…gs, and move CountiesSelect out of MemberAdministration
@SelmaBergstrand
Copy link
Collaborator

Status: denne PR-en introduserer useActionState på alle actions som trigges når et form sendes inn i både tavle-redigering og organisasjons-redigering. Da returneres "resultatet" (enten ingenting eller en feilmelding) fra det firebase-kallet, og brukeren informeres med en toast om lagring fungerte eller ikke (standardisert via fireToastFeedback

Senere vil vi endre på dette slik at bruker ikke får toast-feedback når noe går galt, men heller en feilmelding i formet. Se organizations/components/DefaultColumns/edit for et eksempel: toasten vises kun om lagringen har fungert. Hvis ikke lagres feilmeldingen i en state-variabel og vises på skjema via getFormFeedbackForField. På sikt skal alle settings gjøre det sånn. Skiller det ut i egen oppgave for at denne PR-en ikke skal bli så stor

Copy link
Collaborator

@emilielr emilielr left a comment

Choose a reason for hiding this comment

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

Deeiilig å se at sjekker er flyttet inn på server-siden :prayage: !! 🚀

Det eneste jeg også tenker på som hadde vært fint i fremtiden er å få samlet alle innstillinger som brukes både i org og på tavlens egen side (f. eks footer) at de bruker én felles action. Da slipper man å huske å endre to plasser om det blir logikk-endring i footer feks. Det er noe vi kan ta i en egen oppgave da - evt når vi finner mer ut av hva vi vil gjøre med standardinnstillinger osv.

Annet spm: vet det er dumt å vente, men kjører vi inn denne og potensielt ender opp med å nedgradere til next14 igjen, så må vi revertere denne og pga useActionState, men det er mye bra her som likevel kan inn uavhengig av next15 eller 14 - som at sjekker er flyttet inn til server actions. Smuudeste hadd vært om disse to var i ulike PRer slik at man kan evt beholde den ene hvis man skal reverte useActionState

@SelmaBergstrand SelmaBergstrand merged commit a64f50d into main Dec 4, 2024
3 checks passed
@SelmaBergstrand SelmaBergstrand deleted the errorhandling branch December 4, 2024 09:37
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.

3 participants