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

[Datepicker/Modal] Datepicker bruker nå Modal på mobil og for nested Modal #2419

Merged
merged 51 commits into from
Nov 1, 2023

Conversation

KenAJoh
Copy link
Collaborator

@KenAJoh KenAJoh commented Oct 25, 2023

Change summary

  • Datepicker/Monthpicker bytter nå til Modal på max-width 780px
  • Bruk av Datepicker/Monthpicker i Modal åpnes nå i egen Modal automatisk, ikke Popover
  • Trukket kode for Popover/Modal-switch ut i egen DateWrapper-komponent (for internt bruk)
  • Fjernet openOnFocus-prop da man bare kan åpne Popover/Modal ved klikk på knapp
  • Bruker nå native onClose fra Popover/Modal for lukking ved outside-click eller ESC. Fjernet da useEscape og useOutsideClick
  • Flyttet forwarded-ref til datepicker-wrapper fra Popover
  • La til useMedia-hook som lytter på mediaqueries (for å bytte mellom Popover og Modal on-the-fly)
  • Fjernet Eksempler for Datepicker i Modal da vi ikke vil oppmuntre til det

Fjernet også bubbleEscape-prop fra Popover og Datepicker da denne ikke trengs lengre. Ser ut som ingen bruker den https://github.com/search?type=code&q=org%3Anavikt+bubbleEscape, så trenger ikke major

Testing

JSDOM (jest) støtter ikke matchMedia. Det finnes polyfills for dette, men ser caniuse-støtten er høyere en de fleste andre features:
Screenshot 2023-10-25 at 14 59 04

Tenker da det er unødvendig med polyfill, å returnerer heller en default-value i useMedia.tsx og kjører ikke matchMedia.

export const noMatchMedia =
  typeof window !== "undefined" &&
  (window.matchMedia === undefined || navigator.userAgent.includes("jsdom"));

Testene fungerer da i jest + brukere trenger ikke lage egen mock for matchMedia og fallbacker til popover som var brukt tidligere. Trenger da heller ikke være redd for at noen av testene til brukere feiler på minor-update

@changeset-bot
Copy link

changeset-bot bot commented Oct 25, 2023

🦋 Changeset detected

Latest commit: 20d8032

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 Minor
@navikt/ds-css Minor
@navikt/aksel-stylelint Minor
@navikt/aksel Minor
@navikt/ds-tokens Minor
@navikt/ds-tailwind Minor
@navikt/aksel-icons Minor

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
Copy link
Contributor

github-actions bot commented Oct 25, 2023

Storybook demo

Endringer til review: 16

480091c5b | 59 komponenter | 389 stories

@navikt/core/react/src/date/hooks/useDatepicker.tsx Outdated Show resolved Hide resolved
@navikt/core/react/src/date/hooks/useMonthPicker.tsx Outdated Show resolved Hide resolved
@navikt/core/react/src/date/parts/DateWrapper.tsx Outdated Show resolved Hide resolved
@navikt/core/react/src/popover/Popover.tsx Show resolved Hide resolved
@navikt/core/react/src/util/useMedia.ts Outdated Show resolved Hide resolved
@navikt/core/react/src/util/useMedia.ts Outdated Show resolved Hide resolved
@navikt/core/react/src/util/useMedia.ts Outdated Show resolved Hide resolved
@navikt/core/react/src/util/useMedia.ts Outdated Show resolved Hide resolved
@KenAJoh KenAJoh requested a review from HalvorHaugan October 31, 2023 16:01
HalvorHaugan
HalvorHaugan previously approved these changes Nov 1, 2023
@KenAJoh KenAJoh merged commit 51eee7f into main Nov 1, 2023
3 checks passed
@KenAJoh KenAJoh deleted the dynamic-datepicker-modal branch November 1, 2023 14:38
@github-actions github-actions bot mentioned this pull request Nov 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants