From 4d26a87e3de12f31d114a023fc8da456561ac4f8 Mon Sep 17 00:00:00 2001 From: Julian Nymark Date: Tue, 20 Feb 2024 14:48:38 +0100 Subject: [PATCH 01/12] :label: fix types for modal issue --- @navikt/core/react/src/modal/types.ts | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/@navikt/core/react/src/modal/types.ts b/@navikt/core/react/src/modal/types.ts index 2053d35c9a6..ed2eaa7d0e3 100644 --- a/@navikt/core/react/src/modal/types.ts +++ b/@navikt/core/react/src/modal/types.ts @@ -81,11 +81,27 @@ interface ModalPropsBase extends React.DialogHTMLAttributes { // Require onClose if you use open & Require either header, aria-labelledby or aria-label export type ModalProps = ModalPropsBase & ( - | { open?: never } | { open: boolean | undefined; onClose: ModalPropsBase["onClose"] } + | { onClose?: ModalPropsBase["onClose"]; open?: never } ) & ( - | { header: ModalPropsBase["header"] } + | ( + | { + header: ModalPropsBase["header"]; + "aria-labelledby": string; + "aria-label"?: never; + } + | { + header: ModalPropsBase["header"]; + "aria-label": string; + "aria-labelledby"?: never; + } + | { + header: ModalPropsBase["header"]; + "aria-labelledby"?: never; + "aria-label"?: never; + } + ) | { "aria-labelledby": string; "aria-label"?: never } | { "aria-labelledby"?: never; "aria-label": string } ); From 259a3aec8fa7e046ca3561c6de332092a082402c Mon Sep 17 00:00:00 2001 From: Halvor Haugan Date: Tue, 20 Feb 2024 15:51:57 +0100 Subject: [PATCH 02/12] liten style justering --- @navikt/core/react/src/modal/types.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/@navikt/core/react/src/modal/types.ts b/@navikt/core/react/src/modal/types.ts index ed2eaa7d0e3..296901da6eb 100644 --- a/@navikt/core/react/src/modal/types.ts +++ b/@navikt/core/react/src/modal/types.ts @@ -81,7 +81,7 @@ interface ModalPropsBase extends React.DialogHTMLAttributes { // Require onClose if you use open & Require either header, aria-labelledby or aria-label export type ModalProps = ModalPropsBase & ( - | { open: boolean | undefined; onClose: ModalPropsBase["onClose"] } + | { onClose: ModalPropsBase["onClose"]; open: boolean | undefined } | { onClose?: ModalPropsBase["onClose"]; open?: never } ) & ( From b091b4d9dbdef382b9a90ab8f593bec809a3b2b1 Mon Sep 17 00:00:00 2001 From: Halvor Haugan Date: Tue, 20 Feb 2024 15:52:15 +0100 Subject: [PATCH 03/12] Tester! --- .../core/react/src/modal/modal.stories.tsx | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/@navikt/core/react/src/modal/modal.stories.tsx b/@navikt/core/react/src/modal/modal.stories.tsx index 6555d868907..50ba931b56f 100644 --- a/@navikt/core/react/src/modal/modal.stories.tsx +++ b/@navikt/core/react/src/modal/modal.stories.tsx @@ -339,3 +339,63 @@ ChromaticViewportTesting.parameters = { }, }, }; + +// eslint-disable-next-line @typescript-eslint/no-unused-vars +const PropTypeTest = () => { + return ( + <> + OK + + + OK + + + + OK + + + OK + + OK + + null}> + OK + + + {/* @ts-expect-error - Mangler label */} + Mangler label + + {/* @ts-expect-error - Mangler onClose eller label */} + Mangler onClose eller label + + {/* @ts-expect-error - Mangler onClose */} + + Mangler onClose + + + {/* @ts-expect-error - Mangler label */} + null}> + Mangler label + + + {/* @ts-expect-error - Mangler onClose */} + + Mangler onClose + + + {/* @ts-expect-error - For mange labels */} + + For mange labels + + + {/* @ts-expect-error - For mange labels */} + + For mange labels + + + ); +}; From cee68323aa0bfdcbd7e50550a8c1e9378e2c5397 Mon Sep 17 00:00:00 2001 From: Halvor Haugan Date: Tue, 20 Feb 2024 15:54:34 +0100 Subject: [PATCH 04/12] changeset --- .changeset/kind-flies-cover.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/kind-flies-cover.md diff --git a/.changeset/kind-flies-cover.md b/.changeset/kind-flies-cover.md new file mode 100644 index 00000000000..fbe1ec39c4e --- /dev/null +++ b/.changeset/kind-flies-cover.md @@ -0,0 +1,5 @@ +--- +"@navikt/ds-react": patch +--- + +Modal: Bedre feilmeldinger ved feil bruk av props From 573174e6f6751874974a5eb4f36a3c9b8fdf228f Mon Sep 17 00:00:00 2001 From: Halvor Haugan Date: Wed, 21 Feb 2024 09:58:16 +0100 Subject: [PATCH 05/12] tester test --- @navikt/core/react/src/modal/Modal.test.tsx | 11 +++++++++++ @navikt/core/react/src/modal/modal.stories.tsx | 1 - 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/@navikt/core/react/src/modal/Modal.test.tsx b/@navikt/core/react/src/modal/Modal.test.tsx index a352aaf8082..7bf8b534219 100644 --- a/@navikt/core/react/src/modal/Modal.test.tsx +++ b/@navikt/core/react/src/modal/Modal.test.tsx @@ -17,6 +17,17 @@ const Test = () => { ); }; +// eslint-disable-next-line @typescript-eslint/no-unused-vars +const Test2 = () => { + return ( + + +

Foobar

+
+
+ ); +}; + describe("Modal", () => { test("should be visible", () => { render(); diff --git a/@navikt/core/react/src/modal/modal.stories.tsx b/@navikt/core/react/src/modal/modal.stories.tsx index 50ba931b56f..6ffcd2d3cf1 100644 --- a/@navikt/core/react/src/modal/modal.stories.tsx +++ b/@navikt/core/react/src/modal/modal.stories.tsx @@ -362,7 +362,6 @@ const PropTypeTest = () => { OK - {/* @ts-expect-error - Mangler label */} Mangler label {/* @ts-expect-error - Mangler onClose eller label */} From 3be665f541147844c969cc23babd598085ea743d Mon Sep 17 00:00:00 2001 From: Halvor Haugan Date: Wed, 21 Feb 2024 10:10:15 +0100 Subject: [PATCH 06/12] tester test 2 --- @navikt/core/react/src/modal/Modal.test.tsx | 6 +++++- @navikt/core/react/src/modal/Modal.tsx | 7 +++++++ @navikt/core/react/src/modal/modal.stories.tsx | 3 +-- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/@navikt/core/react/src/modal/Modal.test.tsx b/@navikt/core/react/src/modal/Modal.test.tsx index 7bf8b534219..e8ac5067a70 100644 --- a/@navikt/core/react/src/modal/Modal.test.tsx +++ b/@navikt/core/react/src/modal/Modal.test.tsx @@ -17,7 +17,6 @@ const Test = () => { ); }; -// eslint-disable-next-line @typescript-eslint/no-unused-vars const Test2 = () => { return ( @@ -29,6 +28,11 @@ const Test2 = () => { }; describe("Modal", () => { + test("types work", () => { + render(); + expect(screen.getByText("Foobar")).toBeVisible(); + }); + test("should be visible", () => { render(); expect(screen.getByText("Foobar")).toBeVisible(); diff --git a/@navikt/core/react/src/modal/Modal.tsx b/@navikt/core/react/src/modal/Modal.tsx index 81b981b314b..8ab012d042a 100644 --- a/@navikt/core/react/src/modal/Modal.tsx +++ b/@navikt/core/react/src/modal/Modal.tsx @@ -188,6 +188,7 @@ export const Modal = forwardRef( {header && ( @@ -223,3 +224,9 @@ Modal.Body = ModalBody; Modal.Footer = ModalFooter; export default Modal; + +// TEST + +export const PropTypeTest = () => { + return Mangler onClose eller label; +}; diff --git a/@navikt/core/react/src/modal/modal.stories.tsx b/@navikt/core/react/src/modal/modal.stories.tsx index 6ffcd2d3cf1..d251a4fb2b4 100644 --- a/@navikt/core/react/src/modal/modal.stories.tsx +++ b/@navikt/core/react/src/modal/modal.stories.tsx @@ -340,8 +340,7 @@ ChromaticViewportTesting.parameters = { }, }; -// eslint-disable-next-line @typescript-eslint/no-unused-vars -const PropTypeTest = () => { +export const PropTypeTest = () => { return ( <> OK From f86f183c133860f7300243cfeddb7225c9725b1a Mon Sep 17 00:00:00 2001 From: Halvor Haugan Date: Wed, 21 Feb 2024 10:18:03 +0100 Subject: [PATCH 07/12] tester test 3 --- @navikt/core/react/src/modal/Modal.tsx | 7 ------- 1 file changed, 7 deletions(-) diff --git a/@navikt/core/react/src/modal/Modal.tsx b/@navikt/core/react/src/modal/Modal.tsx index 8ab012d042a..81b981b314b 100644 --- a/@navikt/core/react/src/modal/Modal.tsx +++ b/@navikt/core/react/src/modal/Modal.tsx @@ -188,7 +188,6 @@ export const Modal = forwardRef( {header && ( @@ -224,9 +223,3 @@ Modal.Body = ModalBody; Modal.Footer = ModalFooter; export default Modal; - -// TEST - -export const PropTypeTest = () => { - return Mangler onClose eller label; -}; From 67fa89733e3b233a511c8f03d08b56f41e5962d2 Mon Sep 17 00:00:00 2001 From: Halvor Haugan Date: Wed, 21 Feb 2024 11:57:17 +0100 Subject: [PATCH 08/12] Revert experiments, add proper tests --- @navikt/core/react/src/modal/Modal.test.tsx | 15 ---- .../core/react/src/modal/modal.stories.tsx | 58 -------------- @navikt/core/react/src/modal/types.test-d.ts | 78 +++++++++++++++++++ @navikt/core/react/tests/vitest.config.ts | 3 + 4 files changed, 81 insertions(+), 73 deletions(-) create mode 100644 @navikt/core/react/src/modal/types.test-d.ts diff --git a/@navikt/core/react/src/modal/Modal.test.tsx b/@navikt/core/react/src/modal/Modal.test.tsx index e8ac5067a70..a352aaf8082 100644 --- a/@navikt/core/react/src/modal/Modal.test.tsx +++ b/@navikt/core/react/src/modal/Modal.test.tsx @@ -17,22 +17,7 @@ const Test = () => { ); }; -const Test2 = () => { - return ( - - -

Foobar

-
-
- ); -}; - describe("Modal", () => { - test("types work", () => { - render(); - expect(screen.getByText("Foobar")).toBeVisible(); - }); - test("should be visible", () => { render(); expect(screen.getByText("Foobar")).toBeVisible(); diff --git a/@navikt/core/react/src/modal/modal.stories.tsx b/@navikt/core/react/src/modal/modal.stories.tsx index d251a4fb2b4..6555d868907 100644 --- a/@navikt/core/react/src/modal/modal.stories.tsx +++ b/@navikt/core/react/src/modal/modal.stories.tsx @@ -339,61 +339,3 @@ ChromaticViewportTesting.parameters = { }, }, }; - -export const PropTypeTest = () => { - return ( - <> - OK - - - OK - - - - OK - - - OK - - OK - - null}> - OK - - - Mangler label - - {/* @ts-expect-error - Mangler onClose eller label */} - Mangler onClose eller label - - {/* @ts-expect-error - Mangler onClose */} - - Mangler onClose - - - {/* @ts-expect-error - Mangler label */} - null}> - Mangler label - - - {/* @ts-expect-error - Mangler onClose */} - - Mangler onClose - - - {/* @ts-expect-error - For mange labels */} - - For mange labels - - - {/* @ts-expect-error - For mange labels */} - - For mange labels - - - ); -}; diff --git a/@navikt/core/react/src/modal/types.test-d.ts b/@navikt/core/react/src/modal/types.test-d.ts new file mode 100644 index 00000000000..290b0adaa0f --- /dev/null +++ b/@navikt/core/react/src/modal/types.test-d.ts @@ -0,0 +1,78 @@ +import { expectTypeOf } from "vitest"; +import { ModalProps } from "./types"; + +test("ModalProps works as intended", () => { + expectTypeOf({ + header: { heading: "Label" }, + children: "OK", + }).toMatchTypeOf(); + + expectTypeOf({ + header: { heading: "Label" }, + "aria-label": "Label", + children: "OK", + }).toMatchTypeOf(); + + expectTypeOf({ + header: { heading: "Label" }, + "aria-labelledby": "Label", + children: "OK", + }).toMatchTypeOf(); + + expectTypeOf({ + "aria-label": "Label", + children: "OK", + }).toMatchTypeOf(); + + expectTypeOf({ + "aria-labelledby": "Label", + children: "OK", + }).toMatchTypeOf(); + + expectTypeOf({ + "aria-label": "Label", + open: true, + onClose: () => null, + children: "OK", + }).toMatchTypeOf(); + + expectTypeOf({ + children: "Mangler label", + }).not.toMatchTypeOf(); + + expectTypeOf({ + open: true, + children: "Mangler onClose eller label", + }).not.toMatchTypeOf(); + + expectTypeOf({ + open: true, + "aria-label": "Label", + children: "Mangler onClose", + }).not.toMatchTypeOf(); + + expectTypeOf({ + open: true, + onClose: () => null, + children: "Mangler label", + }).not.toMatchTypeOf(); + + expectTypeOf({ + header: { heading: "Label" }, + open: true, + children: "Mangler onClose", + }).not.toMatchTypeOf(); + + expectTypeOf({ + header: { heading: "Label" }, + "aria-label": "Label", + "aria-labelledby": "Label", + children: "For mange labels", + }).not.toMatchTypeOf(); + + expectTypeOf({ + "aria-label": "Label", + "aria-labelledby": "Label", + children: "For mange labels", + }).not.toMatchTypeOf(); +}); diff --git a/@navikt/core/react/tests/vitest.config.ts b/@navikt/core/react/tests/vitest.config.ts index faae4e20b76..9c717e0f169 100644 --- a/@navikt/core/react/tests/vitest.config.ts +++ b/@navikt/core/react/tests/vitest.config.ts @@ -6,5 +6,8 @@ export default defineConfig({ globals: true, setupFiles: "./tests/setup.ts", environment: "jsdom", + typecheck: { + enabled: true, + }, }, }); From 54fe0692abb2c7c0456a9aead289d3dfed11c210 Mon Sep 17 00:00:00 2001 From: Halvor Haugan Date: Wed, 21 Feb 2024 12:11:32 +0100 Subject: [PATCH 09/12] fix Portal.stories.tsx --- .../core/react/src/overlays/portal/Portal.stories.tsx | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/@navikt/core/react/src/overlays/portal/Portal.stories.tsx b/@navikt/core/react/src/overlays/portal/Portal.stories.tsx index 2d5248b8d79..72442f8237b 100644 --- a/@navikt/core/react/src/overlays/portal/Portal.stories.tsx +++ b/@navikt/core/react/src/overlays/portal/Portal.stories.tsx @@ -12,7 +12,7 @@ export default { export const Default = () => { return ( - +

In regular DOM tree

Lorem ipsum dolor sit amet consectetur adipisicing elit. Temporibus @@ -65,7 +65,10 @@ export const CustomPortalRootFromProvider = () => {

This is mounted to Tree B, while created inside Tree A

- + el && setPortalContainer(el)} + >

Tree B

@@ -75,7 +78,7 @@ export const CustomPortalRootFromProvider = () => { export const AsChild = () => { return ( - +

In regular DOM tree

Lorem ipsum dolor sit amet consectetur adipisicing elit. Temporibus From 2c9707a5339448a08bf0c51095ac71fd9dc1101f Mon Sep 17 00:00:00 2001 From: Ken Date: Wed, 21 Feb 2024 12:51:59 +0100 Subject: [PATCH 10/12] =?UTF-8?q?:label:=20Endret=20rekkef=C3=B8lge=20p?= =?UTF-8?q?=C3=A5=20modal=20conditional-typer=20for=20bedre=20feilmelding?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- @navikt/core/react/src/modal/types.ts | 32 +++++++++++++-------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/@navikt/core/react/src/modal/types.ts b/@navikt/core/react/src/modal/types.ts index 296901da6eb..f16b8d5939d 100644 --- a/@navikt/core/react/src/modal/types.ts +++ b/@navikt/core/react/src/modal/types.ts @@ -85,23 +85,21 @@ export type ModalProps = ModalPropsBase & | { onClose?: ModalPropsBase["onClose"]; open?: never } ) & ( - | ( - | { - header: ModalPropsBase["header"]; - "aria-labelledby": string; - "aria-label"?: never; - } - | { - header: ModalPropsBase["header"]; - "aria-label": string; - "aria-labelledby"?: never; - } - | { - header: ModalPropsBase["header"]; - "aria-labelledby"?: never; - "aria-label"?: never; - } - ) | { "aria-labelledby": string; "aria-label"?: never } | { "aria-labelledby"?: never; "aria-label": string } + | { + header: ModalPropsBase["header"]; + "aria-labelledby": string; + "aria-label"?: never; + } + | { + header: ModalPropsBase["header"]; + "aria-label": string; + "aria-labelledby"?: never; + } + | { + header: ModalPropsBase["header"]; + "aria-labelledby"?: never; + "aria-label"?: never; + } ); From 0325bba0856182f894e0ffb231e4fca15abe15e5 Mon Sep 17 00:00:00 2001 From: Halvor Haugan Date: Wed, 21 Feb 2024 12:57:11 +0100 Subject: [PATCH 11/12] =?UTF-8?q?revert=20rekkef=C3=B8lge?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- @navikt/core/react/src/modal/types.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/@navikt/core/react/src/modal/types.ts b/@navikt/core/react/src/modal/types.ts index f16b8d5939d..c78881ad7ca 100644 --- a/@navikt/core/react/src/modal/types.ts +++ b/@navikt/core/react/src/modal/types.ts @@ -85,8 +85,6 @@ export type ModalProps = ModalPropsBase & | { onClose?: ModalPropsBase["onClose"]; open?: never } ) & ( - | { "aria-labelledby": string; "aria-label"?: never } - | { "aria-labelledby"?: never; "aria-label": string } | { header: ModalPropsBase["header"]; "aria-labelledby": string; @@ -102,4 +100,6 @@ export type ModalProps = ModalPropsBase & "aria-labelledby"?: never; "aria-label"?: never; } + | { "aria-labelledby": string; "aria-label"?: never } + | { "aria-labelledby"?: never; "aria-label": string } ); From 1e757f66996bff22a6a21250443993a37e11521f Mon Sep 17 00:00:00 2001 From: Halvor Haugan Date: Wed, 21 Feb 2024 12:59:50 +0100 Subject: [PATCH 12/12] testkode --- .../core/react/src/modal/modal.stories.tsx | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/@navikt/core/react/src/modal/modal.stories.tsx b/@navikt/core/react/src/modal/modal.stories.tsx index 6555d868907..dca30c6bc76 100644 --- a/@navikt/core/react/src/modal/modal.stories.tsx +++ b/@navikt/core/react/src/modal/modal.stories.tsx @@ -339,3 +339,57 @@ ChromaticViewportTesting.parameters = { }, }, }; + +// For testing TS error messages: + +/* const PropTypeTest = () => { + return ( + <> + OK + + + OK + + + + OK + + + OK + + OK + + null}> + OK + + + Mangler label + + Mangler onClose eller label + + + Mangler onClose + + + null}> + Mangler label + + + + Mangler onClose + + + + For mange labels + + + + For mange labels + + + ); +}; */