From e2e20a3085dae8fbf3ab8522a1c3e8c834258ad2 Mon Sep 17 00:00:00 2001 From: Liam Smith Date: Mon, 29 Jul 2024 21:23:13 +0100 Subject: [PATCH 1/6] Fix dialog background scroll --- .../__tests__/__e2e__/dialog/Dialog.cy.tsx | 14 +++++++++++++ packages/core/src/dialog/Dialog.tsx | 21 +++++++++++++++---- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/packages/core/src/__tests__/__e2e__/dialog/Dialog.cy.tsx b/packages/core/src/__tests__/__e2e__/dialog/Dialog.cy.tsx index fb5c71d98fb..c77fa015cdc 100644 --- a/packages/core/src/__tests__/__e2e__/dialog/Dialog.cy.tsx +++ b/packages/core/src/__tests__/__e2e__/dialog/Dialog.cy.tsx @@ -183,5 +183,19 @@ describe("GIVEN a Dialog", () => { cy.findByRole("dialog").should("be.visible"); cy.findByRole("button", { name: "Next" }).should("be.focused"); }); + + it("THEN should set the root overflow to hidden", () => { + cy.mount(); + cy.document().its("documentElement.style.overflow").should("equal", ""); + + cy.findByRole("button", { name: "Open dialog" }).realClick(); + cy.findByRole("dialog").should("be.visible"); + cy.document() + .its("documentElement.style.overflow") + .should("equal", "hidden"); + + cy.findByRole("button", { name: "Cancel" }).realClick(); + cy.document().its("documentElement.style.overflow").should("equal", ""); + }); }); }); diff --git a/packages/core/src/dialog/Dialog.tsx b/packages/core/src/dialog/Dialog.tsx index c00a6416277..c270c8d22b8 100644 --- a/packages/core/src/dialog/Dialog.tsx +++ b/packages/core/src/dialog/Dialog.tsx @@ -14,6 +14,7 @@ import { forwardRef, useEffect, useMemo, + useRef, useState, } from "react"; import { Scrim } from "../scrim"; @@ -122,18 +123,30 @@ export const Dialog = forwardRef( const floatingRef = useForkRef(floating, ref); + const timeoutId = useRef(null); + useEffect(() => { if (open && !showComponent) { setShowComponent(true); + document.documentElement.style.overflow = "hidden"; } if (!open && showComponent) { - const animate = setTimeout(() => { + timeoutId.current = setTimeout(() => { setShowComponent(false); - }, 300); // var(--salt-duration-perceptible) - return () => clearTimeout(animate); + document.documentElement.style.removeProperty("overflow"); + }, 300); } - }, [open, showComponent, setShowComponent]); + }, [open, showComponent]); + + useEffect(() => { + return () => { + if (timeoutId.current) { + clearTimeout(timeoutId.current); + } + document.documentElement.style.removeProperty("overflow"); + }; + }, []); const contextValue = useMemo(() => ({ status, id }), [status, id]); From a821139b55a3ba6e0b93461e3a9e066ff11b9aff Mon Sep 17 00:00:00 2001 From: Liam Smith Date: Mon, 21 Oct 2024 12:48:27 +0100 Subject: [PATCH 2/6] Update changeset --- .changeset/three-years-visit.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/three-years-visit.md diff --git a/.changeset/three-years-visit.md b/.changeset/three-years-visit.md new file mode 100644 index 00000000000..3989005e208 --- /dev/null +++ b/.changeset/three-years-visit.md @@ -0,0 +1,5 @@ +--- +"@salt-ds/core": patch +--- + +Disable background scrolling while Dialog is open. From e95bf598ac1650e0eecff2f93563bacaa712fd7c Mon Sep 17 00:00:00 2001 From: Liam Smith Date: Sat, 9 Nov 2024 15:41:08 +0000 Subject: [PATCH 3/6] Add disableScroll prop --- .../__tests__/__e2e__/dialog/Dialog.cy.tsx | 14 ---------- .../__e2e__/utils/useFloatingUI.cy.tsx | 27 +++++++++++++++++++ packages/core/src/dialog/Dialog.tsx | 22 ++++----------- .../src/utils/useFloatingUI/useFloatingUI.tsx | 20 ++++++++++++++ 4 files changed, 52 insertions(+), 31 deletions(-) diff --git a/packages/core/src/__tests__/__e2e__/dialog/Dialog.cy.tsx b/packages/core/src/__tests__/__e2e__/dialog/Dialog.cy.tsx index 153aafca5ed..ebdf97be155 100644 --- a/packages/core/src/__tests__/__e2e__/dialog/Dialog.cy.tsx +++ b/packages/core/src/__tests__/__e2e__/dialog/Dialog.cy.tsx @@ -183,20 +183,6 @@ describe("GIVEN a Dialog", () => { cy.findByRole("dialog").should("be.visible"); cy.findByRole("button", { name: "Next" }).should("be.focused"); }); - - it("THEN should set the root overflow to hidden", () => { - cy.mount(); - cy.document().its("documentElement.style.overflow").should("equal", ""); - - cy.findByRole("button", { name: "Open dialog" }).realClick(); - cy.findByRole("dialog").should("be.visible"); - cy.document() - .its("documentElement.style.overflow") - .should("equal", "hidden"); - - cy.findByRole("button", { name: "Cancel" }).realClick(); - cy.document().its("documentElement.style.overflow").should("equal", ""); - }); }); describe("WHEN overflowing content is detected", () => { it("THEN it should add padding to the right of the scroll bar", () => { diff --git a/packages/core/src/__tests__/__e2e__/utils/useFloatingUI.cy.tsx b/packages/core/src/__tests__/__e2e__/utils/useFloatingUI.cy.tsx index e2d89260d73..0897a2684de 100644 --- a/packages/core/src/__tests__/__e2e__/utils/useFloatingUI.cy.tsx +++ b/packages/core/src/__tests__/__e2e__/utils/useFloatingUI.cy.tsx @@ -11,11 +11,13 @@ const TestComponent = ({ contentId = "test-1-content", focusManager, open = true, + disableScroll = false, }: { id?: string; contentId?: string; focusManager?: boolean; open?: boolean; + disableScroll?: boolean; }) => { const { Component: FloatingComponent } = useFloatingComponent(); const { context } = useFloatingUI({ @@ -26,6 +28,7 @@ const TestComponent = ({
@@ -84,4 +87,28 @@ describe("Use useFloatingComponent", () => { ); }); }); + describe("without disableScroll", () => { + it("the document body should not have hidden overflow", () => { + mount( + + + , + ); + + cy.document().its("documentElement.style.overflow").should("equal", ""); + }); + }); + describe("with disableScroll", () => { + it("the document body should have hidden overflow", () => { + mount( + + + , + ); + + cy.document() + .its("documentElement.style.overflow") + .should("equal", "hidden"); + }); + }); }); diff --git a/packages/core/src/dialog/Dialog.tsx b/packages/core/src/dialog/Dialog.tsx index 6695725006d..72b3a1e2101 100644 --- a/packages/core/src/dialog/Dialog.tsx +++ b/packages/core/src/dialog/Dialog.tsx @@ -14,7 +14,6 @@ import { forwardRef, useEffect, useMemo, - useRef, useState, } from "react"; import { Scrim } from "../scrim"; @@ -123,30 +122,18 @@ export const Dialog = forwardRef( const floatingRef = useForkRef(floating, ref); - const timeoutId = useRef(null); - useEffect(() => { if (open && !showComponent) { setShowComponent(true); - document.documentElement.style.overflow = "hidden"; } if (!open && showComponent) { - timeoutId.current = setTimeout(() => { + const animate = setTimeout(() => { setShowComponent(false); - document.documentElement.style.removeProperty("overflow"); - }, 300); + }, 300); // var(--salt-duration-perceptible) + return () => clearTimeout(animate); } - }, [open, showComponent]); - - useEffect(() => { - return () => { - if (timeoutId.current) { - clearTimeout(timeoutId.current); - } - document.documentElement.style.removeProperty("overflow"); - }; - }, []); + }, [open, showComponent, setShowComponent]); const contextValue = useMemo(() => ({ status, id }), [status, id]); @@ -161,6 +148,7 @@ export const Dialog = forwardRef( ref={floatingRef} width={elements.floating?.offsetWidth} height={elements.floating?.offsetHeight} + disableScroll={true} focusManagerProps={{ context: context, initialFocus, diff --git a/packages/core/src/utils/useFloatingUI/useFloatingUI.tsx b/packages/core/src/utils/useFloatingUI/useFloatingUI.tsx index 1f6a45e358d..598c7204e10 100644 --- a/packages/core/src/utils/useFloatingUI/useFloatingUI.tsx +++ b/packages/core/src/utils/useFloatingUI/useFloatingUI.tsx @@ -19,6 +19,7 @@ import { createContext, forwardRef, useContext, + useEffect, useMemo, } from "react"; import { SaltProvider, SaltProviderNext, useTheme } from "../../salt-provider"; @@ -45,6 +46,10 @@ export interface FloatingComponentProps width?: number; height?: number; position?: Strategy; + /** + * Disables scrolling the document body while the floating component is open. + */ + disableScroll?: boolean; } const DefaultFloatingComponent = forwardRef< @@ -59,6 +64,7 @@ const DefaultFloatingComponent = forwardRef< width, height, focusManagerProps, + disableScroll, ...rest } = props; const style = { @@ -71,6 +77,20 @@ const DefaultFloatingComponent = forwardRef< const ChosenSaltProvider = themeNext ? SaltProviderNext : SaltProvider; + useEffect(() => { + if (!disableScroll) { + return; + } + if (open) { + document.documentElement.style.overflow = "hidden"; + } else { + document.documentElement.style.removeProperty("overflow"); + } + return () => { + document.documentElement.style.removeProperty("overflow"); + }; + }, [disableScroll, open]); + if (focusManagerProps && open) { return ( From d85190b55a0c99cc71b991f970a35373e579e8ff Mon Sep 17 00:00:00 2001 From: Liam Smith Date: Sat, 9 Nov 2024 15:47:58 +0000 Subject: [PATCH 4/6] Update changeset --- .changeset/three-years-visit.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.changeset/three-years-visit.md b/.changeset/three-years-visit.md index 3989005e208..339c64824ee 100644 --- a/.changeset/three-years-visit.md +++ b/.changeset/three-years-visit.md @@ -1,5 +1,5 @@ --- -"@salt-ds/core": patch +"@salt-ds/core": minor --- -Disable background scrolling while Dialog is open. +Added `disableScroll` prop to FloatingComponent From 8c0da910757e27425e23d7f8ac30958d7c001bfe Mon Sep 17 00:00:00 2001 From: Liam Smith Date: Sun, 10 Nov 2024 21:01:02 +0000 Subject: [PATCH 5/6] Fix lint --- .changeset/three-years-visit.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/three-years-visit.md b/.changeset/three-years-visit.md index 339c64824ee..909567a3cf9 100644 --- a/.changeset/three-years-visit.md +++ b/.changeset/three-years-visit.md @@ -2,4 +2,4 @@ "@salt-ds/core": minor --- -Added `disableScroll` prop to FloatingComponent +Added `disableScroll` prop to FloatingComponent. From e447d779862c06161ae1c51a967ea17db5b8be51 Mon Sep 17 00:00:00 2001 From: Liam Smith Date: Mon, 11 Nov 2024 21:22:08 +0000 Subject: [PATCH 6/6] Refactor to use targetWindow --- .../core/src/utils/useFloatingUI/useFloatingUI.tsx | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/core/src/utils/useFloatingUI/useFloatingUI.tsx b/packages/core/src/utils/useFloatingUI/useFloatingUI.tsx index 598c7204e10..ce7eb7e4244 100644 --- a/packages/core/src/utils/useFloatingUI/useFloatingUI.tsx +++ b/packages/core/src/utils/useFloatingUI/useFloatingUI.tsx @@ -13,6 +13,7 @@ import { shift, useFloating, } from "@floating-ui/react"; +import { useWindow } from "@salt-ds/window"; import { type ComponentPropsWithoutRef, type ReactNode, @@ -75,21 +76,23 @@ const DefaultFloatingComponent = forwardRef< const { themeNext } = useTheme(); + const targetWindow = useWindow(); + const ChosenSaltProvider = themeNext ? SaltProviderNext : SaltProvider; useEffect(() => { - if (!disableScroll) { + if (!disableScroll || !targetWindow) { return; } if (open) { - document.documentElement.style.overflow = "hidden"; + targetWindow.document.documentElement.style.overflow = "hidden"; } else { - document.documentElement.style.removeProperty("overflow"); + targetWindow.document.documentElement.style.removeProperty("overflow"); } return () => { - document.documentElement.style.removeProperty("overflow"); + targetWindow.document.documentElement.style.removeProperty("overflow"); }; - }, [disableScroll, open]); + }, [disableScroll, open, targetWindow]); if (focusManagerProps && open) { return (