From 63a09a1fbc3a17947e34db407734d4c717f70f49 Mon Sep 17 00:00:00 2001 From: Kevin Ghadyani <97464104+KevinGhadyani-Okta@users.noreply.github.com> Date: Thu, 6 Feb 2025 18:15:19 -0600 Subject: [PATCH] fix: shell content area overlaps side nav in mobile view (#2497) OKTA-864167 fix: fixes SSR warnings from useLayoutEffect fix: exports missing UiShellContent component fix: removes src from our package by adding .npmignore fix: refactors setStylesToMatchElement, so its type-safe fix: side nav overlapping content in mobile view fix: side nav toggle being incorrectly overlapped by top nav fix: top nav missing overflow-x: hidden causing whitespace at the bottom --- packages/odyssey-react-mui/.npmignore | 2 + .../src/theme/components.tsx | 2 +- .../ui-shell/SideNav/SideNavToggleButton.tsx | 2 +- .../src/ui-shell/TopNav/TopNav.tsx | 1 + .../src/ui-shell/UiShellContent.tsx | 99 +++++++++++++------ .../odyssey-react-mui/src/ui-shell/index.ts | 5 +- .../odyssey-react-mui/src/useContrastMode.tsx | 6 +- 7 files changed, 79 insertions(+), 38 deletions(-) create mode 100644 packages/odyssey-react-mui/.npmignore diff --git a/packages/odyssey-react-mui/.npmignore b/packages/odyssey-react-mui/.npmignore new file mode 100644 index 0000000000..c070c33188 --- /dev/null +++ b/packages/odyssey-react-mui/.npmignore @@ -0,0 +1,2 @@ +* +!dist/** diff --git a/packages/odyssey-react-mui/src/theme/components.tsx b/packages/odyssey-react-mui/src/theme/components.tsx index 50ad546458..13aba06cdd 100644 --- a/packages/odyssey-react-mui/src/theme/components.tsx +++ b/packages/odyssey-react-mui/src/theme/components.tsx @@ -1217,7 +1217,7 @@ export const getComponents = ({ [`.${inputBaseClasses.root}.${inputBaseClasses.disabled} &`]: { color: odysseyTokens.TypographyColorDisabled, WebkitTextFillColor: odysseyTokens.TypographyColorDisabled, - } as CSSProperties, + } satisfies CSSProperties, }, deleteIcon: { diff --git a/packages/odyssey-react-mui/src/ui-shell/SideNav/SideNavToggleButton.tsx b/packages/odyssey-react-mui/src/ui-shell/SideNav/SideNavToggleButton.tsx index aeb747d033..b16dc80f7e 100644 --- a/packages/odyssey-react-mui/src/ui-shell/SideNav/SideNavToggleButton.tsx +++ b/packages/odyssey-react-mui/src/ui-shell/SideNav/SideNavToggleButton.tsx @@ -51,7 +51,7 @@ const StyledToggleButton = styled(MuiButton, { width: odysseyDesignTokens.Spacing6, height: odysseyDesignTokens.Spacing6, border: 0, - zIndex: 2, + zIndex: 200, "&:focus-visible": { boxShadow: `inset 0 0 0 2px ${odysseyDesignTokens.PalettePrimaryMain}`, diff --git a/packages/odyssey-react-mui/src/ui-shell/TopNav/TopNav.tsx b/packages/odyssey-react-mui/src/ui-shell/TopNav/TopNav.tsx index 2cd47dab99..5556d51290 100644 --- a/packages/odyssey-react-mui/src/ui-shell/TopNav/TopNav.tsx +++ b/packages/odyssey-react-mui/src/ui-shell/TopNav/TopNav.tsx @@ -53,6 +53,7 @@ const StyledTopNavContainer = styled("div", { justifyContent: "space-between", maxHeight: TOP_NAV_HEIGHT, minHeight: TOP_NAV_HEIGHT, + overflowX: "hidden", paddingBlock: odysseyDesignTokens.Spacing2, paddingInline: odysseyDesignTokens.Spacing8, position: "relative", diff --git a/packages/odyssey-react-mui/src/ui-shell/UiShellContent.tsx b/packages/odyssey-react-mui/src/ui-shell/UiShellContent.tsx index 6c6e9cfba0..5b5e37eaf9 100644 --- a/packages/odyssey-react-mui/src/ui-shell/UiShellContent.tsx +++ b/packages/odyssey-react-mui/src/ui-shell/UiShellContent.tsx @@ -11,7 +11,14 @@ */ import styled from "@emotion/styled"; -import { memo, useEffect, useMemo, useRef, type ReactElement } from "react"; +import { + CSSProperties, + memo, + useEffect, + useMemo, + useRef, + type ReactElement, +} from "react"; import { ErrorBoundary, ErrorBoundaryProps } from "react-error-boundary"; import { AppSwitcher, type AppSwitcherProps } from "./AppSwitcher/index.js"; @@ -105,7 +112,7 @@ export type UiShellContentProps = { */ appBackgroundContrastMode?: ContrastMode; /** - * The element within which the app will be rendered. This will be positioned appropriately while being kept out of the shadow DOM. + * When passed, the app is expected to render into this element, not the Shadow DOM. UI Shell will position this element appropriately as if it was rendered in the app content area of the Shadow DOM. */ appContainerElement: HTMLDivElement; /** @@ -136,20 +143,39 @@ export type UiShellContentProps = { }; } & UiShellNavComponentProps; -const setStylesToMatchElement = ( - elementToStyle: HTMLElement, - elementToReference: HTMLElement, - additionalStyles: Record, -) => { - const boundingRect = elementToReference.getBoundingClientRect(); - elementToStyle.style.setProperty("position", "absolute"); - elementToStyle.style.setProperty("top", `${boundingRect.y}px`); - elementToStyle.style.setProperty("left", `${boundingRect.x}px`); - elementToStyle.style.setProperty("width", `${boundingRect.width}px`); - elementToStyle.style.setProperty("height", `${boundingRect.height}px`); - for (const key in additionalStyles) { - elementToStyle.style.setProperty(key, additionalStyles[key]); - } +export const convertCamelCaseToKebabCase = (string: string) => + string.replace(/([A-Z])/g, "-$1").toLowerCase(); + +export const setStylesToMatchElement = ({ + additionalStyles, + appContainerElement, + appContentReferenceElement, +}: { + additionalStyles: CSSProperties; + appContainerElement: HTMLElement; + appContentReferenceElement: HTMLElement; +}) => { + const boundingRect = appContentReferenceElement.getBoundingClientRect(); + + appContainerElement.style.setProperty("position", "absolute"); + appContainerElement.style.setProperty("top", `${boundingRect.y}px`); + appContainerElement.style.setProperty("left", `${boundingRect.x}px`); + appContainerElement.style.setProperty("width", `${boundingRect.width}px`); + appContainerElement.style.setProperty("height", `${boundingRect.height}px`); + + ( + Object.entries(additionalStyles) as Array< + [ + keyof typeof additionalStyles, + (typeof additionalStyles)[keyof typeof additionalStyles], + ] + > + ).forEach(([cssPropertyName, cssPropertyValue]) => { + appContainerElement.style.setProperty( + convertCamelCaseToKebabCase(cssPropertyName), + String(cssPropertyValue), + ); + }); }; /** @@ -177,29 +203,29 @@ const UiShellContent = ({ const topNavContainerRef = useRef(null); const uiShellContext = useUiShellContext(); - const paddingStyles = useMemo>( + const appContainerElementStyles = useMemo( () => ({ ...(hasStandardAppContentPadding ? { - "padding-block": odysseyDesignTokens.Spacing5 ?? null, - "padding-inline": odysseyDesignTokens.Spacing8 ?? null, + paddingBlock: odysseyDesignTokens.Spacing5 ?? null, + paddingInline: odysseyDesignTokens.Spacing8 ?? null, } : {}), ...(appContainerScrollingMode === "horizontal" || appContainerScrollingMode === "both" ? { - "overflow-x": "auto", + overflowX: "auto", } : { - "overflow-x": "hidden", + overflowX: "hidden", }), ...(appContainerScrollingMode === "vertical" || appContainerScrollingMode === "both" ? { - "overflow-y": "auto", + overflowY: "auto", } : { - "overflow-y": "hidden", + overflowY: "hidden", }), }), [ @@ -218,33 +244,42 @@ const UiShellContent = ({ cancelAnimationFrame(animationFrameId); animationFrameId = requestAnimationFrame(() => { if (appContainerRef.current) { - setStylesToMatchElement( + setStylesToMatchElement({ + additionalStyles: appContainerElementStyles, + appContentReferenceElement: appContainerRef.current, appContainerElement, - appContainerRef.current, - paddingStyles, - ); + }); } }); }; + // This element might have changed from `.current`, so it's important to keep track of the old one when we attach event listeners. + const sideNavElement = sideNavContainerRef.current; + + sideNavElement?.addEventListener("transitionend", updateStyles); + // Setup a mutation observer to sync later updates const observer = new ResizeObserver(updateStyles); observer.observe(appContainerRef.current); - if (sideNavContainerRef.current) { - observer.observe(sideNavContainerRef.current); + if (sideNavElement) { + observer.observe(sideNavElement); } if (topNavContainerRef.current) { observer.observe(topNavContainerRef.current); } - // Set the initial style + // Set the initial styles updateStyles(); - return () => observer.disconnect(); + + return () => { + observer.disconnect(); + sideNavElement?.removeEventListener("transitionend", updateStyles); + }; } return () => {}; - }, [appContainerRef, appContainerElement, paddingStyles]); + }, [appContainerElement, appContainerElementStyles, appContainerRef]); return ( diff --git a/packages/odyssey-react-mui/src/ui-shell/index.ts b/packages/odyssey-react-mui/src/ui-shell/index.ts index 6829e23684..80f3352eeb 100644 --- a/packages/odyssey-react-mui/src/ui-shell/index.ts +++ b/packages/odyssey-react-mui/src/ui-shell/index.ts @@ -18,4 +18,7 @@ export * from "./useHasUiShell.js"; export * from "../web-component/renderReactInWebComponent.js"; // This is located here because some teams use React v17, and this uses React v18's `ReactDOM/client` import which isn't in older versions. export { UiShell, type UiShellProps } from "./UiShell.js"; -export { type UiShellNavComponentProps } from "./UiShellContent.js"; +export { + UiShellContent, + type UiShellNavComponentProps, +} from "./UiShellContent.js"; diff --git a/packages/odyssey-react-mui/src/useContrastMode.tsx b/packages/odyssey-react-mui/src/useContrastMode.tsx index dc83e96937..dc4ea75910 100644 --- a/packages/odyssey-react-mui/src/useContrastMode.tsx +++ b/packages/odyssey-react-mui/src/useContrastMode.tsx @@ -12,11 +12,11 @@ import { createContext, + useCallback, useContext, + useEffect, useRef, - useLayoutEffect, useState, - useCallback, } from "react"; import * as Tokens from "@okta/odyssey-design-tokens"; @@ -109,7 +109,7 @@ export const useContrastMode = ({ } }, [explicitContrastMode]); - useLayoutEffect(() => { + useEffect(() => { const observer = new MutationObserver(updateBackgroundColor); observer.observe(document.querySelector("html") as HTMLHtmlElement, { attributes: true,