Community.VisualStudio.Toolkit icon indicating copy to clipboard operation
Community.VisualStudio.Toolkit copied to clipboard

Tool Window Feedback: Interacting with Control and life cycle

Open niemyjski opened this issue 2 years ago • 2 comments

There are often times where you need to call methods on the controls hosted within the Tool Window and operate within the Tool Window life cycle. When this happens the current abstraction feels repetitive.

I've found myself doing this in quite a bit of my components:

    internal class MapEditorToolWindow : BaseToolWindow<MapEditorToolWindow>, IDisposable {
        private Lazy<MapEditControl> _mapEdit = new Lazy<MapEditControl>(() => new MapEditControl());

        public MapEditorToolWindow() {
                   //wire up to control handlers
        }

        public MapEditControl MapEdit => _mapEdit.Value;
        public override string GetTitle(int toolWindowId) => "Map Editor";
        public override Type PaneType => typeof(Pane);

        public override Task<FrameworkElement> CreateAsync(int toolWindowId, CancellationToken cancellationToken) {
            return Task.FromResult<FrameworkElement>(MapEdit);
        }

        public void Dispose() {
            if (_mapEdit.IsValueCreated) {
                   //unwire handlers
            }
        }

I find that in all my tool windows I'm never doing async operations within CreateAsync. Next, my control event handlers need to do async over sync (WPF / WinForms event handlers) to show a tool window (kinda expected but not good).

           // my event handler.
            var pane = MapEditorToolWindow.ShowAsync().GetAwaiter().GetResult();
            var mapEdit = pane?.Content as MapEditControl;
            if (mapEdit != null)
                mapEdit.LoadFile(e.FilePath);

Is there a better way to handle these scenarios? I liked how VSXtra handled this with attributes, and then the base tool window exposed a UIControl property which was the result of calling present day CreateAsync

niemyjski avatar Sep 17 '21 14:09 niemyjski

I've found myself doing this in quite a bit of my components:

The BaseToolWindow is never disposed at the moment, so your event handlers will never be removed. Once a tool window is created, it lives forever (even if you hide the tool window, the object still exists), and a package can never be unloaded, so I'm not sure if there's any advantage to trying to remove event handlers in a tool window.

Next, my control event handlers need to do async over sync

It's unclear from your example where that code lives. Is that event handler in a second tool window?

Regarding the async in the event handlers, you're better off doing something like this:

void MyEventHandler() {
    MyEventHandlerAsync().FireAndForget();
}

Task MyEventHandlerAsync() {
    var pane = await MapEditorToolWindow.ShowAsync();
    var mapEdit = pane?.Content as MapEditControl;
    // ...
}

reduckted avatar Oct 04 '21 05:10 reduckted

Yes a wpf event handler. Doing fire and forget as well async void is a very bad practice that even Microsoft does not recommend.

niemyjski avatar Oct 05 '21 12:10 niemyjski