components icon indicating copy to clipboard operation
components copied to clipboard

bug(cdk/drag-drop): new preview container breaks styling

Open Segdo opened this issue 2 months ago • 4 comments

Is this a regression?

  • [X] Yes, this behavior used to work in the previous version

The previous version in which this bug was not present was

17.3.5

Description

I have a html table where each tr has the cdkDrag directive so i can sort them. If the drag preview is outside of the table the styling breaks so i have set the cdkDragPreviewContainer to parent, which still works but since the latest update there is a new wrapper div around the table row which breaks the styling and html semantics because we now have a div instead of a tr in the table body.

I would expect that there is an option to disable the wrapping div or maybe even having it opt in as it breaks existing styling. The popover also comes with some own user agent styling which was not there before.

Reproduction

StackBlitz link: https://stackblitz.com/edit/components-issue-ycfvod?file=src%2Fmain.ts Steps to reproduce: 1. 2.

Expected Behavior

I would expect the tr to be directly inside the parent container when i set cdkDragPreviewContainer to parent.

Actual Behavior

The tr is inside a div instead of the parent container.

Environment

  • Angular: 17.3.6
  • CDK/Material: 17.3.6
  • Browser(s): Chrome
  • Operating System (e.g. Windows, macOS, Ubuntu): Windows 11

Segdo avatar Apr 27 '24 23:04 Segdo

Hi Segdo, Not an expert here, but i'm guessing your styling (color:red) does'nt apply because of when the tr element gets dragged, it gets removed from the table to the preview. So its no longer in table, nor tbody. It's basically a DOM/css issue imo Changing your css to reflect this could be a workaround:

  tr {
    color: red;
  }

instead of

  table>tbody>tr {
    color: red;
  }

https://stackblitz.com/edit/components-issue-qxwkxp?file=src%2Fmain.ts

if you need more specificity, you could add a class to the tr aswell:

tr.someclass{
color:red;
}

Hope this helps. Have a nice day!

PsychoSpike avatar Apr 28 '24 11:04 PsychoSpike

Yes I can workaround this issue by changing the css in the example. In my app it is a bit more problematic because I inherit the text color from the parent component, which now gets overridden for the drag preview by the user agent color property for the popover attribute on the wrapper div.

I think I could workaround this by setting the text color from my parent components using ng-deep onto the tr but this seems quite ugly.

Segdo avatar Apr 28 '24 11:04 Segdo

For context, this is caused by #28945. I went with the preview wrapper because:

  1. It ensures that the drag&drop works with native popovers. Previously it would render the preview behind them.
  2. It fixes positioning issues if the container is set to parent and the parent has a transform.

I think this is a valid issue that should be addressed, I'm just wondering how. For sure we should reset the color to be inherit on the popover instead of the user agent style. I'm a bit on the fence whether to have an opt-out from the popover completely. Aside from the text color, did you notice any layout issues from the tr being inside a div @Segdo?

crisbeto avatar Apr 28 '24 16:04 crisbeto

No issues from the div which can't be fixed by some css changes in my project. But I am a bit worried about people using css frameworks which require the tr to be directly inside the tbody. Bootstrap does this for example. Here the popover div would make bootstrap basically useless because the classes won't work anymore.

Segdo avatar Apr 28 '24 16:04 Segdo