edda commented 4 weeks ago

Summary

This PR tries to improve upon the previous fix for the z-index stacking problem introduced in the recent refactoring of PortalProvider. While the previous fix in https://github.com/cloudoperators/juno/pull/565 solved the issue for Juno apps we still had potential z-index stacking issues if a Juno app was embedded into a host app with its own z-index stack. This is because the solution from the previous PR aimed to standardize all z-index on the root level to 1 and then added stacks above this in the portaled components as necessary.

In the cases where a Juno app is embedded in a host app with z-indexes higher than 1 the whole thing would break.

This PR aims to fix this by setting a very high z-index (9999) for the portal root div which acts as the wrapper for all portals. So as long as the host app's z-index is at max 9999 the z-stacking will be correct.

Changes Made

Added z-index: 9999 to portal root div in PortalProvider
Undid z-index change to PageHeader and Label from PR#565 since the new portal root z-index ensure proper stacking above these components for portaled components

Related Issues

https://github.com/cloudoperators/juno/pull/565

Testing Instructions

start exampleapp or supernova in this branch
open a Panel --> it should be stacked above the PageHeader
in Supernova scroll down so that the filter toolbar gets scrolled under PageHeader --> the Select's Label should not be visible

Checklist

[x] I have performed a self-review of my code.
[x] I have commented my code, particularly in hard-to-understand areas.
[ ] I have added tests that prove my fix is effective or that my feature works.
[x] New and existing unit tests pass locally with my changes.
[ ] I have made corresponding changes to the documentation (if applicable).
[x] My changes generate no new warnings or errors.

changeset-bot[bot] commented 4 weeks ago

🦋 Changeset detected

Latest commit: 7bf8b8c577ee60c7672a84033e5a4289892df499

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

| Name | Type | | ---------------------------------- | ----- | | @cloudoperators/juno-ui-components | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

github-actions[bot] commented 4 weeks ago

PR Preview Action v1.4.8 :---: Preview removed because the pull request was closed. 2024-11-04 09:43 UTC

ArtieReus commented 3 weeks ago

Tested changes in elektra:
Tested changes in example:
Tested changes in supernova:
all look good

andypf commented 3 weeks ago

We could also define a property in the AppShellProvider and other components like AppShell to set the starting z-index. For example, we could specify startZIndex={60}. This would allow us to handle the z-index as usual (e.g., starting from 1 as in your last fix) while covering the Elektra case by setting the starting z-index higher than 50, such as 60.

edda commented 3 weeks ago