Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
1 task done
Segdo opened this issue Apr 27, 2024 · 4 comments · Fixed by #28976
Closed
1 task done

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

Segdo opened this issue Apr 27, 2024 · 4 comments · Fixed by #28976
Assignees
Labels
area: cdk/drag-drop P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent

Comments

@Segdo
Copy link

Segdo commented Apr 27, 2024

Is this a regression?

  • 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 Segdo added the needs triage This issue needs to be triaged by the team label Apr 27, 2024
@PsychoSpike
Copy link

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!

@Segdo
Copy link
Author

Segdo commented Apr 28, 2024

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.

@crisbeto
Copy link
Member

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?

@Segdo
Copy link
Author

Segdo commented Apr 28, 2024

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.

crisbeto added a commit to crisbeto/material2 that referenced this issue Apr 29, 2024
After angular#28945 the preview is inserted into a `popover` element which has a user agent styling of `color: canvastext`. These changes reset it to `inherit` to match the old behavior.

Fixes angular#28974.
@crisbeto crisbeto self-assigned this Apr 29, 2024
@crisbeto crisbeto added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent area: cdk/drag-drop and removed needs triage This issue needs to be triaged by the team labels Apr 29, 2024
crisbeto added a commit to crisbeto/material2 that referenced this issue Apr 29, 2024
After angular#28945 the preview is inserted into a `popover` element which has a user agent styling of `color: canvastext`. These changes reset it to `inherit` to match the old behavior.

Fixes angular#28974.
crisbeto added a commit to crisbeto/material2 that referenced this issue Apr 30, 2024
After angular#28945 the preview is inserted into a `popover` element which has a user agent styling of `color: canvastext`. These changes reset it to `inherit` to match the old behavior.

Fixes angular#28974.
crisbeto added a commit that referenced this issue May 1, 2024
After #28945 the preview is inserted into a `popover` element which has a user agent styling of `color: canvastext`. These changes reset it to `inherit` to match the old behavior.

Fixes #28974.
crisbeto added a commit that referenced this issue May 1, 2024
After #28945 the preview is inserted into a `popover` element which has a user agent styling of `color: canvastext`. These changes reset it to `inherit` to match the old behavior.

Fixes #28974.

(cherry picked from commit a3b3211)
crisbeto added a commit that referenced this issue May 1, 2024
After #28945 the preview is inserted into a `popover` element which has a user agent styling of `color: canvastext`. These changes reset it to `inherit` to match the old behavior.

Fixes #28974.

(cherry picked from commit a3b3211)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: cdk/drag-drop P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants