patternfly / patternfly

This repo contains core (HTML/CSS) implementation for PatternFly. Issues related to CSS/HTML and layout should be filed here.
https://patternfly.org
MIT License
712 stars 97 forks source link

Bug - [Drawer] - layout change upgrading to v5 #6018

Open christianvogt opened 1 year ago

christianvogt commented 1 year ago

Describe the problem Affects Drawer component.

Caused by this change: https://github.com/patternfly/patternfly/issues/4797

min-height was removed from .pf-v5-c-drawer__body.

Excerpt from: https://drafts.csswg.org/css-flexbox/#min-size-auto

To provide a more reasonable default minimum size for flex items, the used value of a main axis automatic minimum size on a flex item that is not a scroll container is its content-based minimum size; for scroll containers the automatic minimum size is zero, as usual.

Since the node with .pf-v5-c-drawer__body is not scrollable, it defaults the min height to its contents. This differs from min-height: 0 and therefore causes lower DOM nodes that want to take over scroll behavior of nested flex elements to no longer be able to.

How do you reproduce the problem? PFv4 Codesandbox PFv5 Codesandbox

These examples are built from the PF sample and simply added a nested drawer with a scroll container. Click the toggle button to open both nested drawers and then drag the drawers to the right.

Expected behavior Expected the scroll container to be within the 2nd nested drawer. Instead the scroll container is the contents of the first drawer.

Is this issue blocking you? Not blocking as the workaround is to simply add the min-height that was removed back.

.pf-v5-c-drawer__body {
  min-height: 0;
}

Screenshots If applicable, add screenshots to help explain the issue.

PFv4 Correct ✅ image

PFv5 Incorrect ❌ image

What is your product and what release date are you targeting? RHODS / ODH

christianvogt commented 1 year ago

Another work around I found for the codesandbox provided is, when rendering these kinds of layouts, to not use DrawerContentBody . However there is no guidance in the docs that explains if DrawerContentBody should be omitted in certain scenarios and I don't want to get into a situation where something breaks in future due to another change in PF.

stale[bot] commented 11 months ago

This issue has been automatically marked as stale because it has not had activity in the last 60 days. It will be closed in 30 days if no further activity occurs.

christianvogt commented 11 months ago

bump

stale[bot] commented 9 months ago

This issue has been automatically marked as stale because it has not had activity in the last 60 days. It will be closed in 30 days if no further activity occurs.

christianvogt commented 8 months ago

bump