From 116a9140f3fb8d45ff6a5558ff012e592783d49f Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Wed, 18 Sep 2024 19:18:35 -0400 Subject: [PATCH 1/8] re-add shadow DOM styles when reparented --- .../src/utils/components/ShadowRoot.tsx | 42 +++++++++++++------ 1 file changed, 29 insertions(+), 13 deletions(-) diff --git a/packages/itwinui-react/src/utils/components/ShadowRoot.tsx b/packages/itwinui-react/src/utils/components/ShadowRoot.tsx index 22de2adf9e8..a11b8b3027e 100644 --- a/packages/itwinui-react/src/utils/components/ShadowRoot.tsx +++ b/packages/itwinui-react/src/utils/components/ShadowRoot.tsx @@ -81,6 +81,24 @@ function useShadowRoot( const latestCss = useLatestRef(css); const latestShadowRoot = useLatestRef(shadowRoot); + const createStyleSheet = React.useCallback(() => { + const shadow = latestShadowRoot.current; + + if (shadow && supportsAdoptedStylesheets) { + // create an empty stylesheet and add it to the shadowRoot + const currentWindow = shadow.ownerDocument.defaultView || globalThis; + if (!(styleSheet.current instanceof currentWindow.CSSStyleSheet)) { + styleSheet.current = new currentWindow.CSSStyleSheet(); + shadow.adoptedStyleSheets.push(styleSheet.current); + } + + // add the CSS immediately to avoid FOUC (one-time) + if (latestCss.current) { + styleSheet.current.replaceSync(latestCss.current); + } + } + }, [latestCss, latestShadowRoot]); + useLayoutEffect(() => { const parent = templateRef.current?.parentElement; if (!parent) { @@ -94,23 +112,13 @@ function useShadowRoot( const shadow = parent.shadowRoot || parent.attachShadow({ mode: 'open' }); - if (supportsAdoptedStylesheets) { - // create an empty stylesheet and add it to the shadowRoot - const currentWindow = shadow.ownerDocument.defaultView || globalThis; - styleSheet.current = new currentWindow.CSSStyleSheet(); - shadow.adoptedStyleSheets = [styleSheet.current]; - - // add the CSS immediately to avoid FOUC (one-time) - if (latestCss.current) { - styleSheet.current.replaceSync(latestCss.current); - } - } - // Flush the state immediately after shadow-root is attached, to ensure that layout // measurements in parent component are correct. // Without this, the parent component may end up measuring the layout when the shadow-root // is attached in the DOM but React hasn't rendered any slots or content into it yet. ReactDOM.flushSync(() => setShadowRoot(shadow)); + + createStyleSheet(); }; queueMicrotask(() => { @@ -118,7 +126,7 @@ function useShadowRoot( }); return () => void setShadowRoot(null); - }, [templateRef, latestCss, latestShadowRoot]); + }, [templateRef, createStyleSheet, latestShadowRoot]); // Synchronize `css` with contents of the existing stylesheet useLayoutEffect(() => { @@ -127,5 +135,13 @@ function useShadowRoot( } }, [css]); + // Re-create stylesheet if the element is moved to a different window (by AppUI) + React.useEffect(() => { + window.addEventListener('appui:reparent', createStyleSheet); + return () => { + window.removeEventListener('appui:reparent', createStyleSheet); + }; + }, [createStyleSheet, latestShadowRoot]); + return shadowRoot; } From b5ee1350713faad8fe0a0556f5a7d703b69c9a94 Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Thu, 19 Sep 2024 11:55:33 -0400 Subject: [PATCH 2/8] add test for popout --- .../e2e/app/routes/VisuallyHidden/route.tsx | 45 +++++++++++++++++++ testing/e2e/app/routes/VisuallyHidden/spec.ts | 11 +++++ 2 files changed, 56 insertions(+) create mode 100644 testing/e2e/app/routes/VisuallyHidden/route.tsx create mode 100644 testing/e2e/app/routes/VisuallyHidden/spec.ts diff --git a/testing/e2e/app/routes/VisuallyHidden/route.tsx b/testing/e2e/app/routes/VisuallyHidden/route.tsx new file mode 100644 index 00000000000..ed6f73f690a --- /dev/null +++ b/testing/e2e/app/routes/VisuallyHidden/route.tsx @@ -0,0 +1,45 @@ +import * as React from 'react'; +import * as ReactDOM from 'react-dom'; +import { VisuallyHidden, ThemeProvider } from '@itwin/itwinui-react'; +import { useSearchParams } from '@remix-run/react'; + +export default function Page() { + const [searchParams] = useSearchParams(); + + if (searchParams.get('popout') === 'true') { + return ; + } + + return null; +} + +/** https://github.com/iTwin/iTwinUI/pull/2252#discussion_r1766676900 */ +function PopoutTest() { + const popout = usePopout(); + + return ( + <> + + {popout.popout && + ReactDOM.createPortal( + + Hello + , + popout.popout.document.body, + )} + + ); +} + +// ---------------------------------------------------------------------------- + +function usePopout() { + const [popout, setPopout] = React.useState(null); + + const open = React.useCallback(() => { + const popout = window.open('', 'popout', 'width=400,height=400'); + setPopout(popout); + }, []); + + return React.useMemo(() => ({ open, popout }), [open, popout]); +} diff --git a/testing/e2e/app/routes/VisuallyHidden/spec.ts b/testing/e2e/app/routes/VisuallyHidden/spec.ts new file mode 100644 index 00000000000..1b075ae3b85 --- /dev/null +++ b/testing/e2e/app/routes/VisuallyHidden/spec.ts @@ -0,0 +1,11 @@ +import { test, expect } from '@playwright/test'; + +test('styles should exist in popout window', async ({ page }) => { + const popoutPromise = page.waitForEvent('popup'); + await page.goto('/VisuallyHidden?popout=true'); + await page.click('button'); + const popout = await popoutPromise; + + const visuallyHidden = popout.getByText('Hello'); + await expect(visuallyHidden).toHaveCSS('position', 'absolute'); +}); From ef0fb8de19ddd6a58bcd83f556b710396e0a1f68 Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Thu, 19 Sep 2024 11:55:51 -0400 Subject: [PATCH 3/8] fix main reparent issue --- .../src/utils/components/ShadowRoot.tsx | 41 ++++++++++--------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/packages/itwinui-react/src/utils/components/ShadowRoot.tsx b/packages/itwinui-react/src/utils/components/ShadowRoot.tsx index a11b8b3027e..7f3ce58d05a 100644 --- a/packages/itwinui-react/src/utils/components/ShadowRoot.tsx +++ b/packages/itwinui-react/src/utils/components/ShadowRoot.tsx @@ -81,23 +81,24 @@ function useShadowRoot( const latestCss = useLatestRef(css); const latestShadowRoot = useLatestRef(shadowRoot); - const createStyleSheet = React.useCallback(() => { - const shadow = latestShadowRoot.current; - - if (shadow && supportsAdoptedStylesheets) { - // create an empty stylesheet and add it to the shadowRoot - const currentWindow = shadow.ownerDocument.defaultView || globalThis; - if (!(styleSheet.current instanceof currentWindow.CSSStyleSheet)) { - styleSheet.current = new currentWindow.CSSStyleSheet(); - shadow.adoptedStyleSheets.push(styleSheet.current); + const createStyleSheet = React.useCallback( + (shadow: ShadowRoot | null) => { + if (shadow && supportsAdoptedStylesheets) { + // create an empty stylesheet and add it to the shadowRoot + const currentWindow = shadow.ownerDocument.defaultView || globalThis; + if (!(styleSheet.current instanceof currentWindow.CSSStyleSheet)) { + styleSheet.current = new currentWindow.CSSStyleSheet(); + shadow.adoptedStyleSheets.push(styleSheet.current); + } + + // add the CSS immediately to avoid FOUC (one-time) + if (latestCss.current) { + styleSheet.current.replaceSync(latestCss.current); + } } - - // add the CSS immediately to avoid FOUC (one-time) - if (latestCss.current) { - styleSheet.current.replaceSync(latestCss.current); - } - } - }, [latestCss, latestShadowRoot]); + }, + [latestCss], + ); useLayoutEffect(() => { const parent = templateRef.current?.parentElement; @@ -118,7 +119,7 @@ function useShadowRoot( // is attached in the DOM but React hasn't rendered any slots or content into it yet. ReactDOM.flushSync(() => setShadowRoot(shadow)); - createStyleSheet(); + createStyleSheet(shadow); }; queueMicrotask(() => { @@ -137,9 +138,11 @@ function useShadowRoot( // Re-create stylesheet if the element is moved to a different window (by AppUI) React.useEffect(() => { - window.addEventListener('appui:reparent', createStyleSheet); + const listener = () => createStyleSheet(latestShadowRoot.current); + + window.addEventListener('appui:reparent', listener); return () => { - window.removeEventListener('appui:reparent', createStyleSheet); + window.removeEventListener('appui:reparent', listener); }; }, [createStyleSheet, latestShadowRoot]); From 2b5e2fde04d7130455c49cc61f5560ec512d97d2 Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Thu, 19 Sep 2024 12:06:46 -0400 Subject: [PATCH 4/8] use `render` instead of `portal` to test first mount --- testing/e2e/app/routes/VisuallyHidden/route.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/e2e/app/routes/VisuallyHidden/route.tsx b/testing/e2e/app/routes/VisuallyHidden/route.tsx index ed6f73f690a..78b748d0c30 100644 --- a/testing/e2e/app/routes/VisuallyHidden/route.tsx +++ b/testing/e2e/app/routes/VisuallyHidden/route.tsx @@ -21,7 +21,7 @@ function PopoutTest() { <> {popout.popout && - ReactDOM.createPortal( + ReactDOM.render( Hello , From 7c1ea7700589eece8a7108e04e07abe7bf31bd3e Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Thu, 19 Sep 2024 12:30:24 -0400 Subject: [PATCH 5/8] fix one-time CSS addition --- .../src/utils/components/ShadowRoot.tsx | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/itwinui-react/src/utils/components/ShadowRoot.tsx b/packages/itwinui-react/src/utils/components/ShadowRoot.tsx index 7f3ce58d05a..b5321984177 100644 --- a/packages/itwinui-react/src/utils/components/ShadowRoot.tsx +++ b/packages/itwinui-react/src/utils/components/ShadowRoot.tsx @@ -84,13 +84,17 @@ function useShadowRoot( const createStyleSheet = React.useCallback( (shadow: ShadowRoot | null) => { if (shadow && supportsAdoptedStylesheets) { - // create an empty stylesheet and add it to the shadowRoot const currentWindow = shadow.ownerDocument.defaultView || globalThis; - if (!(styleSheet.current instanceof currentWindow.CSSStyleSheet)) { - styleSheet.current = new currentWindow.CSSStyleSheet(); - shadow.adoptedStyleSheets.push(styleSheet.current); + + // bail if stylesheet already exists in the current window + if (styleSheet.current instanceof currentWindow.CSSStyleSheet) { + return; } + // create an empty stylesheet and add it to the shadowRoot + styleSheet.current = new currentWindow.CSSStyleSheet(); + shadow.adoptedStyleSheets.push(styleSheet.current); + // add the CSS immediately to avoid FOUC (one-time) if (latestCss.current) { styleSheet.current.replaceSync(latestCss.current); From 2052989d7ef12851f8ac76bb68cf22c8c7620b76 Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Thu, 19 Sep 2024 12:30:55 -0400 Subject: [PATCH 6/8] use same order as before --- packages/itwinui-react/src/utils/components/ShadowRoot.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/itwinui-react/src/utils/components/ShadowRoot.tsx b/packages/itwinui-react/src/utils/components/ShadowRoot.tsx index b5321984177..8618abe3d1c 100644 --- a/packages/itwinui-react/src/utils/components/ShadowRoot.tsx +++ b/packages/itwinui-react/src/utils/components/ShadowRoot.tsx @@ -116,14 +116,13 @@ function useShadowRoot( } const shadow = parent.shadowRoot || parent.attachShadow({ mode: 'open' }); + createStyleSheet(shadow); // Flush the state immediately after shadow-root is attached, to ensure that layout // measurements in parent component are correct. // Without this, the parent component may end up measuring the layout when the shadow-root // is attached in the DOM but React hasn't rendered any slots or content into it yet. ReactDOM.flushSync(() => setShadowRoot(shadow)); - - createStyleSheet(shadow); }; queueMicrotask(() => { From 9c61f36ec741592941c92a8db1edee423ea79f61 Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Thu, 19 Sep 2024 12:37:49 -0400 Subject: [PATCH 7/8] comment --- packages/itwinui-react/src/utils/components/ShadowRoot.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/itwinui-react/src/utils/components/ShadowRoot.tsx b/packages/itwinui-react/src/utils/components/ShadowRoot.tsx index 8618abe3d1c..7c5e4a74935 100644 --- a/packages/itwinui-react/src/utils/components/ShadowRoot.tsx +++ b/packages/itwinui-react/src/utils/components/ShadowRoot.tsx @@ -143,6 +143,7 @@ function useShadowRoot( React.useEffect(() => { const listener = () => createStyleSheet(latestShadowRoot.current); + // See https://github.com/iTwin/appui/blob/0a4cc7d127b50146e003071320d06064a09a06ae/ui/appui-react/src/appui-react/layout/widget/ContentRenderer.tsx#L74-L80 window.addEventListener('appui:reparent', listener); return () => { window.removeEventListener('appui:reparent', listener); From 6a039213e83bee861eb4c25ebef2c48f3116df55 Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Thu, 19 Sep 2024 12:50:31 -0400 Subject: [PATCH 8/8] changeset --- .changeset/shy-mangos-arrive.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/shy-mangos-arrive.md diff --git a/.changeset/shy-mangos-arrive.md b/.changeset/shy-mangos-arrive.md new file mode 100644 index 00000000000..545197158d6 --- /dev/null +++ b/.changeset/shy-mangos-arrive.md @@ -0,0 +1,5 @@ +--- +'@itwin/itwinui-react': patch +--- + +Fixed an issue where some components (e.g. `VisuallyHidden` inside `ProgressRadial`) were losing their styles when reparented into a different window.