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

Windows Task Dialog #146

Closed
kpreisser opened this issue Dec 4, 2018 · 94 comments · Fixed by #1133
Closed

Windows Task Dialog #146

kpreisser opened this issue Dec 4, 2018 · 94 comments · Fixed by #1133
Assignees
Labels
api-approved (4) API was approved in API review, it can be implemented design-discussion Ongoing discussion about design without consensus tenet-compatibility-OS Compatibility with OS features

Comments

@kpreisser
Copy link
Contributor

kpreisser commented Dec 4, 2018

Hi,

On Windows Vista and higher, the Task Dialog is available that provides many more features than a Message Box. While you can show a Message Box in WinForms and WPF, there is no "official" implementation of the Task Dialog yet in .NET WinForms/WPF.

There was an implementation in the Windows API Code Pack 1.1, but it is no longer available/updated, it did not implement all features (like navigation or modifying common/standard buttons), and I believe it had some memory management issues (like not calling Marshal.DestroyStructure() after calling Marshal.StructureToPtr() in order to free allocated strings for custom/radio buttons) and a few other issues.

At my company, we currently use the Task Dialog in a (commercial) WPF application to show a marquee progress bar while an operation (like database backup) is running, and then navigate it to one showing a green header to indicate the operation is finished.

Visual Studio is also using a Task Dialog:
taskdialog-vs

Also, the Windows Design Guidelines (Desktop Apps) for Messages and Dialog Boxes show features of the task dialog.

Do you think a Task Dialog could also be added directly to WinForms/WPF? Thank you!

Edit:


Rationale and Usage

The Windows Task Dialog (which is available since Windows Vista) has a lot of configuration options comparing to a regular Message Box, can show additional controls like a progress bar, and supports event handling. However, it has not yet been integrated officially into WinForms/WPF, so if you wanted to use it, you had to implement the native APIs yourself, or use a 3rd party library.

Implementing the Task Dialog directly in WinForms allows users to directly use the Task Dialog in any new WinForms/WPF .NET Core application, just like a MessageBox. You can then either use the simple static Show() method (similar to a MessageBox), or you can create an instance of the TaskDialog, configure its TaskDialogPage and then show it.

Features of the proposed Task Dialog:

  • Supports all of the native Task Dialog elements (like custom buttons/command links, progress bar, radio buttons, checkbox, expanded area, footer)
  • Some dialog elements can be updated while the dialog is displayed, and the dialog can be closed from code
  • Additionally to standard icons, supports shield icons that show a green, yellow, red, gray or blue bar
  • Can navigate to a new page (by reconstructing the dialog from current properties) by calling TaskDialogPage.Navigate(TaskDialogPage) while the dialog is displayed
  • Can be shown modal or non-modal (when showing modal, can be centered to the parent)
  • Exposes its window handle (hWnd) through the Handle property so that the dialog window can be further manipulated (or used as owner for another window)

See also the Task Dialog Demo App for examples.

Show a simple Task Dialog

    TaskDialogButton resultButton = TaskDialog.ShowDialog(new TaskDialogPage()
    {
        Text = "Hello World!",
        Heading = "Hello Task Dialog!   👍",
        Caption = "Dialog Title",
        Buttons = {
            TaskDialogButton.Yes,
            TaskDialogButton.Cancel
        },
        Icon = TaskDialogIcon.ShieldSuccessGreenBar
    });

    if (resultButton == TaskDialogButton.Yes)
    {
        // Do something...
    }

simpletaskdialog

Dialog similar to the Visual Studio dialog

    TaskDialogCommandLinkButton buttonRestart = new TaskDialogCommandLinkButton()
    {
        Text = "&Restart under different credentials",
        DescriptionText = "This text will be shown in the second line.",
        ShowShieldIcon = true
    };

    TaskDialogCommandLinkButton buttonCancelTask = new TaskDialogCommandLinkButton()
    {
        Text = "&Cancel the Task and return"
    };

    var page = new TaskDialogPage()
    {
        Icon = TaskDialogIcon.Shield,
        Heading = "This task requires the application to have elevated permissions.",
        // TODO - Hyperlinks will be possible in a future version
        Text = "Why is using the Administrator or other account necessary?",

        // TODO - will be possible in a future version
        //EnableHyperlinks = true,

        Buttons =
        {
            TaskDialogButton.Cancel,
            buttonRestart,
            buttonCancelTask
        },
        DefaultButton = buttonCancelTask,

        // Show a expander.
        Expander = new TaskDialogExpander()
        {
            Text = "Some expanded Text",
            CollapsedButtonText = "View error information",
            ExpandedButtonText = "Hide error information",
            Position = TaskDialogExpanderPosition.AfterFootnote
        }
    };

    // Show the dialog and check the result.
    TaskDialogButton result = TaskDialog.ShowDialog(page);

    if (result == buttonRestart)
    {
        Console.WriteLine("Restarting...");
    }

grafik

Show a multi-page dialog that shows current progress, then navigates to a result

See also: Multi-page dialog boxes

    // Disable the "Yes" button and only enable it when the check box is checked.
    // Also, don't close the dialog when this button is clicked.
    var initialButtonYes = TaskDialogButton.Yes;
    initialButtonYes.Enabled = false;
    initialButtonYes.AllowCloseDialog = false;

    var initialPage = new TaskDialogPage()
    {
        Caption = "My Application",
        Heading = "Clean up database?",
        Text = "Do you really want to do a clean-up?\nThis action is irreversible!",
        Icon = TaskDialogIcon.ShieldWarningYellowBar,
        AllowCancel = true,

        Verification = new TaskDialogVerificationCheckBox()
        {
            Text = "I know what I'm doing"
        },

        Buttons =
        {
            TaskDialogButton.No,
            initialButtonYes
        },
        DefaultButton = TaskDialogButton.No
    };

    // For the "In Progress" page, don't allow the dialog to close, by adding
    // a disabled button (if no button was specified, the task dialog would
    // get an (enabled) 'OK' button).
    var inProgressCloseButton = TaskDialogButton.Close;
    inProgressCloseButton.Enabled = false;

    var inProgressPage = new TaskDialogPage()
    {
        Caption = "My Application",
        Heading = "Operation in progress...",
        Text = "Please wait while the operation is in progress.",
        Icon = TaskDialogIcon.Information,

        ProgressBar = new TaskDialogProgressBar()
        {
            State = TaskDialogProgressBarState.Marquee
        },

        Expander = new TaskDialogExpander()
        {
            Text = "Initializing...",
            Position = TaskDialogExpanderPosition.AfterFootnote
        },

        Buttons =
        {
            inProgressCloseButton
        }
    };

    var finishedPage = new TaskDialogPage()
    {
        Caption = "My Application",
        Heading = "Success!",
        Text = "The operation finished.",
        Icon = TaskDialogIcon.ShieldSuccessGreenBar,
        Buttons =
        {
            TaskDialogButton.Close
        }
    };

    TaskDialogButton showResultsButton = new TaskDialogCommandLinkButton("Show &Results");
    finishedPage.Buttons.Add(showResultsButton);

    // Enable the "Yes" button only when the checkbox is checked.
    TaskDialogVerificationCheckBox checkBox = initialPage.Verification;
    checkBox.CheckedChanged += (sender, e) =>
    {
        initialButtonYes.Enabled = checkBox.Checked;
    };

    // When the user clicks "Yes", navigate to the second page.
    initialButtonYes.Click += (sender, e) =>
    {
        // Navigate to the "In Progress" page that displays the
        // current progress of the background work.
        initialPage.Navigate(inProgressPage);

        // NOTE: When you implement a "In Progress" page that represents
        // background work that is done e.g. by a separate thread/task,
        // which eventually calls Control.Invoke()/BeginInvoke() when
        // its work is finished in order to navigate or update the dialog,
        // then DO NOT start that work here already (directly after
        // setting the Page property). Instead, start the work in the
        // TaskDialogPage.Created event of the new page.
        //
        // This is because if you started it here, then when that other
        // thread/task finishes and calls BeginInvoke() to call a method in
        // the GUI thread to update or navigate the dialog, there is a chance
        // that the callback might be called before the dialog completed
        // navigation (*) (as indicated by the Created event of the
        // new page), and the dialog might not be updatable in that case.
        // (The dialog can be closed or navigated again, but you cannot
        // change e.g. text properties of the page).
        //
        // If that's not possible for some reason, you need to ensure
        // that you delay the call to update the dialog until the Created
        // event of the next page has occured.
        // 
        // 
        // (*) Background info: Although the WinForms implementation of
        // Control.Invoke()/BeginInvoke() posts a new message in the
        // control's owning thread's message queue every time it is
        // called (so that the callback can be called later by the
        // message loop), when processing the posted message in the
        // control's window procedure, it calls ALL stored callbacks
        // instead of only the next one.
        // 
        // This means that even if you start the work after setting
        // the Page property (which means BeginInvoke() can only be
        // called AFTER starting navigation), the callback specified
        // by BeginInvoke might still be called BEFORE the task dialog
        // can process its posted navigation message.
    };

    // Simulate work by starting an async operation from which we are updating the
    // progress bar and the expander with the current status.
    inProgressPage.Created += async (s, e) =>
    {
        // Run the background operation and iterate over the streamed values to update
        // the progress. Because we call the async method from the GUI thread,
        // it will use this thread's synchronization context to run the continuations,
        // so we don't need to use Control.[Begin]Invoke() to schedule the callbacks.
        var progressBar = inProgressPage.ProgressBar;

        await foreach (int progressValue in StreamBackgroundOperationProgressAsync())
        {
            // When we display the first progress, switch the marquee progress bar
            // to a regular one.
            if (progressBar.State == TaskDialogProgressBarState.Marquee)
                progressBar.State = TaskDialogProgressBarState.Normal;

            progressBar.Value = progressValue;
            inProgressPage.Expander.Text = $"Progress: {progressValue} %";
        }

        // Work is finished, so navigate to the third page.
        inProgressPage.Navigate(finishedPage);
    };

    // Show the dialog (modeless).
    TaskDialogButton result = TaskDialog.ShowDialog(initialPage);
    if (result == showResultsButton)
    {
        Console.WriteLine("Showing Results!");
    }


    static async IAsyncEnumerable<int> StreamBackgroundOperationProgressAsync()
    {
        // Note: The code here will run in the GUI thread - use
        // "await Task.Run(...)" to schedule CPU-intensive operations in a
        // worker thread.

        // Wait a bit before reporting the first progress.
        await Task.Delay(2800);

        for (int i = 0; i <= 100; i += 4)
        {
            // Report the progress.
            yield return i;

            // Wait a bit to simulate work.
            await Task.Delay(200);
        }
    }

wizarddialog

Other examples from existing applications

