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

Wrong DragAdorner offset in TreeView #346

Closed
Suplanus opened this issue Mar 13, 2020 · 5 comments · Fixed by #382
Closed

Wrong DragAdorner offset in TreeView #346

Suplanus opened this issue Mar 13, 2020 · 5 comments · Fixed by #382
Labels
Milestone

Comments

@Suplanus
Copy link

What steps will reproduce this issue?

  • Create a TreeView with parent & child items
  • Collapse childs
  • Drag with dd:DragDrop.UseDefaultDragAdorner="True"

2020-03-13_14-32-07 (1)

Expected outcome

No offset of the dragged items

Environment

  • GongSolutions.WPF.DragDrop 2.2.0
  • Windows OS 10
  • .NET Framework 4.7.2
@punker76 punker76 added the Bug label Apr 20, 2020
@Koepisch
Copy link

Koepisch commented May 10, 2020

Hi punker76,

i want to back Suplanus issue, because i'm struggling with the behavior also. Dragging groups with attached child items is a common requirement and the offset is challenging to the end user. I hope you will find the time to dig into the issue.

Thanks and regards.
Koepisch

@punker76
Copy link
Owner

punker76 commented May 10, 2020

First investigation result, I think this is a bug (or feature) in WPF. If I drag a collapsed TreeViewItem, then the captured TreeViewItem is correct. If I expand and collapse this TreeViewItem again, then the captured visual is wrong, it's the expanded descended and not the collapsed. It feels like that there is a cache mechanism.

@Metritutus
Copy link
Contributor

It's not a cache mechanism. What's happening is VisualTreeHelper.GetDescendantBounds() does not take into account the bounds (or lack thereof) of collapsed Visual elements.

I happened upon this whilst trying to figure out why the Highlight adorner would highlight the wrong amount of area when I'd collapsed a DataGridRow containing row details.

I've raised a case on the official WPF repo here.

In the mean time I have applied a workaround I have developed (detailed in the above case) to a separate branch, forked from develop, here.

I've also made alterations to DragDropExtensions.cs to try and correctly render the dragged item, which also fixes this issue (#346).

I have not created a pull request for all this yet as the reflection-based workaround is fairly horrifying, although it does appear to work as desired. With that being said however, I'm not sure if there is a better way of working around this that wouldn't risk the behaviour starting to increasingly drift from the original VisualTreeHelper.GetDescendantBounds() method.

Let me know if you do actually want me to create a pull-request with these changes, @punker76.

punker76 added a commit that referenced this issue Aug 12, 2021
…eViewItem) with our own GetVisibleDescendantBounds version

Credits and idea goes to @Metritutus
@punker76
Copy link
Owner

punker76 commented Aug 12, 2021

@Metritutus First, really thank you for looking in to this bad issue. Indeed, reflections, ah damn :-)

After looking in your code I made an alternative with some ideas from your code: https://github.com/punker76/gong-wpf-dragdrop/tree/fix/346

I changed the implementation of GetVisibleDescendantBounds and created the bounds with the height of the DesiredSize.
And yeah, this works now for the collapsed TreeViewItem :-D

It would be nice if you can check this for collapsed DataGridRow in a DataGrid, because I don't have a good sample. If you have one then feel free to add it to this PR.

@Metritutus
Copy link
Contributor

@punker76 The change in my original code was general purpose, it would work for the evaluation of any Visual with collapsed descendants (I've tested it working with DataGrid rows with collapsed row details). The only other thing I needed to change for this issue was how the drag-drop preview image was being rendered (which fixes the preview looking squashed when dragging around a Visual containing collapsed components such as a TreeViewItem after using the new GetVisibleDescendantBounds() method).

I have also had a look at your change. Because you're using VisualTreeHelper.GetDescendantBounds(), both the Width and the Location properties you're using would be affected by collapsed elements and thus potentially misaligned.

Looking at where you're using DesiredSize though, you could probably just replace what you have there with new Rect(0, 0, uiElement.DesiredSize.Width, uiElement.DesiredSize.Height).

However, although DesiredSize does seem to work in this case, it looks like it's calculated in a very different manner to how GetDescendantBounds() operates, not to mention that it depends on a Visual being a UIElement in order to correctly account for collapsed children.

On top of this, my understanding is that DesiredSize refers to how big a UIElement wishes to be, not how big it actually is after the whole layout process has completed. ActualWidth and ActualHeight would probably be closer to what is needed, but they're on FrameworkElement, which would narrow the useful usability of the function further still.

As far as I can see, the reflection solution seems to remain the best approach to this in terms of reliability and minimising functional changes. With any luck, Microsoft will come back with a positive response that either fixes this or adds a new method to VisualTreeHelper. If that is the case then I've mentioned in the case that I'm happy to try and put together a pull request for WPF for whatever change is needed to ultimately address all of this.

In the mean time, having slept on this for a bit, I'll create a pull request with my changes for this repo. If WPF gets updated to resolve this then the reflection workaround can then be removed later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants