Modal: Fiks lukking ved slipp av museknapp på backdrop

[cskrov]

Description

Forslag til fiks for #2751.

Change summary

• Legger til sjekk på hvor click-eventen startet.

[changeset-bot]

🦋 Changeset detected

Latest commit: 412a276

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 7 packages

Name	Type
@navikt/ds-react	Patch
@navikt/ds-css	Patch
@navikt/ds-tokens	Patch
@navikt/ds-tailwind	Patch
@navikt/aksel-icons	Patch
@navikt/aksel	Patch
@navikt/aksel-stylelint	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[github-actions]

Storybook demo

cdc4b6855 | 57 komponenter | 221 stories

[HalvorHaugan]

Takk for bugrapport og løsningsforslag 🤩

Jeg tok meg friheten til å bygge videre på forslaget ditt, og gjorde følgende justeringer:

• La til sjekk av event.clientX/Y slik at vi også unngår det motsatte (klikker utenfor og slipper inni).
• Flyttet onBeforeClose() helt til slutt, slik at den ikke kalles unødvendig.
• Fjernet useCallback, da jeg ikke tror det gir noen fordeler i dette tilfellet.

[cskrov]

Bare hyggelig :D

La til sjekk av event.clientX/Y slik at vi også unngår det motsatte (klikker utenfor og slipper inni).

Jeg lot være å implementere utenfra-og-inn tilfellet, siden jeg ikke kunne være sikker på bruker sin intensjon.
Personlig er jeg enig i endringen din :)

Flyttet onBeforeClose() helt til slutt, slik at den ikke kalles unødvendig.

Grunnen til at jeg ikke flyttet denne var at getBoundingClientRect er relativt dyrt å kjøre. Mest sannsynlig betydelig dyrere enn onBeforeClose.

[cskrov]

Jeg pushet en endring nå som bruker clientX/Y i stedet for pageX/Y når startposisjonen settes.
Det er mer korrekt å sammenligne med returverdien fra getBoundingClientRect.

[cskrov]

Pushet en optimalisering av handleModalClick.

• Kjører ikke getBoundingClientRect før den absolutt må kjøres.
• Oppretter ikke const end = { x: event.clientX, y: event.clientY }; uten behov.
• Unngår ett nivå med nøsting.

Tenker det er lurt å ikke kjøre getBoundingClientRect unødig, men om dette strider med et prinsipp om at onBeforeClose aldri skal kjøres med mindre den "har siste ord", så endrer jeg gjerne rekkefølgen igjen :)

[HalvorHaugan]

onBeforeClose brukes gjerne for å spørre brukeren om bekreftelse før modalen lukkes, så den "har siste ord", ja 😄 Du kan teste dette i Storybook i den første storyen "With useRef" ved å klikke på "Secondary"-knappen i den første modalen. Da får du opp en ny modal som implementerer dette.

[cskrov]

Skjønner, takk for forklaring, det gir mening :)
Har flyttet onBeforeClose til å være siste sjekk i handleModalClick.

[cskrov]

Ønsker dere at PRer blir redusert til få eller én commit før de merges, @HalvorHaugan?

[HalvorHaugan]

Ønsker dere at PRer blir redusert til få eller én commit før de merges, @HalvorHaugan?

Vi pleier å squashe til én. Tror ikke det er mulig å velge noe annet her uansett (?)

[cskrov]

Ønsker dere at PRer blir redusert til få eller én commit før de merges, @HalvorHaugan?

Vi pleier å squashe til én. Tror ikke det er mulig å velge noe annet her uansett (?)

Ok, da er historikken ryddet opp til å være én commit i PRen.

Har fortsatt den fulle historikken lokalt det ikke var dette du mente.

Er det noe mer utestående før vi merger denne?

[HalvorHaugan]

Ok, da er historikken ryddet opp til å være én commit i PRen.

Har fortsatt den fulle historikken lokalt det ikke var dette du mente.

Det var ikke nødvendig, da det skjer automatisk ved merge, men det gjør ikke noe 😊

Er det noe mer utestående før vi merger denne?

Skal bare få noen andre på teamet til å se igjennom og godkjenne 👍

[it-vegard]

👍 🎉

[KenAJoh]

👏👏👏

[cskrov]

Jeg har ikke rettigheter til å merge denne selv. Trykker en av dere på knappen? 🙂