"Save document" dialog from Notepad/Paint/WordPad

    TaskDialogButton btnCancel = TaskDialogButton.Cancel;
    TaskDialogButton btnSave = new TaskDialogButton("&Save");
    TaskDialogButton btnDontSave = new TaskDialogButton("Do&n't save");

    var page = new TaskDialogPage()
    {
        Caption = "My Application",
        Heading = "Do you want to save changes to Untitled?",
        Buttons =
        {
            btnCancel,
            btnSave,
            btnDontSave
        }
    };

    // Show a modal dialog, then check the result.
    TaskDialogButton result = TaskDialog.ShowDialog(this, page);

    if (result == btnSave)
        Console.WriteLine("Saving");
    else if (result == btnDontSave)
        Console.WriteLine("Not saving");
    else
        Console.WriteLine("Canceling");

taskdialog-savedocument

Windows 7 Minesweeper Difficulty Selection

    var page = new TaskDialogPage()
    {
        Caption = "Minesweeper",
        Heading = "What level of difficulty do you want to play?",
        AllowCancel = true,

        Footnote = new TaskDialogFootnote()
        {
            Text = "Note: You can change the difficulty level later " +
                "by clicking Options on the Game menu.",
        },

        Buttons =
        {
            new TaskDialogCommandLinkButton("&Beginner", "10 mines, 9 x 9 tile grid")
            {
                Tag = 10
            },
            new TaskDialogCommandLinkButton("&Intermediate", "40 mines, 16 x 16 tile grid")
            {
                Tag = 40
            },
            new TaskDialogCommandLinkButton("&Advanced", "99 mines, 16 x 30 tile grid")
            {
                Tag = 99
            }
        }
    };

    TaskDialogButton result = TaskDialog.ShowDialog(this, page);

    if (result.Tag is int resultingMines)
        Console.WriteLine($"Playing with {resultingMines} mines...");
    else
        Console.WriteLine("User canceled.");

taskdialog-minesweeperdifficulty

Windows Security dialog when trying to access network files

    var page = new TaskDialogPage()
    {
        Caption = "My App Security",
        Heading = "Opening these files might be harmful to your computer",
        Text = "Your Internet security settings blocked one or more files from " + 
            "being opened. Do you want to open these files anyway?",
        Icon = TaskDialogIcon.ShieldWarningYellowBar,
        // TODO - will be possible in a future version
        //EnableHyperlinks = true,

        Expander = new TaskDialogExpander("My-File-Sample.exe"),

        Footnote = new TaskDialogFootnote()
        {
            // TODO - Hyperlinks will be possible in a future version
            Text = "How do I decide whether to open these files?",
        },

        Buttons =
        {
            TaskDialogButton.OK,
            TaskDialogButton.Cancel
        },
        DefaultButton = TaskDialogButton.Cancel
    };

    TaskDialogButton result = TaskDialog.ShowDialog(this, page);

    if (result == TaskDialogButton.OK)
    {
        Console.WriteLine("OK selected");
    }

taskdialog-windowssecurity

Auto-closing Dialog (closes after 5 seconds)

    const string textFormat = "Reconnecting in {0} seconds...";
    int remainingTenthSeconds = 50;

    var reconnectButton = new TaskDialogButton("&Reconnect now");
    var cancelButton = TaskDialogButton.Cancel;

    var page = new TaskDialogPage()
    {
        Heading = "Connection lost; reconnecting...",
        Text = string.Format(textFormat, (remainingTenthSeconds + 9) / 10),
        ProgressBar = new TaskDialogProgressBar()
        {
            State = TaskDialogProgressBarState.Paused
        },
        Buttons =
        {
            reconnectButton,
            cancelButton
        }
    };

    // Create a WinForms timer that raises the Tick event every tenth second.
    using var timer = new System.Windows.Forms.Timer()
    {
        Enabled = true,
        Interval = 100
    };

    timer.Tick += (s, e) =>
    {
        remainingTenthSeconds--;
        if (remainingTenthSeconds > 0)
        {
            // Update the remaining time and progress bar.
            page.Text = string.Format(textFormat, (remainingTenthSeconds + 9) / 10);
            page.ProgressBar.Value = 100 - remainingTenthSeconds * 2;
        }
        else
        {
            // Stop the timer and click the "Reconnect" button - this will
            // close the dialog.
            timer.Enabled = false;
            reconnectButton.PerformClick();
        }
    };

    TaskDialogButton result = TaskDialog.ShowDialog(this, page);
    if (result == reconnectButton)
        Console.WriteLine("Reconnecting.");
    else
        Console.WriteLine("Not reconnecting.");

autoclosingdialog

Proposed API

TODO: Which namespace to use for the types? In the PR I used System.Windows.Forms for now.

public class TaskDialog : IWin32Window
{
    // Returns the window handle while the dialog is shown, otherwise returns IntPtr.Zero.
    public IntPtr Handle { get; }

    // Note: The ShowDialog() methods do not return until the
    // dialog is closed (similar to MessageBox.Show()), regardless of whether the
    // dialog is shown modal or non-modal.

    public static TaskDialogButton ShowDialog(TaskDialogPage page, TaskDialogStartupLocation startupLocation = TaskDialogStartupLocation.CenterOwner);
    public static TaskDialogButton ShowDialog(IWin32Window owner, TaskDialogPage page, TaskDialogStartupLocation startupLocation = TaskDialogStartupLocation.CenterOwner);
    public static TaskDialogButton ShowDialog(IntPtr hwndOwner, TaskDialogPage page, TaskDialogStartupLocation startupLocation = TaskDialogStartupLocation.CenterOwner);

    // Close() will simulate a click on the "Cancel" button (but you don't
    // have to add a "Cancel" button for this).
    public void Close();
}
public class TaskDialogPage
{
    public TaskDialogPage();

    public TaskDialog? BoundDialog { get; }

    // Properties "Caption", "MainInstruction", "Text", "Icon" can be set
    // set while the dialog is shown, to update the dialog.
    public string? Caption { get; set; }
    public string? Heading { get; set; }
    public string? Text { get; set; }

    // Icon can either be a standard icon or a icon handle.
    // (In the future, we could allow to show a resource icon.)
    // Note that while the dialog is shown, you cannot switch between a 
    // handle icon type and a non-handle icon type.
    // The same applies to the footer icon.
    public TaskDialogIcon? Icon { get; set; }    
    
    public bool AllowCancel { get; set; }
    public bool AllowMinimize { get; set; }
    public bool RightToLeftLayout { get; set; }
    public bool SizeToContent { get; set; }

    public TaskDialogButtonCollection Buttons { get; set; }
    public TaskDialogRadioButtonCollection RadioButtons { get; set; }
    public TaskDialogButton? DefaultButton { get; set; }

    // Note: When creating a TaskDialogPage instance, these four properties
    // contain default/empty control instances (for better Designer support) that
    // do not show up in the dialog unless you modify their properties
    // (because their initial "Text" is null and initial ProgressBarState is "None" -
    // however when you create a new ProgressBar instance, its default State
    // is "Normal"), but you can also set them to null.
    public TaskDialogVerificationCheckBox? Verification { get; set; }
    public TaskDialogExpander? Expander { get; set; }
    public TaskDialogFootnote? Footnote { get; set; }
    public TaskDialogProgressBar? ProgressBar { get; set; }

    // See section "Event Cycle" for a diagram illustrating the events.

    // Raised after this TaskDialogPage is bound to a TaskDialog (and therefore
    // the controls have been created): after the dialog was created (directly after event
    // TaskDialog.Opened/TDN_CREATED) or navigated (in the TDN_NAVIGATED handler).
    public event EventHandler? Created;
    // Raised when this TaskDialogPage is about to be unbound from a TaskDialog
    // (and therefore the controls are about to be destroyed): when the dialog is
    // about to be destroyed (directly before event TaskDialog.Closed) or about
    // to navigate.
    public event EventHandler? Destroyed;

    // Raised when the user presses F1 or clicks the "Help" standard button
    public event EventHandler? HelpRequest;

    protected internal void OnCreated(EventArgs e);
    protected internal void OnDestroyed(EventArgs e);
    protected internal void OnHelpRequest(EventArgs e);
}
public class TaskDialogIcon : IDisposable
{
    // "Standard" icons
    public static readonly TaskDialogIcon None;
    public static readonly TaskDialogIcon Information;
    public static readonly TaskDialogIcon Warning;
    public static readonly TaskDialogIcon Error;
    public static readonly TaskDialogIcon Shield;
    public static readonly TaskDialogIcon ShieldBlueBar;
    public static readonly TaskDialogIcon ShieldGrayBar;
    public static readonly TaskDialogIcon ShieldWarningYellowBar;
    public static readonly TaskDialogIcon ShieldErrorRedBar;
    public static readonly TaskDialogIcon ShieldSuccessGreenBar;

    public TaskDialogIcon(Bitmap image);
    public TaskDialogIcon(Icon icon);
    public TaskDialogIcon(IntPtr iconHandle);

    // Note: This will throw (InvalidOperationException) if this is an
    // instance representing a standard icon.
    public IntPtr IconHandle { get; }
}
public abstract class TaskDialogControl
{
    public TaskDialogPage? BoundPage { get; }
    public object? Tag { get; set; }
}
public class TaskDialogButton : TaskDialogControl
{
    public TaskDialogButton();
    public TaskDialogButton(string? text, bool enabled = true, bool allowCloseDialog = true);

    public static TaskDialogButton OK { get; }
    public static TaskDialogButton Cancel { get; }
    public static TaskDialogButton Abort { get; }
    public static TaskDialogButton Retry { get; }
    public static TaskDialogButton Ignore { get; }
    public static TaskDialogButton Yes { get; }
    public static TaskDialogButton No { get; }
    public static TaskDialogButton Close { get; }
    // Note: Clicking the "Help" button will not close the dialog, but will
    // raise the TaskDialogPage.Help event.
    public static TaskDialogButton Help { get; }
    public static TaskDialogButton TryAgain { get; }
    public static TaskDialogButton Continue { get; }
    
    // Properties "Enabled" and "ShowShieldIcon" can be set while
    // the dialog is shown.
    public string? Text { get; set; }
    public bool AllowCloseDialog { get; set; }
    public bool Enabled { get; set; }
    public bool ShowShieldIcon { get; set; }
    // Setting "Visible" to false means the button will not be shown in the task dialog
    // (the property cannot be set while the dialog is shown). This allows you
    // to intercept button click events, e.g. "Cancel" when "AllowCancel" is true
    // and the user presses ESC, without having to actually show a "Cancel" button.
    public bool Visible { get; set; }

    // Raised when this button is clicked while the dialog is shown (either because the
    // user clicked it, or by calling PerformClick() or TaskDialog.Close()).
    public event EventHandler? Click;

    public override bool Equals(object? obj);
    public override int GetHashCode();
    public void PerformClick();
    public override string ToString();

