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

SharpTreeView: Split into ILSpyX.TreeView and ILSpy.Controls.TreeView #3240

Merged
merged 7 commits into from
Jul 28, 2024

Conversation

christophwille
Copy link
Member

@christophwille christophwille commented Jul 22, 2024

#3172 (comment)

This is the basics - splitting xplat and WPF-specific parts of SharpTreeView and distribute to ILSpyX and ILSpy projects respectively.

@christophwille
Copy link
Member Author

@Rpinski please add your comments from the viewpoint of the ilspy-vscode extension

@jmacato @maxkatz6 from the point of view of Avalonia / Avalonia XPF - would such a tree node move/change as outlined in the issue comment plus this PoC PR be in line with that architecture or do the IPlatform* abstractions completely run counter what Avalonia would need?

@jmacato
Copy link

jmacato commented Jul 23, 2024

Hi @christophwille !

The changes looks fine for my perspective on XPF, as it just abstracts out the needed interfaces for the treenode controls.

@Rpinski
Copy link
Member

Rpinski commented Jul 23, 2024

From the view of ilspy-vscode, what I'm interested in is:

  1. What does the node represent? (any sort of type to determine icon, display name etc.)
  2. How does the node look like decompiled?
  3. What are the node's children? (preferably nodes themselves should know how to create their children)

It looks like this approach would deliver all of that. I don't need anything related to UI interactions like Drag&Drop or cut/copy support. But I don't think that would harm as long as any dependencies on UI frameworks are abstracted away. That needs some more effort.

@christophwille christophwille changed the title SharpTreeNode cross platform PoC SharpTreeView: Split into ILSpyX.TreeView and ILSpy.Controls.TreeView Jul 24, 2024
@christophwille christophwille marked this pull request as ready for review July 24, 2024 14:16
{
public interface IPlatformDragDrop
{
XPlatDragDropEffects DoDragDrop(object dragSource, object data, XPlatDragDropEffects allowedEffects);
Copy link

@maxkatz6 maxkatz6 Jul 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: this and other DnD operations should be async, as they are async in nature.

On desktop (and Browser with multithreading and hacks enabled), we can solve it by using DispatcherFrame to sync-wait on async operations. But since you are refactoring this part, consider making it async from the start as well.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note, DispatcherFrame is usable with both XPF and pure Avalonia. That's what I was using so far in new port too.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Except mobile, Avalonia doesn't have any way to use DispatcherFrame on mobile yet. But I assume ILSpy won't benefit much from mobile support either way. Browser might be useful though.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, interesting - I was only reshuffling existing code, and there DoDragDrop was used in exactly that fashion.

@@ -144,7 +146,8 @@
<Style TargetType="{x:Type MenuItem}" BasedOn="{StaticResource {x:Type MenuItem}}">
<Setter Property="Command" Value="{x:Static local:ILSpyCommands.SetTheme}" />
<Setter Property="CommandParameter" Value="{Binding}" />
<Setter Property="IsCheckable" Value="True" /> <!-- Required by AvalonDock's MenuItem style to show the checkmark -->
<Setter Property="IsCheckable" Value="True" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment was moved to the wrong line...

@christophwille christophwille merged commit d54ac41 into master Jul 28, 2024
9 checks passed
@christophwille christophwille deleted the cw/stnpoc branch July 28, 2024 13:28
mattsh247 pushed a commit to mattsh247/ILSpy that referenced this pull request Jul 30, 2024
…icsharpcode#3240)

* Changes necessary for making SharpTreeNode cross platform by proxying System.Windows dependencies
* Add ITreeNodeImagesProvider for node icons
* Move InternalsVisibleTo to csproj (possible since net50)
* Move view models and other xplat class for SharpTreeView to ILSpyX, Windows-dependent classes to ILSpy/Controls/TreeView
* Move GetDoubleClickTime to NativeMethods
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 this pull request may close these issues.

5 participants