Closed jtbrower closed 4 years ago
Thanks for all these comments @jtbrower We have addressed some of them on the new version of the spec: https://github.com/microsoft/microsoft-ui-xaml-specs/blob/Win32/active/Win32/Window_and_Application_API_Spec.md
We are still updating it.
@marb2000 excellent, keep up the great effort. It's going to be fun to watch WinUI take shape after it's fully separated from the OS.
Window Spec Review
While trying to decide if I could migrate a significant amount of WPF XAML over to WinUI, I needed to understand the Windowing model for WinUI Desktop applications. I was pointed to the Window and Application API Spec as a source of information. Since the Windowing model is really important to the type of applications I write, I spent extra time reading over this spec. Trust me that I never intended to provide a thorough peer review like this, the initial goal was to mention the issues I had with the IWindowInterop class, but I ended up adding everything I came across.
I think it should be Windows.UI.Xaml.Window with Windows being plural.
Windows Misspelled
The word proposed used twice
Markup example does not match code example
The code example shows how to add a TextBox to the Window, the markup example shows how to add a grid with a button. Typically you see these examples show how to achieve the same thing with code vs xaml.
This section indicates that multiple windows are supported. I understand this is just a spec, but I was pointed to this spec by another Microsoft developer and its hard for me to know what's implemented and what isn't.
Throughout the document the XAML and code examples use mixed quotation marks that cause compilation errors. For example, see this markup example and notice how the button's content property and Click handler are assigned using ” as a quotation mark instead of " .
The Multiple Window Example inside the click handler has a missing semicolon
The example to create a Window in a separate thread has compilation errors and seems like a WPF copy and paste.
Maybe this example is "how it will eventually be"? This code will not compile under UWP or WinUI Desktop because InvokeShutdown() is not defined on Dispatcher which happens to be a CoreDispatcher. The Dispatcher.Run() cannot be found either. This is normally in WindowsBase.dll.
This code is perfectly valid WPF code, it compiles under WPF. If the end user needs to include a different NuGet package then we should let them know.
In This Section Maybe it should clarify that a developer needs to use the Win32 pinvoke APIs to change the window size. Also, the word specify should be specified.
Since the Window does not have an Icon and the document says its a new property I will assume that its a future property and not in the current release.
Small issues like these send developers off on a Google / Bing search mission wondering why its not there and how to use pinvoke calls to resolve it.
The code example for VisibilityChanged uses the wrong event arg type.
The sample event handler prefixes the event args with 'MUX' and this one is similar.
None of the other examples use MUX. This can cause a newbie developer to create help posts on Reddit wondering what it was. I think its great to include namespaces when it is done consistently or it is at least defined.
The example using the IWindowInterop interface will not compile because the classes required cannot be found anywhere.
This example is truly what led to me creating this document. Another Microsoft employee pointed me to this example as a method of obtaining the Window's handle. I spent quite a bit of time blaming myself for not being able to locate the COM interface or any potential extension methods. Both Google and Github return one result and it points back to the same github issue where I was asking how to do this. Here is the solution.
This section on Window.UIContext talks about removing that property. I might be mistaken but I think it is already gone and could be yanked from the document.
The wrong type is listed for WindowActivationState
Similar to above, The VisibilityChangedArgs should be Microsoft.UI.Xaml.WindowVisibilityChangedEventArgs
The example code for override of OnLaunched has multiple compilation errors.
The line
window.FileName = arguments[0]
is missing a semicolon but this code snippet would only be meaningful in the context of an additional class 'MainWindow' that had a property on it called FileName. Without knowing that the reader could be led to believe that the Window class has a FileName property because the entire document is about the Window.Out of curiosity I tried to create a UWP WinUI app, declared a window and called Activate just as was done below. This led to an access violation. Considering I am new to UWP and this was outside the scope of what I wanted to do today, I didn't dig in to see if this code snippet could be logically used in UWP or not, maybe the author should double check that.
Note that the section about the event arg type LaunchActivatedEventArgs is out of order from the section that lists the event. Maybe the LaunchActivatedEventArgs section should be right next to the OnLaunched section.
Application APIs that are no called on Win32 uses the word "no" but I think the author intended to use "not" or "now". I am guessing "not" but there is a big difference between the two.