    public static bool operator ==(TaskDialogButton? b1, TaskDialogButton? b2);
    public static bool operator !=(TaskDialogButton? b1, TaskDialogButton? b2);
}
public sealed class TaskDialogCommandLinkButton : TaskDialogButton
{
    public TaskDialogCommandLinkButton();
    public TaskDialogCommandLinkButton(string? text, string? descriptionText = null, bool enabled = true, bool allowCloseDialog = true);

    public string? DescriptionText { get; set; }
}
public sealed class TaskDialogRadioButton : TaskDialogControl
{
    public TaskDialogRadioButton();
    public TaskDialogRadioButton(string? text);

    public string? Text { get; set; }
    // Properties "Enabled" and "Checked" can be set while the dialog is shown
    // (but "Checked" can then only be set to "true").
    public bool Checked { get; set; }
    public bool Enabled { get; set; }

    // Raised when the "Checked" property changes while the dialog is shown (because
    // the user clicked one of the radio buttons, or due to setting the "Checked"
    // property of one of the radio buttons to "true").
    public event EventHandler? CheckedChanged;

    public override string ToString();
}
public sealed class TaskDialogVerificationCheckBox : TaskDialogControl
{
    public TaskDialogVerificationCheckBox();
    public TaskDialogVerificationCheckBox(string? text, bool isChecked = false);

    public string? Text { get; set; }
    // "Checked" can be set while the dialog is shown.
    public bool Checked { get; set; }

    // Raised when the "Checked" property changes while the dialog is shown (because
    // the user clicked it or due to setting the "Checked" property).
    public event EventHandler? CheckedChanged;

    public override string ToString();

    public static implicit operator TaskDialogVerificationCheckBox(string verificationText);
}
public sealed class TaskDialogExpander : TaskDialogControl
{
    public TaskDialogExpander();
    public TaskDialogExpander(string? text);

     // "Text" can be set while the dialog is shown.
    public string? Text { get; set; }
    public string? ExpandedButtonText { get; set; }
    public string? CollapsedButtonText { get; set; }
    // Note: "Expanded" can NOT be set while the dialog is shown.
    public bool Expanded { get; set; }
    public TaskDialogExpanderPosition Position { get; set; }

    // Raised when the "Expanded" property changes while the dialog is shown (because
    // the user clicked the expando button).
    public event EventHandler? ExpandedChanged;

    public override string ToString();
}
public sealed class TaskDialogFootnote : TaskDialogControl
{
    public TaskDialogFootnote();
    public TaskDialogFootnote(string? text);

    // Properties "Text",  "Icon" can be set while
    // the dialog is shown (see comments for TaskDialogPage.Icon).
    public string? Text { get; set; }
    public TaskDialogIcon? Icon { get; set; }

    public override string ToString();

    public static implicit operator TaskDialogFootnote(string footnoteText);
}
public sealed class TaskDialogProgressBar : TaskDialogControl
{
    public TaskDialogProgressBar();
    public TaskDialogProgressBar(TaskDialogProgressBarState state);

    // Properties "State", "Minimum", "Maximum", "Value", "MarqueeSpeed" can
    // be set while the dialog is shown.
    public TaskDialogProgressBarState State { get; set; } // "Style"?
    public int Minimum { get; set; }
    public int Maximum { get; set; }
    public int Value { get; set; }
    public int MarqueeSpeed { get; set; }
}
// Note: The button order in this collection is not necessarily the same as the actual
// order of buttons displayed in the dialog. See:
// https://github.com/kpreisser/winforms/issues/5#issuecomment-584318609
public class TaskDialogButtonCollection : Collection<TaskDialogButton>
{
    public TaskDialogButtonCollection();

    public TaskDialogButton Add(string? text, bool enabled = true, bool allowCloseDialog = true);
    protected override void ClearItems();
    protected override void InsertItem(int index, TaskDialogButton item);
    protected override void RemoveItem(int index);
    protected override void SetItem(int index, TaskDialogButton item);
}
public class TaskDialogRadioButtonCollection : System.Collections.ObjectModel.Collection<TaskDialogRadioButton>
{
    public TaskDialogRadioButtonCollection();

    public TaskDialogRadioButton Add(string? text);

    protected override void ClearItems();
    protected override void InsertItem(int index, TaskDialogRadioButton item);
    protected override void RemoveItem(int index);
    protected override void SetItem(int index, TaskDialogRadioButton item);
}
public enum TaskDialogStartupLocation
{
    CenterScreen = 0,
    CenterOwner = 1
}
// Rename to "TaskDialogProgressBarStyle"?
public enum TaskDialogProgressBarState
{
    Normal = 0,
    Paused = 1,
    Error = 2,
    Marquee = 3,
    MarqueePaused = 4,
    // "None" is used for the default ProgressBar instance in the TaskDialogPage so
    // that you need to set the State to a different value (or create a new ProgressBar
    // instance) to actually show a progress bar in the dialog.
    None = 5
}
public enum TaskDialogExpanderPosition
{
    AfterText = 0,
    AfterFootnote = 1
}

Event Cycle

The events in the proposed API currently have the folowing cycle at runtime (the diagram illustrates navigating the dialog in the TaskDialogButton.Click event):

Caller                                          Events

TaskDialog.ShowDialog();
       ↓
    (Calls TaskDialogIndirect())
       ────────────>
                    ↓      (Window handle available now)
                Callback(TDN_CREATED) ─────────> TaskDialogPage[1].Created
                    ↓      (Window becomes visible)
                    ↓
                  (...)
                    ↓
                Callback(TDN_BUTTON_CLICKED) ──> TaskDialogButton.Click
                                                   ↓
                      TaskDialogPage.Navigate() <───────
                              ↓
                              ─────────────────> TaskDialogPage[1].Destroyed
                              ↓
                    <──────────
                    ↓
                Callback(TDN_NAVIGATED) ───────> TaskDialogPage[2].Created
                    ↓
                  (...)
                    ↓
                Callback(TDN_BUTTON_CLICKED) ──> TaskDialogButton.Click
                    ↓      (Window closes; Dialog no longer navigable as it set a result button)
                    ↓
                Callback(TDN_DESTROYED) ───────> TaskDialogPage[2].Destroyed
                    ↓      (Window handle no longer available)
       <────────────
       (TaskDialogIndirect() returns)
       ↓
(TaskDialog.ShowDialog() returns)

Implementation

The proposed API is implemented with PR #1133.

