Tool Window Helpers

[reduckted]

Fixes #20

Overview

This pull request introduces a base class to help with the creation of tool windows. This makes using tool windows similar to commands, and allows all of the setup and creation of the tool window to occur in one file, instead of having some of it implemented in the AsyncPackage class.

Creating a Tool Window

Define a tool window by inheriting from BaseToolWindow<T>, where T is the class that inherits from BaseToolWindows<T> (similar to commands).

Example:

public class TestToolWindow : BaseToolWindow<TestToolWindow>;
{
    public override string GetTitle(int toolWindowId) => "Test Window";

    public override Type PaneType => typeof(Pane);

    public override async Task<FrameworkElement> CreateAsync(int toolWindowId, CancellationToken cancellationToken)
    {
        await ThreadHelper.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken);
        var dte = await VS.GetDTEAsync();
        return new TestWindowControl(dte);
    }

    [Guid("d0050678-2e4f-4a93-adcb-af1370da941d")]
    public class Pane : ToolWindowPane
    {
        public Pane()
        {
            BitmapImageMoniker = KnownMonikers.Test;
        }
    }
}

Declare the Pane class on the package using the ProvideToolWindow attribute:

[ProvideToolWindow(typeof(TestWindow.Pane))]

Initialize the tool window in the ToolkitPackage.InitializeAsync method. ℹ️ Note that you must inherit from ToolkitPackage instead of AsyncPackage.

public sealed class TestPackage : ToolkitPackage
{
    protected override async Task InitializeAsync(CancellationToken cancellationToken, IProgress<ServiceProgressData> progress)
    {
        TestWindow.Initialize(this);
    }
}

And to show the tool window, call the ShowAsync method on the tool window class:

await TestWindow.ShowAsync();

Known Issues

By implementing tool windows like this, the analyzer warning VSSDK003: Tool windows should support async construction will appear.

Discussion Points

• ✔️ (Done) Should the BaseToolWindow class have a Package property like commands do, so that the implementation can access the package?
• The ToolkitPackage seems like it could become large if we start adding more stuff to it in the future. How does everyone feel about splitting that file into partial files so that all of the tool window related code is in a file called ToolkitPackage.ToolWindows.cs? This is similar to how the String class is split up in the .NET runtime, as just one example.
• Would it be a good idea to add another base class that is specifically for multi-instance tool windows? Doing that would allow us to hide the tool window ID parameter for single-instance tool windows where it has no meaning.

[bluetarpmedia]

Should the BaseToolWindow class have a Package property like commands do, so that the implementation can access the package?

Yes, I'd like that so that I can pass the package to the ViewModel when constructing it inside CreateAsync. (Alternatively CreateAsync could have an AsyncPackage parameter instead of the CancellationToken.)

[reduckted]

Yes, I'd like that so that I can pass the package to the ViewModel when constructing it inside CreateAsync.

Cool. I've pushed up that change.

I've also tried out having separate base classes for single-instance and multi-instance tool windows. I've prototyped it on a separate branch. Here's the diff with this PR: https://github.com/reduckted/Community.VisualStudio.Toolkit/compare/0f28a6113524ed645c5c32226af121dd8656c8be...reduckted:4094db37bea4c829c4299b01ca51458be4646908

Essentially, the current BaseToolWindow becomes BaseMultiInstanceToolWindow and a new BaseToolWindow class is created that is the same except it drops the toolWindowId parameters to the abstract methods.

What does everyone think about that?

[yannduran]

Personally, I'd prefer overloaded methods to separate base classes. BaseMultiInstanceToolWindow is a bit too wordy as well.

A tool windows is a tool window whether it allows multiple instances or not.

[madskristensen]

I agree on having overloaded methods that take the toolWindowId. That way there is only a single base class for tool windows which keeps the concept count lower and simplifies the API.

To get rid of the analyzer error reporting the toolwindow isn't async, I think there is a great fix for that. The Pane class' constructor could take a FrameworkElement and then assign that to the Content property in the constructor. That way it looks almost identical to the standard tool window implementation and the existing documentation

[reduckted]

@yannduran @madskristensen What method are you suggesting is overloaded? There are three methods that take the toolWindowId. Two of those are abstract, and in the other one the tool window ID is optional.

public static async Task<ToolWindowPane?> ShowAsync(int id = 0, bool create = true);
public abstract string GetTitle(int toolWindowId);
public abstract Task<FrameworkElement> CreateAsync(int toolWindowId, CancellationToken cancellationToken);

[reduckted]

@madskristensen The Pane class' constructor could take a FrameworkElement and then assign that to the Content property in the constructor. That way it looks almost identical to the standard tool window implementation and the existing documentation.

I think it would be a better idea to fix the analyzer. Making the consumer do more work that this library could do automatically doesn't sound very user-friendly.

[madskristensen]

Yeah, you might be right. Is the PR ready to be merged then?

[reduckted]

@madskristensen Is the PR ready to be merged then?

Yep, if you're happy with it, then I'm happy with it. 😃

[reduckted]

Corresponding PR for the VSSDK Analyzers: https://github.com/microsoft/VSSDK-Analyzers/pull/131