More SideNav, shadowDom improvements #2351
Conversation
| @@ -60,7 +64,8 @@ const OdysseyProvider = <SupportedLanguages extends string>({ | |||
| shadowRootElement={shadowRootElement || shadowDomElement} | |||
| themeOverride={themeOverride} | |||
| > | |||
| <ScopedCssBaseline> | |||
| {/* This component creates a div; for flexibility of layout of children, make it inherit its parent's height */} | |||
| <ScopedCssBaseline sx={scopedCssBaselineStyles}> | |||
There was a problem hiding this comment.
I've discussed this with @KevinGhadyani-Okta and the need to allow this to respect its parent's wishes
| @@ -21,6 +21,9 @@ export const createShadowDomElements = (containerElement: HTMLElement) => { | |||
| // React app root component. | |||
| const shadowRootElement = document.createElement("div"); | |||
| shadowRootElement.setAttribute("id", "shadow-root"); | |||
| // This div could cause issues with layout of children. | |||
| // For flexibility, make it inherit its parent's height | |||
| shadowRootElement.style.setProperty("height", "inherit"); | |||
There was a problem hiding this comment.
Same here: I've discussed this with @KevinGhadyani-Okta and the need to allow this to respect its parent's wishes
There was a problem hiding this comment.
Likely this is temporary; it's copied wholesale (with formatting changes) from the existing Admin impl. @KevinGhadyani-Okta agreed we should have some kind of fallback, but I'd anticipate this changes as Design finalizes the Logo portion of the SideNav spec
| const sideNavStyles = useMemo( | ||
| () => ({ | ||
| display: "flex", | ||
| height: "100vh", | ||
| height: "100%", | ||
| maxWidth: expandedWidth, | ||
| overflow: "hidden", | ||
| }), | ||
| [], | ||
| [expandedWidth], | ||
| ); |
There was a problem hiding this comment.
We have a mix of styled components and Box styles. Is it possible we could use only styled components the whole way? Those components can take props, so you'd pass these in as props rather than as a style object. Should make it so we don't have two ways of styling components in here.
This is time-pending. It's a big change, and I don't like asking for it last-minute. If you have time, it'd be nice to unify where styles are located.
There was a problem hiding this comment.
I didn't like that there was a mix and was converting to styled when it seemed to make sense. If you're open to this ballooning, I can convert all of them
| } | ||
|
|
||
| // Determine if the scrollable container has overflow or not on load | ||
| updateIsContentScrollable(); |
oktapp-aperture-okta
bot
deleted the
bs-OKTA-798189-more-sidenav-improvements
branch
OKTA-798189 feat: side nav improvements feat: side nav, shadow dom improvements fix: revert constant testing fix: cleanup fix: pr comments fix: more pr comments fix: convert boxes to styled components fix: test fixes
OKTA-798189
Summary
This will require followup work when the Logo portion of the SideNav spec is complete, but I'd like to get this current state in to allow for future iteration.
Testing & Screenshots