API Updates

  • Removed property TaskDialogContents.DoNotSetForeground as it doesn't seem to have an effect
  • Removed base classes TaskDialogControlCollection and TaskDialogButtonCollection
  • TaskDialogCustomButtonCollection and TaskDialogRadioButtonCollection inherit from Collection instead of KeyedCollection
  • Added an implicit cast operator from TaskDialogButtons to TaskDialogCommonButtonCollection
  • Removed property ResultVerificationFlagChanged from TaskDialog
  • Renamed property ExpandedByDefault to Expanded (TaskDialogExpander) (so its value will be updated when the user clicks the expando button)
  • Removed non-predefined icons (that were used from imageres.dll)
  • Class TaskDialog extends System.ComponentsModel.Component (and is disposable)
  • Added Tag property to TaskDialogControl
  • Class TaskDialogCommonButton now has a default constructor (like other control classes)
  • Renamed properties/events (e.g. MainInstruction -> Instruction, Content -> Text, ButtonClicked -> Click)
  • Properties and events of TaskDialogRadioButton and TaskDialogVerificationCheckbox has been aligned with WinForms concepts (property Checked, event CheckedChanged).
  • Renamed class TaskDialogVerificationCheckbox to TaskDialogCheckBox (along with properties)
  • Created struct TaskDialogProgressBarRange to be used for the TaskDialogProgressBar.Range property instead of (int min, int max) for better designer support
  • Restored property TaskDialogContents.DoNotSetForeground as it is actually working.
  • Removed TaskDialogBooleanStatusEventArgs.
  • Remaned TaskDialogProgressBarState enum value MarqueeDisabled to MarqueePaused
  • Made class TaskDialogControl abstract
  • Renamend enum value TaskDialogIcon.Stop to Error
  • Removed the TaskDialogProgressBar.Range property (along with the TaskDialogProgressBarRange struct) and instead added properties Minimum and Maximum directly on the TaskDialogProgressBar and also renamed property Position to Value, to simplify the API and align with the WinForms ProgressBar
  • Removed the WPF types
  • Extracted the footer-specific properties on TaskDialogContents (FooterText, FooterIcon, FooterIconHandle) into their own TaskDialogFooter class. The reasoning for this is that a separate dialog area is added for the footer when it is used (as shown in the below image), similar to the expander (and it reduces the number of properties in TaskDialogContents).
    Also, when you intially don't create a footer when showing the task dialog, you cannot later add one by updating the FooterText property, similar to the Text property of the Expander (which is different from the other text properties like TaskDialogContents.Text and Instruction that can be added later).
    A separate TaskDialogFooter class that inherits from TaskDialogControl can thus share the behavior with taskDialogExpander to throw an InvalidOperationException when trying to update its Text property but the control wasn't created because it was initially null (or string.Empty).
    taskdialog-footer
  • Renamed events TaskDialog.Closing to Closed and TaskDialogContents.Destroying to Destroyed, and added a new TaskDialog.Closing event that is called directly after a TaskDialogButton.Click event if the button would close the dialog, and it allows to cancel the close (similar to Form.FormClosing event in WinForms) - see this comment (Option B).
  • Renamed property TaskDialogExpander.ExpandoButtonClicked to ExpandedChanged
  • Renamed class TaskDialogContents to TaskDialogPage and property TaskDialog.CurrentContents to TaskDialog.Page. This is because the documentation also talks about "navigating to a new page" - for example see Multi-page dialog boxes.
  • Removed the TimerTick event on TaskDialogPage:
    This event represents the TDN_TIMER notification that is called every 200 ms if the TDF_CALLBACK_TIMER flag was set in the task dialog config. The previous implementation specified the flag if the event handler has been set (like the implementation in the Windows API Code Pack did), but this means you could not add an event handler to the TimerTick event after the dialog is displayed/navigated. Also, the interval of 200 is fixed (and a user might get the impression that the dialog can only be updated every 200 ms, which is not the case).
    Instead, the user can use one of the already existing UI timer implementations like System.Windows.Forms.Timer. Both the Task Dialog timer and the WinForms Timer use a Windows timer (WM_TIMER messages), so using the WinForms timer should have the same behavior as the TaskDialog timer but with more flexibility.
  • Moved property StartupLocation from TaskDialogPage to TaskDialog because it only has an effect when showing the dialog (but not when navigating it) and therefore isn't related to the page (which represents the contents of the dialog).
  • Added events TaskDialog.Activated and TaskDialog.Deactivated. Edit: Removed these again because of an unresolved issue when closing the dialog.
  • Added event TaskDialog.Shown (similar to Form.Shown).
  • Renamed class TaskDialogCommonButton to TaskDialogStandardButton (along with collections and property names).
  • Moved property TaskDialogPage.DoNotSetForeground to TaskDialog because it only has an effect when showing the dialog, but not when navigating it.
  • Unified mutually exclusive properties Icon+IconHandle on TaskDialogPage and TaskDialogFooter into a single Icon property use subclasses to differentiate between icon types (see Windows Task Dialog #146 (comment)).
    This should avoid confusion about having two mutually exclusive properties (and it allows to initially not showing an icon but then updating it to one using a handle (without using navigation)).
    Additionally, it will allow us in the future to add an additional icon type that represents integer/string resource icons (e.g. from imageres.dll or the application's executable), which could also be shown using a colored bar (which is not possible when using a handle).
  • Renamed property TaskDialogPage.CommandLinkMode to CustomButtonStyle (along with the enum).
  • TaskDialog no longer inherits from System.ComponentModel.Component which was used for trying to implement designer support, but that would require additional work. It be revisited for a future version.
  • Renamed event TaskDialogPage.Help to HelpRequest (and method OnHelp to OnHelpRequest) as discussed in Implement the Windows Task Dialog #1133.
  • Renamed property TaskDialog.DoNotSetForeground to TaskDialog.SetToForeground (the default value is still false), as per the feedback in Implement the Windows Task Dialog #1133.
  • Enabled nullable reference types.
  • Made events nullable (see Make all event types nullable coreclr#25752).
  • API Review Feedback:
    • Renamed method group TaskDialog.Show to ShowDialog.
    • Renamed property TaskDialogPage.Instruction to MainInstruction (same with parameter names for the static TaskDialog.Show methods).
    • Renamed property TaskDialogPage.Title to Caption (same with parameter names for the static TaskDialog.Show methods).
    • Removed class TaskDialogButtonClickedEventArgs and instead added boolean property TaskDialogButton.ShouldCloseDialog that allows to specify whether clicking the button should close the task dialog.
    • Removed types TaskDialogStandardIcon and TaskDialogIconHandle, and instead added static fields on TaskDialogIcon for the standard icons, and added a constructor taking an icon or icon handle.
  • Added an int indexer to TaskDialogStandardButtonCollection to avoid an overload resolution in the C# compiler for expressions like page.StandardButtons[0]. See Implement the Windows Task Dialog #1133 (comment)
  • Changes from Streamlime footer control kpreisser/winforms#1:
    • Added implicit cast operator from string to TaskDialogFooter.
  • Replaced property TaskDialogExpander.ExpandFooterArea with Position (using enum type TaskDialogExpanderPosition).
  • Added properties TaskDialogPage.BoundDialog and TaskDialogControl.BoundPage, so that it is possible e.g. to access the current TaskDialog instance in a button click handler. See discussion here.
  • Renamed icons SecurityShield, SecurityShieldBlueBar, SecurityShieldGrayBar, SecurityWarningYellowBar, SecurityErrorRedBar, SecuritySuccessGreenBar to Shield, ShieldBlueBar, ShieldGrayBar, ShieldWarningYellowBar, ShieldErrorRedBar, ShieldSuccessGreenBar; as the term "security" would imply that such icons will/must be used for security purposes.
  • Changes from Tweak api kpreisser/winforms#3:
    • Renamed TaskDialogPage.CanBeMinimized to AllowMinimize.
    • Renamed TaskDialogButton.ShouldCloseDialog to AllowCloseDialog.
    • Add optional parameters to the TaskDialogStandardButton and TaskDialogCustomButton constructors and to the TaskDialogStandardButtonCollection.Add and TaskDialogCustomButtonCollection.Add methods.
  • Simplified type of event TaskDialogButton.Click from EventHandler<EventArgs> to EventHandler.
  • Removed hyperlink functionality for now - see Disable TaskDialog "hyperlink" functionality kpreisser/winforms#4.
  • Refactored Button API - see Rework task dialog button API kpreisser/winforms#12
  • Streamlined single/multipage API - see Streamline single/multipage API kpreisser/winforms#14
  • Move instance property StartupLocation to a parameter of ShowDialog() - see Set dialog properties via ShowDialog kpreisser/winforms#16
  • Allow to create TaskDialogIcon from a Bitmap - see Simplify setting dialog icon from bitmaps kpreisser/winforms#15
  • Renamed property TaskDialogPage.MainInstruction to Heading - see [minor] MainInstruction is confusing kpreisser/winforms#6
  • Renamed class TaskDialogFooter (and corresponding properties) to TaskDialogFootnote - see Consider renaming Footer to Footnote kpreisser/winforms#8
  • Renamed TaskDialogStartupLocation.CenterParent to CenterOwner
  • Removed method TaskDialogCheckBox.Focus()
  • Renamed property TaskDialogButton.ElevationRequired to ShowShieldIcon
  • Renamed class TaskDialogCheckBox (and corresponding properties) to `TaskDialogVerificationCheckBox´ - see Streamlime verification control kpreisser/winforms#18
  • Removed property TaskDialogPage.Width

Possible API TODOs

  • Maybe rename TaskDialogProgressBarState to TaskDialogProgressBarStyle
  • Maybe add property Tag on TaskDialogPage (which is already present on TaskDialogControl)
  • Check how method ShowDialog() should behave. Currently, it either shows the dialog modal (when specifying an owner) or non-modal, but in both cases the method does not return until the dialog has closed (similar to Form.ShowDialog()), which is the behavior of the native TaskDialogIndirect function.
    This is the same as with MessageBox.Show(); however, the MessageBox automatically uses the current foreground window as owner when you don't specify the owner. For the Task Dialog however, it definitely should be possible to show it non-modal.
    Note that this means you can show multiple non-modal dialogs at once, but each open dialog will add a TaskDialog.Show() entry in the call stack.

API Usage Notes

  • Because some parts of the Task Dialog can be updated while it is shown (while others cannot), properties that cannot be updated while the dialog is shown will throw an InvalidOperationException (this was also the behavior of the task dialog implementation in the Windows API Code Pack).
    For example, radio buttons cannot be unselected while the dialog is shown (but they can be selected). This means that assigning false to the TaskDialogRadioButton.Checked property (while the radio button is shown in a task dialog) will throw.
  • The button order in the TaskDialogButtonCollection does not necessarily reflect the order in which the task dialog actually displays the buttons (since common buttons are specified by flags in the TASKDIALOGCONFIG structure, whereas custom buttons are stored in an array).
    The native task dialog displays buttons from the collection in the following order:
    1. Custom Buttons/Command Links in their relative order from the collection
    2. Standard Buttons in an OS-defined order:
      1. OK
      2. Yes
      3. No
      4. Abort
      5. Retry
      6. Cancel
      7. Ignore
      8. TryAgain
      9. Continue
      10. Close
      11. Help
  • TaskDialog.ShowDialog() can return a TaskDialogButton which was not added to the dialog in the following cases (which results from the native task dialog implementation):
    • No button has been added to the dialog, in which case it automatically gets an OK button, and so the TaskDialogButton.OK button is returned when the user clicks it.
    • No Cancel button has been added to the dialog, but the dialog is programmatically closed by calling the Close() method, or TaskDialogPage.AllowCancel has been set and the dialog is closed by the user by pressing ESC or Alt+F4 or clicking the X button in the title bar. In these cases the TaskDialogButton.Cancel button will be returned.
      This can also happen when a non-modal task dialog is shown but the main window of the app is closed, in which case the task dialog is also closed and returns a Cancel result.
  • It is possible to use a color bar with a different icon (one of the predefined icons or an icon resource from imageres.dll), by initially specifying one of the Shield...Bar icons, and when the dialog is shown, update it to a different icon:
    taskdialog-colorbar-customicon
    However, it isn't possible to use a color bar with a icon handle, because after showing the dialog you can only update the icon member that was initially used to show a dialog, and specifying a color bar requires to use the non-handle icon member.
    This means currently you can only use one of the standard icons with a color bar, but in a later version we could add support for showing icons from integer/string resources of DLLs/EXEs (e.g. from imageres.dll) (by specifying the hInstance field of the native TASKDIALOGCONFIG struct), which would then allow you to show a custom icon with a colored bar.
@merriemcgaw merriemcgaw added enhancement Product code improvement that does NOT require public API changes/additions design-discussion Ongoing discussion about design without consensus labels Dec 4, 2018
@KlausLoeffelmann
Copy link
Member

KlausLoeffelmann commented Dec 5, 2018

I like this idea in general. What I'd like to suggest first though is an additional and more detailed concept as a separate issue which depicts the rationale/business value, public surface area (Class, Methods, Properties, etc.), small code samples, maybe along with a link to a repo fork which would include the work that you already did (probably with way fewer partial classes/code files ;-) ). Something I could put the label api-suggestion on.

Thoughts?

@wjk
Copy link
Contributor

wjk commented Dec 5, 2018

@KlausLoeffelmann I might want to take a stab at integrating @kpreisser's library into WinForms core. I'm not too good at writing formal rationales, though. I hope this won't be a problem. 😄

One other question before I begin, however. Where should I put the native types when I start work on this branch? I know WinForms uses three separate classes: NativeMethods, SafeNativeMethods, and UnsafeNativeMethods. I never understood why one would want three different classes for holding P/Invoke code, or what the criteria are for choosing between them. (Some legacy reason involving CAS, no doubt.) CoreFX uses a partial Interop class in the root namespace that is subdivided by the DLL being called into using nested partial classes. Moving one approach into another is of course way outside the scope of this issue; I just was wondering what the design guidance is. Thanks!

@karelz karelz added api-suggestion (1) Early API idea and discussion, it is NOT ready for implementation and removed enhancement Product code improvement that does NOT require public API changes/additions labels Dec 5, 2018
@karelz
Copy link
Member

karelz commented Dec 5, 2018

@KlausLoeffelmann BTW: I suggest to put api-suggestion on any issue which will most likely require API process. api-suggestions are enhancements which require additional process of API review and explicit approval on the API shape.

@karelz karelz added this to the Future milestone Dec 5, 2018
@kpreisser
Copy link
Contributor Author

kpreisser commented Dec 5, 2018

Hi @KlausLoeffelmann and @wjk,
thanks, that's good to hear!

I would be happy to contribute my implementation of the Task Dialog, but note that there are some areas in the API which I think are not yet ideal from an OOP point of view. Also, I implemented some unofficial features which probably shouldn't be in the public API.

I have created the branch apiProposal to start a clean-up (edit: this is now in the master branch).

The workflow that I used in my Task Dialog implementation is something like this:

  • Create a TaskDialog instance
  • Set properties on the instance, add controls like custom buttons
  • Show() the dialog (making it "active"); then while it is displayed:
    • Set properties, then call Update() to update the currently displayed elements to the new property values
    • Call Reset(), set new property values, then call Navigate() to reconstruct the dialog from the current properties
  • After the dialog is closed, retrieve the Result properties.

The TaskDialog supports both standard/common buttons (like Yes, No, Cancel etc) that are supplied as flags to the native API, and custom buttons where you can set your own string, which are supplied as array in the native API.
In my library, custom buttons are represented by ITaskDialogCustomButton instances where you can set properties or add a Click event handler. E.g. you can set the Enabled and ElevationRequired properties either before displaying/navigating the dialog, or while it is shown (in which case the displayed dialog is updated).

However, common buttons are currently not represented as objects, as I wasn't sure how to ideally do that.

For example, you can use custom buttons like this:

    // Create and add the buttons.
    var button1 = taskDialog.AddCustomButton("My Button 1");
    var button2 = taskDialog.AddCustomButton("My Button 2");

    // Set properties.
    button1.Enabled = false;
    button2.ElevationRequired = true;

    // Add click event handlers.
    button1.ButtonClicked += (sender, e) => { /* ... */ };
    button2.ButtonClicked += (sender, e) => { /* ... */ };

    // While the dialog is shown: Click the button.
    button1.Click();

However, when using standard/common buttons, you have to use the "raw" methods (and you cannot set the Enabled and ElevationRequired properties before the dialog is shown):

    // Specify the buttons.
    taskDialog.CommonButtons = TaskDialogButtons.Yes | TaskDialogButtons.No;

    // Cannot set Enabled/ElevationRequired until the dialog is shown, so
    // need to use the Opened event:
    taskDialog.Opened += (sender, e) =>
    {
        taskDialog.SetCommonButtonEnabled(TaskDialogResult.Yes, false);
        taskDialog.SetCommonButtonElevationRequired(TaskDialogResult.No, true);
    };

    // Add click event handlers.
    taskDialog.CommonButtonClicked += (sender, e) =>
    {
        if (e.Button == TaskDialogResult.Yes) { /* ... */ }
        else if (e.Button == TaskDialogResult.No) {  /* ... */ }
    };

    // While the dialog is shown: Click the button.
    taskDialog.ClickCommonButton(TaskDialogResult.Yes);

I think this should be changed to also represent standard/common buttons as objects, but I'm not sure how to do that when keeping the CommonButtons flags property.

One idea that I had was to create a dictionary-like class that maps a TaskDialogResult to an ITaskDialogButton instance, and use an implicit cast operator from TaskDialogButtons, so you could so something like this:

    // Specify the buttons.
    taskDialog.CommonButtons = TaskDialogButtons.Yes | TaskDialogButtons.No;

    // We now can retrieve the button instances.
    var buttonYes = taskDialog.CommonButtons[TaskDialogResult.Yes];
    var buttonNo = taskDialog.CommonButtons[TaskDialogResult.No];

    // or maybe:
    //var buttonYes = taskDialog.CommonButtons.Add(TaskDialogResult.Yes);
    //var buttonNo = taskDialog.CommonButtons.Add(TaskDialogResult.No);

    // Set properties.
    buttonYes.Enabled = false;
    buttonNo.ElevationRequired = true;

    // Add event handler
    buttonYes.ButtonClicked += (sender, e) => { /* ... */ };
    buttonNo.ButtonClicked += (sender, e) => { /* ... */ };

    // While the dialog is shown: Click the button.
    buttonYes.Click();

(this would require refactoring so that the TaskDialog can accept button instances that were created outside of the class (whereas currently you can only add custom buttons by calling TaskDialog.AddCustomButton().)

Also, the properties and methods related to a progress bar should probably be refactored into a separate ProgressBar class (because currently you have to set either property ShowProgressBar or ShowMarqueeProgressBar to true before displaying the dialog, and then call one of SwitchProgressBarMode(), SetProgressBarMarquee(), SetProgressBarState(), SetProgressBarRange(), SetProgressBarPos() methods to update it while the dialog is shown.

What do you think?

Thanks!

@wjk
Copy link
Contributor

wjk commented Dec 5, 2018

Sounds great, modulo the following:

  • I would greatly prefer the second approach to adding common buttons (var buttonYes = taskDialog.CommonButtons.Add(TaskDialogResult.Yes);).
  • Don't care about the removal of the "unofficial" TaskDialog icons (Lock, Defrag, et al), as I can easily accomplish the same thing by setting the MainIconHandle property. However, please don't get rid of the icons that provide the colored status bars. Those are way cool, and no other TaskDialog implementation I've seen exposes this functionality.

@GSPP
Copy link

GSPP commented Dec 5, 2018

It seems like your .NET solution (which is nice) is a fairly literal translation of the C++ API. The C++ API is a bit nasty, though. I think we can make the .NET API nicer and abstract away certain nastiness. For example, why does the dialog need to be opened to be configured? I think the native API likely does it this way because it's the only reasonable way to do it in native code. In object based .NET code we are less constrained. We don't have lifetimes and memory management issues for example. We don't need the ITaskDialogCustomButton interface just to have callbacks.

Some more concrete points:

  1. There should be a Show and ShowDialog method.
  2. Can the ShowDialog method return some kind of result?
  3. It should be possible to determine how the dialog was closed (e.g. using the Window X button).
  4. Is there a need for ITaskDialogCustomButton at all? Maybe just new TaskDialogButton() { Text = ..., Clicked += ... }.
  5. It is not idiomatic for WinForms to require a Update method to apply property values. Is it not possibly the apply changes immediately?
  6. That said I wonder how to build the Navigate model without a delayed update. What does navigate do actually? From the Windows docs it seems that it just replaces the old contents with new contents? Is this not equivalent to just setting all properties to new desired values? I admit I am not too experienced with task dialogs so I might misunderstand this concept.

@kpreisser
Copy link
Contributor Author

kpreisser commented Dec 6, 2018

Hi @wjk,

thanks! Yes, the second approach seems better, but I think it might still be good to also support assigning the flags, so that you can assign them in the object initializer (if you just want to add standard buttons but don't need to customize them), just as you can specify button flags in the static Show() methods.

Thinking about it, it probably should also be possible to manually create a TaskDialogButton instance (not using the interface, which is also what @GSPP described) and then add it to the collection:

var button = new TaskDialogCustomButton("Hello World") {
    ElevationRequired = True
}
taskDialog.CustomButtons.Add(button)

var standardButton = new TaskDialogCommonButton(TaskDialogResult.Yes);
taskDialog.CommonButtons.Add(standardButton);

I also like the colored status bar icons, but I have to admit I didn't find the values myself (I think I found them on pinvoke.net).

Hi @GSPP,
thank you for your feedback, much appreciated!

You are right, I simply looked at the native Windows API (TaskDialogIndirect) and then built a .NET API around it. I agree it has room for improvement, to be more .NET-like.

For example, why does the dialog need to be opened to be configured? I think the native API likely does it this way because it's the only reasonable way to do it in native code. In object based .NET code we are less constrained.

Yes, it should be possible to do all the initialization before actually opening the dialog. This is currently the case with custom (and radio) buttons, where you can set properties like Enabled, and the TaskDialog automatically applies them when it is shown or navigated.

  1. There should be a Show and ShowDialog method.

Can you elaborate on these? I know from System.Windows.Forms.Form that it has a Show() method that opens a non-modal window and returns immediately; and it has a ShowDialog() method that shows a modal window, and only returns after the window is closed.

Unfortunately, the native Task Dialog API only has one implementation where the method does not return until the dialog is closed, even when it is shown non-modal. I also described this behavior in the README (Non-modal dialog).

This means you can open multiple non-modal Task Dialogs that the same time, but each dialog will cause a new Show() call to appear on the call stack.

  1. Can the ShowDialog method return some kind of result?

This should be possible. Currently, the static Show() methods return a TaskDialogResult because they only allow to specify standard ("common") buttons. The instance Show() methods currently don't return a result because it can either be a standard/common button or a custom button.

When we have classes that represent these buttons (e.g. TaskDialogCommonButton and TaskDialogCustomButton that inherit from TaskDialogButton), it might be possible to return the base class TaskDialogButton, but this means the user has to check if it is an instance of TaskDialogCommonButton or TaskDialogCustomButton, which might not be elegant (although with C# 8.0 pattern matching it might get better).

  1. It should be possible to determine how the dialog was closed (e.g. using the Window X button).

Unfortunately, with the native API it is not possible to distinguish a close through clicking on the "Cancel" button from a close caused by clicking the Window X button, as in both cases the result returned by TaskDialogIndirect is the "Cancel" result. It is however possible to check if the dialog was closed programatically (by calling the Close() method) as this can be tracked by the C# code.

  1. Is there a need for ITaskDialogCustomButton at all? Maybe just new TaskDialogButton() { Text = ..., Clicked += ... }.

I initially used these interfaces so that I could make nested classes within TaskDialog (so they can access private methods and properties), but I don't think these are necessary (the corresponding private methods can be made internal), so I can remove them and just use a TaskDialogCustomButton.

Do you mean to use only a single class to represent both a standard/common button, and a custom button?

  1. It is not idiomatic for WinForms to require a Update method to apply property values. Is it not possibly the apply changes immediately?

I also thought about this, as it is not intuitive that you have to call Update() to actually update some of the GUI parts while the dialog is shown.

It should be possible to apply the changes immediately when you set the property and the dialog is shown. This has a small disadvantage however, because if you want to navigate the dialog (and therefore change the properties), the displayed Task Dialog would (unnecessarily) update its GUI even though shortly after that it will be navigated (which means it will completely reconstruct the GUI elements).

From a performance view, I think this is negligible because navigation is much more expensive than a simple update. The only visible effect would be that if you change the property, then wait some time (meaning the GUI event loop continues) and then navigate the dialog, you would see that the Task Dialog GUI is already changing before the navigation, but I think the user should be able to expect this, and only change the properties directly before navigation.

Or maybe the Reset() method (that resets all properties to their default values, which can be used for navigation because then you can set the properties as if you created a new TaskDialog instance) could have the effect that it not only resets the properties, but also disables auto-update until the dialog is navigated (or closed and shown again).

6. That said I wonder how to build the Navigate model without a delayed update. What does navigate do actually? From the Windows docs it seems that it just replaces the old contents with new contents? Is this not equivalent to just setting all properties to new desired values?

Navigation basically destroys all currently displayed GUI within the Task Dialog window, and then creates a new GUI from the supplied TASKDIALOGCONFIG structure (just as if you show a new Task Dialog and pass the structure to the TaskDialogIndirect function). This also means changes to the current state are lost (e.g. the focus of a control, the state of radio buttons or the checkbox, the state of the progress bar, collapsed/expanded state etc.).

Navigation is also resource-intensive and much slower than a simple "Update". However, with an Update you can only update following GUI elements/properties:

  • Main Instruction
  • Content
  • Footer
  • Expanded information
  • Main Icon
  • Footer Icon
    and additionally (not by calling the current Update() method but setting the corresponding properties, or (in the current version) call the methods):
  • Enabled state of buttons/radio buttons
  • ElevationRequired state of buttons
  • Checked state of the verification checkbox
  • Progress bar state

E.g. you cannot update the text of custom buttons, this is only possible using navigation.

My model that I implemented for navigation is like:

  • Create a TaskDialog instance
  • Set properties for the initial dialog
  • Call Show()
    then while Show() is still in the call stack:
  • Call Reset()
  • Set properties for the navigated dialog
  • Call Navigate()

This is similar to the native API, where you create a TASKDIALOGCONFIG structure with values for the initial dialog and then call TaskDialogIndirect(), and while that function is in the call stack, create a new TASKDIALOGCONFIG structure and then send the TDM_NAVIGATE message to do navigation.

Thank you! And sorry for the long text... 😇

@GSPP
Copy link

GSPP commented Dec 6, 2018

Another issue is that the parent window must be specified. MessageBox has the same issue. I believe that code like MessageBox.Show(title, text) is wrong. The first parameter must be the parent window. If this is not done then the message box can have weird behavior such as having the wrong parent and remaining locked behind another window. I do not fully remember what the problems were but I have hit this issue before.

Regarding 1, yes this is about modal/non-modal. Both should be possible. For a MessageBox a non-modal version does not seem to have much of a use case, but task dialogs are like real dialogs that can be controlled after having been opened. Many applications will want the modal version, though, and just get a result from the dialog.

Regarding 3, I guess I retract this suggestion 😄

Do you mean to use only a single class to represent both a standard/common button, and a custom button?

Yes, that was my idea but I misunderstood the distinction between the two. They must be two distinct concepts. But custom buttons don't need any inheritance. They always have the same structure: A text string and a click event.

Thank you for describing the navigation concept. Maybe it can work like this: The task dialog contents are described by a separate class TaskDialogContents. A TaskDialog has a CurrentContents property. Individual elements can be modified through dlg.CurrentContents.SomeProperty = x;. This would immediately apply. But you can also navigate by creating an entirely new TaskDialogContents instance and assigning it to dlg.CurrentContents. That way updates are immediate and it's a nice, logical object model.

One more point: Can the dialog be hidden and reopened? That would be nice to make it consistent with any other Form. The motivating use case would be a progress dialog with a cancel button. The user can hide the progress dialog and minimize it to the systray. Double-clicking the systray icon reopens the progress dialog.

E.g. you cannot update the text of custom buttons, this is only possible using navigation.

This unfortunately means that we cannot abstract this away in the managed API. We could disallow button text changes or automatically "navigate" on text change.


I think task dialogs are going to be a very popular feature. Your work is appreciated! Once we have shipped the first version we can never again change the API. We need to get it exactly right.

@kpreisser
Copy link
Contributor Author

kpreisser commented Dec 6, 2018

Hi @GSPP,

thanks for your reply!

Another issue is that the parent window must be specified. MessageBox has the same issue.

Note that the Task Dialog explicitely supports not using a parent window (documentation of the hwndParent parameter):

Handle to the parent window. This member can be NULL.

When not specifying a parent window, the Task Dialog will be displayed as non-modal window (so it does not lock/hide other windows). This is also the behavior with a native MessageBox (if you directly call the MessageBoxW API), whereas the MessageBox implemented in WinForms and WPF seem to have a logic to detect the current window and specify that as parent (which I think is the issue you mentioned).

Regarding 1, yes this is about modal/non-modal. Both should be possible. For a MessageBox a non-modal version does not seem to have much of a use case, but task dialogs are like real dialogs that can be controlled after having been opened. Many applications will want the modal version, though, and just get a result from the dialog.

Yes, displaying a non-modal Task Dialog is possible by specifying null as owner window as mentioned above, but the native API TaskDialogIndirect always has the behavior of not returning while the dialog is displayed, regardless of whether the Task Dialog is shown modal or non-modal. So I'm not sure if we can implement a Show() method that is different from a ShowDialog() method.

Yes, that was my idea but I misunderstood the distinction between the two. They must be two distinct concepts. But custom buttons don't need any inheritance. They always have the same structure: A text string and a click event.

Note that in a Task Dialog, there are three different kinds of "buttons":

  • Common buttons (like Yes, No, Ok, Cancel etc.): These will always be displayed as normal buttons and will display a localized text from the OS.
  • Custom Buttons: Like common buttons but you can specifiy you own text, and they are either displayed like the common buttons, or as "command links" if you specify the UseCommandLinks or UseNoIconCommandLinks flag.
  • Radio Buttons: Shown as radio buttons where one can be selected; you can also specify your own text for them.

Common buttons are initially specified by a flags field (enum TaskDialogButtons in my implementation) when showing the dialog, and are later referenced (in events, when changing their properties or in the result variable) using a dialog result enum (TaskDialogResult).

Both custom buttons and radio buttons are initially specified by a structure containing a custom ID and the display text, and are later identified by that ID.

Common buttons, custom buttons and radio buttons have the following members (I'm using the names from my current implementation):

  • Property Enabled: To enable/disable the button (native API: TDM_ENABLE_BUTTON, TDM_ENABLE_RADIO_BUTTON)
  • Method Click(): Click the button/select the radio button (native API: TDM_CLICK_BUTTON, TDM_CLICK_RADIO_BUTTON)
  • Event Clicked (ButtonClicked): Raised when the button has been clicked or the radio button has been selected (by the user or by calling the Click() method) (native API: TDN_BUTTON_CLICKED, TDN_RADIO_BUTTON_CLICKED)

Custom buttons and radio buttons additionally have a Text property specifying the display text (in the structure as mentioned above). (Native field: pszButtonText)
(A common button, when represented by a class, would then have a TaskDialogResult property instead of the Text property.)

Common buttons and custom buttons additionally have the property ElevationRequired which specifies if an UAC shield symbol should be shown for the button (native API: TDM_SET_BUTTON_ELEVATION_REQUIRED_STATE).

When clicking a common button or a custom button, the dialog will close by default, but the close can be canceled in the event handler (this does not apply for radio buttons); except for the Help button where by default the Help event will be raised but the dialog will stay open.

Therefore, I was thinking to specify a base button class, and then have subclasses for these three kind of buttons.

Maybe it can work like this: The task dialog contents are described by a separate class TaskDialogContents. A TaskDialog has a CurrentContents property. Individual elements can be modified through dlg.CurrentContents.SomeProperty = x;. This would immediately apply. But you can also navigate by creating an entirely new TaskDialogContents instance and assigning it to dlg.CurrentContents. That way updates are immediate and it's a nice, logical object model.

Although this has a small disadvantage of adding an additional indirection to the TaskDialog when you want to set its properties, I think I like this idea, as it removes the need for the Reset() method, it solves the problem of unnecessarily updating dialog elements when you actually want to navigate it, and it even allows you to pre-declare different TaskDialogContents instances and navigate a dialog back and forth with them by simply setting the CurrentContents property.
(However, as for naming, note that the term Content also specifies a part of the Task Dialog).

One more point: Can the dialog be hidden and reopened? That would be nice to make it consistent with any other Form. The motivating use case would be a progress dialog with a cancel button. The user can hide the progress dialog and minimize it to the systray. Double-clicking the systray icon reopens the progress dialog.

I think the Task Dialog can not be hidden using the available Task Dialog messages, but it should be possible by using APIs like ShowWindow() to hide and show an existing Task Dialog.
Note that you can also specify the flag CanBeMinimized to show a minimize button in the Task Dialog's title bar (if the dialog is shown non-modal), so that the user can minimize it to the taskbar.

This unfortunately means that we cannot abstract this away in the managed API. We could disallow button text changes or automatically "navigate" on text change.

I would prefer to disallow text changes while the buttons are part of a currently displayed dialog, and use navigation only when actually setting the CurrentContents property to a different TaskDialogContents instance.

When I have some free time, I can try to implement the mentioned changes (TaskDialogContents) to see how they will behave.

Thank you!

@GSPP
Copy link

GSPP commented Dec 7, 2018

Great!

Therefore, I was thinking to specify a base button class, and then have subclasses for these three kind of buttons.

I see. These would be framework provided derived classes and there would be no option to implement your own. These buttons should then not use an interface because that can be implemented by user code. It should be an abstract base class with a private constructor and the derived classes sealed. Inheritance is used as a discriminated union type here. It's not for Liskov substitution.

Regarding TaskDialogContents, it must be ensured that each instance is only bound to a single TaskDialog at a time. While bound the two become connected and while unbound TaskDialogContents just a DTO.

@jnm2
Copy link
Contributor

jnm2 commented Dec 7, 2018

Custom buttons and radio buttons additionally have a Text property specifying the display text (in the structure as mentioned above). (Native field: pszButtonText)

My implementation also has a CommandLinkDescription property for custom buttons because when UseCommandLinks is true, up to two lines of text in pszButtonText are used. This might be nicer as an API than requiring the user to know about concatenating text + '\n' + commandLinkDescription to get what they want.

@kpreisser
Copy link
Contributor Author

kpreisser commented Dec 9, 2018

I have now done the changes in branch apiProposal master.

@jnm2 Thanks for your suggestion! You are right, it is more user-friendly to have different properties so that the user doesn't need to concatenate the strings with an \n. I have implemented this in the TaskDialogCustomButton class by using a Text and DescriptionText property.

The main change that I have done is that I have extracted all properties (and events) that describe the task dialog's contents like MainInstruction, Content, Footer etc. into class TaskDialogContents. This class also also has events Created and Destroying that are raised when the GUI elements for the contents were created (when the dialog is shown, or has navigated) or destroyed (when the dialog is closed, or is about to navigate to a different TaskDialogContents).

The TaskDialog class now has a property CurrentContents where you can set a TaskDialogContents instance. When you set the property while the dialog is shown, it will do navigation (which means the Destroying event of the previous TaskDialogContents is called, then the dialog is reconstructed, and then the Created event of the new TaskDialogContents is called).

When you change updatable properties MainInstruction, Content, Footer or the Text of the TaskDialogExpander control (see below) while the contents is bound to a task dialog, the task dialog's GUI will be updated immediately (so you do no longer need to call TaskDialog.UpdateElements(...).
Otherwise (when the contents are not bound to a task dialog), nothing special will happen when setting the properties.

Regarding TaskDialogContents, it must be ensured that each instance is only bound to a single TaskDialog at a time. While bound the two become connected and while unbound TaskDialogContents just a DTO.

Yes, that's exactly what I had in mind. When the TaskDialog is shown (or navigates), the CurrentContents are bound to the TaskDialog instance, and you cannot bind the same TaskDialogContent instance to a different dialog at the same time. This also applies to all the TaskDialogControl instances that are present in the contents.

As long as the TaskDialogContents is bound to a TaskDialog, you cannot change properties that cannot be updated in the GUI (e.g. you cannot add or remove buttons or modify their text), but when you change properties that can be updated in the GUI, they will updated immediately (without having to call a separate method like UpdateElements()).


The next important change is that I have extracted properties, methods and events that belong to a specific task dialog control (like progress bar, check box, expander) into their own classes, to reduce clutter in the TaskDialogContents class and make it much more user-friendly (as this is now similar to the GUI classes in WinForms).
Also, this allows to completely pre-configure the GUI elements, without having to handle the Opened event and call methods that customize the GUI.

All the control classes inherit from TaskDialogControl.

For example, previously you had to use the following code to show an marquee progress bar in the Task Dialog (which is simply the translation of the native API):

    dialog.ShowMarqueeProgressBar = true;

    dialog.Opened += (s, e) =>
    {
        // Actually enable the marquee.
        dialog.SetProgressBarMarquee(true);
    };

    // or, for navigation, handle the "dialog.Navigated" event

Now you can use this code:

    dialogContents.ProgressBar = new TaskDialogProgressBar()
    {
        State = TaskDialogProgressBarState.Marquee
    };

Or, previously if you wanted to specify a verification checkbox that is initially checked, and later programatically uncheck it (while the dialog is displayed):

    dialog.VerificationText = "Checkbox Text";
    dialog.VerificationFlagCheckedByDefault = true;

    dialog.Show();

    // Later (while the dialog is shown): Uncheck the box
    dialog.ClickVerification(false);

Now:

    var checkbox = dialogContents.VerificationCheckbox = new TaskDialogVerificationCheckbox()
    {
        Text = "Checkbox Text",
        Checked = true
    };
            
    dialog.Show();

    // Later (while the dialog is shown): Uncheck the box
    checkbox.Checked = false;

I have also created the TaskDialogCommonButton class that represents a button created with one of the TaskDialogResult enum values (TODO: Maybe the enum should be renamed), so that both TaskDialogCommonButton and TaskDialogCustomButton inherit from class TaskDialogButton which has the properties, methods and events shared by these two types of buttons. So you also no longer need to call methods like dialog.ClickCommonButton(), dialog.SetCommonButtonElevationRequired() etc., but simply can set the properties in the TaskDialogCommonButton class.

Note that because the common button is now represented by its own class, it is no longer possible to specify the common buttons by a flags value like TaskDialogButtons.Ok | TaskDialogButtons.Yes | TaskDialogButtons.No when using the TaskDialogContents; instead, you have to add these buttons like custom buttons with e.g. contents.CommonButtons.Add(TaskDialog.Result.Yes). (However, for the simple static methods, it is still possible to specify the flags).

Note: Because radio buttons have a bit different semantics and event types, they just inherit from TaskDialogControl.

The TaskDialogContents class now has three collections: TaskDialogCommonButtonCollection, TaskDialogCustomButtonCollection and TaskDialogRadioButtonCollection where you can add these three button types.

There are now the following classes that represents controls (I have removed the interfaces):

TaskDialogControl
    TaskDialogButton
        TaskDialogCommonButton
        TaskDialogCustomButton
    TaskDialogRadioButton
    TaskDialogExpander
    TaskDialogProgressBar
    TaskDialogVerificationCheckbox

Note: I have also removed the properties DefaultButton, DefaultCustomButton and DefaultRadioButton and NoDefaultRadioButton.

Instead, to set the default common or custom button, you can simply set its Default property to true. To set the default radio button, you can simply set its Checked property to true (if no radio button is checked, none will be selected when the dialog is shown).

Also, when the Clicked event for the radio button occurs, the code will internally set its Checked property to true (and the one of all other radio buttons to false) so it's more easy for the developer to check the current states of the radio buttons.

This has one small disadvantage however: It's not possible to "uncheck" (clear the selection) of a radio button while the dialog is active. This is reflected in the code by throwing an InvalidOperationException when setting to Checked property to false while the radio button is bound.

Generally, I think with the recent commit the API is more user-friendly and .NET/OOP-like.

What do you think?
Thank you!


Edit: I had to remove the property ResultRadioButton from the TaskDialog class (and, for consistency and because Show() now returns the button, properties ResultCommonButton and ResultCustomButton) to solve a problem that can occur when using navigation within ButtonClicked event handler, but not setting CancelClose to true:

    var dialogContents = new TaskDialogContents()
    {
        Content = "Before navigation"
    };
    var dialog = new TaskDialog(dialogContents);

    var radioButton = dialogContents.RadioButtons.Add("My Radio 1");
    radioButton.Checked = true;

    var customButton = dialogContents.CustomButtons.Add("My Custom Button");
    customButton.ButtonClicked += (s, e) =>
    {
        // Create new contents and navigate the dialog, but do
        // NOT set e.CancelClose to true. This will cause the dialog to
        // close after this handler returns, but specifying the common
        // button as result even if is not present in the current dialog's
        // GUI (and therefore the current TaskDialogContent's button
        // collections) as the resulting button ID by TaskDialogIndirect().
        // Also, it will contain the ID of the selected radio button before
        // navigation, to which the dialog no longer has access.
        var newContents = new TaskDialogContents()
        {
            MainInstruction = "After navigation",
            Content = "Text",
            MainIcon = TaskDialogIcon.SecurityShieldGrayBar
        };
        dialog.CurrentContents = newContents;

        // Show a new dialog to delay the close of the outer dialog.
        TaskDialog.Show("Close Me");
    };

    var resultButton = dialog.Show();
    Console.WriteLine("Resulting Button: " + resultButton);

In this example, when clicking the custom button, the dialog navigates to a new GUI that doesn't have a custom button (and shows another non-modal task dialog to delay the return of the event handler). However, because the event handler doesn't set CancelClose, the main dialog will close after closing the inner dialog, and TaskDialogIndirect will return the button ID of the custom button as resulting button ID, which is no longer present in the Task Dialog's CurrentContents in its CustomButtons collection. The same would happen with the resulting radio button ID, which would no longer be present in the RadioButtons collection.

I fixed the first problem by caching the last button instance with its original ID when the ButtonClicked event handler didn't set CancelClose to true (as that means that button will be set as the result of the Task Dialog). However, as for the radio buttons, I think this would be more complex, so I removed the ResultRadioButton property, as the user can retrieve the Checked property of a radio button to see if it was checked.
For consistency, I also removed the ResultCommonButton and ResultCustomButton properties as the Show() method now returns the TaskDialogButton instance.

What is also strange, that when running this code a few times, then in some cases I get an AccessViolationException in the outer dialog's TaskDialogIndirect method (or other strange behavior occurs which seems to be caused by incorrect memory access), but to me this seems to be a problem in the native implementation of the dialog - if I set CancelClose to true in the ButtonClicked event (which is what you normally should do when navigating the dialog within the event handler), everything works without problems.

Maybe we should track that the dialog was navigated within a ButtonClick event handler, and in that case act as if CancelTrue was set to true to prevent closing the dialog as result of the event handler, to avoid such situations.

@GSPP
Copy link

GSPP commented Dec 10, 2018

Sounds very good to me.

Maybe we should create a few succinct demos of this new TaskDialog just to see how the API feels. Both simple and elaborate use cases. Once we ship this API it's frozen forever. You already posted a few code snippets but maybe this should be done in a more systematic way. The team can then better review the API.

Is there a way to get that native code error fixed? Clearly, this requires a Windows change. If you can create a repro this could be initiated by the team.

@gilfusion
Copy link

If I might interject a question or two... is the TaskDialog you're designing only meant to be used from code, or would it be possible to at least configure it's properties from the designer (once it's up and running on Core), like ColorDialog, OpenFileDialog, and friends? How would this design affect that use case?

@kpreisser
Copy link
Contributor Author

kpreisser commented Dec 10, 2018

Hi @GSPP,

good idea! I think I can edit my first post and provide the current public API surface along with a few real-world examples of the Task Dialog (e.g. the ones that we use in a commercial application and ones that I have seen on other applications) and their corresponding code for the current API state. I'm sure there still are a lot of TODOs/fine tuning for the API (especially the naming, because often I'm not sure how to name certain things).

Edit: I updated my first post to show example usages and the current public API.

About the native code error, I will need to check if it can be reproduced directly from a C++ application. However, even with that I don't think the native Windows implementation of the Task Dialog is likely to change in the near future, so I think the best way to fix is to track if the dialog was navigated from within a ButtonClicked event handler, and in that case always return S_FALSE to prevent the dialog from closing.

Hi @gilfusion,

thank you, that's a good question! My primary goal to date was to allow the API to be used from code, but we probably should check if we can add support to set its properties from the designer. Unfortunately, I only have little knowledge in that area, so I would appreciate if someone who knows how to do this can give hints what would need to be done in order to support designer scenarios.

Thank you!

@wjk
Copy link
Contributor

wjk commented Dec 10, 2018

@kpreisser For proper designer support, you would need to do the following:

  • Make the TaskDialog class inherit from System.ComponentModel.Component and annotate it with [ToolboxItem(true)]. This tells the designer that this control should appear in the toolbox.
  • Annotate the properties that should be editable in the designer with [Category("...")], [Description("...")], and [DefaultValue(...)] attributes. You might also want to annotate the class with the [ToolboxBitmap(typeof(TaskDialog), "...")], [DefaultProperty(nameof(...))], and [DefaultEvent(nameof(...))] attributes. These attributes provide more information to the user as to the meaning and values of the properties.
  • Annotate the TaskDialogContents class with [TypeConverter(typeof(System.ComponentModel.ExpandableObjectConverter))], and annotate the TaskDialog.Contents property with [DesignerSerializationVisibility(DesignerSerializationVisibility.Content)]. This will instruct the designer to let the user edit the properties on the TaskDialogContents class, while the TaskDialog class is selected in the form.

Hope this helps!

@jnm2
Copy link
Contributor

jnm2 commented Dec 11, 2018

[Browsable(false)] should also be specified for any properties (including get-only properties) which you don't want visible in the properties window.

@kpreisser
Copy link
Contributor Author

Hi @wjk and @jnm2,
thank you very much for your help! I will look into doing the necessary changes in the next few days.

@kpreisser
Copy link
Contributor Author

kpreisser commented Jan 3, 2019

Thanks to the help of @wjk and @jnm2, I was able to having the TaskDialog appear in the Windows Forms designer toolbar and provide basic designer support:
taskdialogproperties
commonbuttoncollectioneditor

Because the designer initially did not show the CheckBox, Expander, and ProgressBar contents (as they were null by default), I have changed the implementation to created default instances of these controls (which are not shown by default as their initial text is null and the initial progress bar state is None).

A minor issue currently is that while you can edit the button/radio button collections, the code is not generated correctly (adds the button to a new collection instead of the existing one):

new KPreisser.UI.TaskDialogCommonButtonCollection().Add(taskDialogCommonButton1);

Also, for TaskDialogProgressBar.Range I used a structure, but the designer tries to set each properties individually instead of setting the whole structure:

this.taskDialog1.CurrentContents.ProgressBar.Range.Maximum = 100;
this.taskDialog1.CurrentContents.ProgressBar.Range.Minimum = 0;

Edit: This is no longer relevant because the struct has been removed.

Another problem (which is probably more important) is that while the designer window shows events of the TaskDialogContents and its controls, you cannot double-click to add a new event handler (probably because the controls don't exist as variables in the form):
taskdialogevents

Additionally, as you can see in the screenshot above, when editing the button collections you cannot add event handlers (e.g. for the TaskDialogButton.Click event and for the TaskDialogRadioButton.CheckedChanged event).

Edit: Regarding the AccessViolationException that occurs when a button clicked handler returns S_OK after the dialog has navigated, I was not able to reproduce it using a C++ application, but I still think it is caused by wrong memory access in the underlying native implementation, probably because this is not an expected usage of the Task Dialog.
Therefore, I implemented navigation tracking using a Stack (where each element represents a stack frame of a button click handler), to ensure to always return S_FALSE from the button click handler when the dialog was navigated since the handler has been called.

@merriemcgaw
Copy link
Member

merriemcgaw commented Jan 25, 2019

Thank you for all the hard work on this! I'm going to have the team pick this up and review the implementation more closely. We'll circle back with questions and chat with you about anything we think needs to be polished up before we get to the formal API review with the review board. I'm excited though, this is a great addition!

Edited to add: I moved the milestone to 3.0 because I think we can get this reviewed and approved for 3.0.

@merriemcgaw merriemcgaw modified the milestones: Future, 3.0 Jan 25, 2019
@kpreisser
Copy link
Contributor Author

kpreisser commented Jan 25, 2019

Hi @merriemcgaw,

thank you very much, that's really good news!


As an update, in the meanwhile I found that it would actually be possible to also update the text of custom/radio buttons while the dialog is shown (additionally to the text elements). This not possible using a regular Task Dialog message, but by retrieving the window handles (hWnd) of the buttons and then calling SetWindowText() to update their text, or sending a BCM_SETNOTE message to update the command link's description/note.
(When updating the button text, the dialog would not change its layout e.g. if the text is too long, but this can be solved by sending a TDM_SET_ELEMENT_TEXT message afterwards.)

taskdialog-updatebuttons

I have also seen other implementations/applications that seem to be doing this (e.g. WinSCP when showing a dialog that will automatically close after a few seconds, where the remaining seconds are displayed in the button text).

However, this has a few drawbacks:

  • Retrieving the button handles is done using EnumChildWindows() to enumerate all child window handles, and then checking if their window class name equals "Button" (because the visible buttons don't have window IDs, so GetWindowLongPtr() with GWLP_ID will not work). However, this relies on EnumChildWindows() always returning the buttons in the correct order and that there are no other buttons that would also be returned. I'm not sure if there could ever be a behavior change in Windows that could cause the implementation to fail, although that is probably very unlikely.
  • A bigger problem is that the key bindings are not updated if you changes the mnemonics of a button. For example, if you initally set a button text "&A" but later (while the dialog is shown) update it to "&B", you would still need to press Alt+A instead of Alt+B to select the button.

I have prepared support for updating the button text while the Task Dialog is shown in the code, but disabled it for the above reasons (to enable it, you can uncomment the AssignControlHandles() call in TaskDialogContents.ApplyInitialization()).

What do you think? Thanks!

@GSPP
Copy link

GSPP commented Jan 26, 2019

It's a trade-off between enabling more scenarios and introducing API "quirks" that must be documented. What is the value of those additional scenarios? What are they actually? Why would an application typically update the button texts in a live dialog?

@RussKie RussKie added this to the 5.0.0 milestone Aug 12, 2019
@terrajobst
Copy link
Contributor

I've added the notes of API review here. Feedback welcome!

@kirsan31
Copy link
Contributor

So, only in 5.0 now? Why so much delay?

@RussKie
Copy link
Member

RussKie commented Aug 14, 2019

Because further work is required, and it won't be completed before we ship v3.0.
We expect to cut a release/3.0 again shortly and then the master will be rebranded to v5.0.

@kpreisser
Copy link
Contributor Author

Hi @terrajobst,

thanks a lot for sharing the review notes! I will start to work on addressing them when I have time.

@kpreisser
Copy link
Contributor Author

I have implemented changes to the PR according to the review notes:

  • TaskDialog.Show() should be renamed to ShowDialog() to match existing terms
  • Rename instruction to mainInstruction
  • Rename title to caption

Done.

  • Remove TaskDialogButtonClickedEventArgs and add a Boolean property to TaskDialogButton that allows to specify whether the dialog will close when the button is clicked.

I removed class TaskDialogButtonClickedEventArgs and added a boolean property TaskDialogButton.ShouldCloseDialog (default value: true). This property can be set to false to prevent the dialog from closing when the button was clicked.

  • Can we remove TaskDialogStandardIcon and replace the enum values with static
    fields on TaskDialogIcon. Also instead of implicit operators, can we just
    have a constructor that takes an Icon?

I removed TaskDialogStandardIcon enum and TaskDialogIconHandle class, and replaced the enum values with static fields on TaskDialogIcon, and added a public constructor where an Icon (or icon handle as IntPtr) can be supplied.

Note that this means a TaskDialogIcon instance can represent either a standard icon (when retrieved from one of the static fields) or a handle icon (when created with the public constructor).

When the instance was created by passing an Icon (or IntPtr) to the public constructor, the icon handle can be retrieved with the IconHandle property. However, when the instance represents a standard icon, that property throws an InvalidOperationException. Is this correct/desired?

- Same for `TaskDialogStandardButton`.

I think that it is not that simple with TaskDialogStandardButton. Note that that type isn't an enum but a subclass of TaskDialogButton, so that you can create an instance of TaskDialogButton and customize it (like setting the Enabled property or add an handler to the Click event).
I don't think it is possible to created shared instances of that button for each TaskDialogResult value that could be retrieved by static fields, because then all task dialogs would share that instances (e.g. when showing multiple dialogs at the same time).

However, I think the dicussion also went into the direction of combining TaskDialogStandardButton and TaskDialogCustomButton classes into a single TaskDialogButton class that can represent either a standard or a custom button.
This would be possible, although then there are other questions e,g. should it be possible to change the type of an existing instance between a standard and custom button by setting the properties; how should properties behave when they are not applicable for the current state etc.

  • We should investigate whether extending CommonDialog makes sense here/is
    possible

I think the main issue with deriving from System.Windows.Forms.CommonDialog (which was mentioned in the review) is that it has ShowDialog() methods that return System.Windows.Forms.DialogResult. This would correspond to the TaskDialogResult enum (we could add the missing enum values in Forms.DialogResult and then remove TaskDialogResult), however the difficulty is that a task dialog result is not necessarily a simple dialog result, but can also be a custom button.

It would be possible to add a enum value like CustomButtonClicked to the DialogResult so that the task dialog could indicate a non-standard button was clicked, and this button could then be retrieved by a separate property. However, this would mean that the CustomButtonClicked value has to be added to the general DialogResult enum, which I think wouldn't make much sense, because then it would also be available for every other dialog where this task-dialog-specific value is not applicable.

Additionally, looking at the implementation (ShowDialog()) it seems to be implemented such that it needs to subclass the owner window to be able to process HELPMSGSTRING messages in order to raise the HelpRequest event. This behavior seems specific to common dialog boxes as stated in the documentation (which is not applicable to the task dialog):

A common dialog box sends the HELPMSGSTRING registered message to the window procedure of its owner window when the user clicks the Help button.

Therefore, I think TaskDialog should probably not derive from CommonDialog because it isn't actually a common dialog box in terms of the Win32 documentation.

Thanks!

@RussKie RussKie added the tenet-compatibility-OS Compatibility with OS features label Jan 8, 2020
@terrajobst
Copy link
Contributor

@RussKie is this ready for another pass or do you want to incorporate the feedback from the UX study first? If the latter, we should mark this issue as api-needs-work.

@RussKie RussKie added api-needs-work (3) API needs work before it is approved, it is NOT ready for implementation; applied by repo owners and removed api-ready-for-review (2) API is ready for formal API review; applied by the issue owner labels Feb 14, 2020
@RussKie
Copy link
Member

RussKie commented Feb 14, 2020

Updated the label, thanks for the heads up.
We're currently working with @kpreisser through the study feedback.

@ygc369
Copy link

ygc369 commented Feb 20, 2020

Can this task dialog have textBox in it? Can it be used as an inputBox?

@RussKie
Copy link
Member

RussKie commented Feb 20, 2020

Can this task dialog have textBox in it? Can it be used as an inputBox?

No, this is an implementation of https://docs.microsoft.com/en-us/windows/desktop/Controls/task-dialogs-overview

RussKie pushed a commit that referenced this issue Apr 18, 2020
@RussKie RussKie added api-approved (4) API was approved in API review, it can be implemented and removed api-needs-work (3) API needs work before it is approved, it is NOT ready for implementation; applied by repo owners labels Apr 18, 2020
@RussKie RussKie removed this from the 5.0 milestone Apr 18, 2020
nikeee added a commit to nikeee/HolzShots that referenced this issue Sep 8, 2020
...and use new Windows Forms TaskDialog instead.
See: dotnet/winforms#146
Addresses #34
@GHosaPhat
Copy link

Is there an implementation of this that contains a textbox control, similar to the exception dialog that contains the stack trace with the JIT debugger (below)?

Stack Trace Dialog

I'm working on rolling my own dialog like this to display details (collapsed unless the user clicks a Show Details button) for certain, very specific events in my application, but I'd be just as happy if I didn't have to reinvent something that's already out there.

@jnm2
Copy link
Contributor

jnm2 commented Oct 11, 2020

@GHosaPhat My understanding is that the underlying Windows API locks it down to only the things shown here: https://docs.microsoft.com/en-us/windows/win32/controls/task-dialogs-overview

There might be ways of getting the window handle and adding controls yourself, but I'm not sure it would interoperate well with Windows' layout logic and it would probably be pretty hacky.

@GHosaPhat
Copy link

All right. I guess "roll my own" it is, then. Thank you.

@ghost ghost locked as resolved and limited conversation to collaborators Feb 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved (4) API was approved in API review, it can be implemented design-discussion Ongoing discussion about design without consensus tenet-compatibility-OS Compatibility with OS features
Projects
None yet
Development

Successfully merging a pull request may close this issue.