Support WPF and WindowsForms-specific FrameworkReferences via profiles #3259

dsplaisted · 2019-05-22T20:49:24Z

This PR supports referencing only Windows Forms or only WPF assets from the WindowsDesktop targeting pack. See https://github.com/dotnet/cli/issues/10536.

This is done by supporting the following additional FrameworkReferences:

Microsoft.WindowsDesktop.App.WindowsForms
Microsoft.WindowsDesktop.App.WPF

In the implementation, a KnownFrameworkReference can specify a Profile of the targeting pack via metadata. https://github.com/dotnet/core-setup/issues/6210 tracks adding this information to the WindowsDesktop FrameworkList.xml. Until then, this PR hard-codes the assemblies which are in each profile.

After this is merged, the WindowsDesktop SDK should be updated to use the .WindowsForms or .WPF FrameworkReference depending on if the UseWPF or UseWindowsForms properties are set. If both of them are set, it should use the base Microsoft.WindomwsDesktop.App FrameworkReference, which will include the integration DLL.

@nguerrera @vatsan-madhavan

vatsan-madhavan · 2019-05-23T01:57:42Z

@miguep, Can you outline the problem with namespace/type collisions you encountered today because we now have references to all theme assemblies at the same time, which IIRC you had described as unavoidable and required a hacky target to remove some of them from the reference list?

/cc @rladuca
/cc @dotnet/wpf-developers

miguep · 2019-05-23T17:01:13Z

@vatsan-madhavan The problem I found is this:
Take for example this class: DataGridHeaderBorder, it is defined in each of the theme assemblies (e.g. PresentationFramework.Aero, PresentationFramework.Aero2). If I simply attempt to use this class, I get a compilation error:
The type 'DataGridHeaderBorder' exists in both 'PresentationFramework.Aero, Version=4.8.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35' and 'PresentationFramework.Aero2, Version=4.8.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35'

This happens because WindowsDesktop brings in all the theme assemblies references, where in .NET Framework you would explicitly reference which assembly you wanted, and attempting to reference more than one and using this type resulted in the same error

vatsan-madhavan · 2019-05-23T17:10:13Z

@dsplaisted, How do we propose we handle these theme assembly conflicts?

nguerrera · 2019-05-23T17:15:13Z

Are the theme assemblies designed to always be mutually exclusive? Are there scenarios where you need to alias and use more than one in the same project.

I suppose we would have to put these into separate profiles to go with current design, but this is really not a pattern we anticipated. It is against the framework design guidelines to have types with same full name in the framework. :(

nguerrera · 2019-05-23T17:18:24Z

I am surprised this hasn't been reported to us until now. Is the usual pattern to not reference these in user code at all?

vatsan-madhavan · 2019-05-23T18:14:10Z

They are typically not referenced - they are used as a source of resources in <ResourceDictionary ..>.

That said, they can be referenced - naturally only one at a time (or more than one at a time with the help of namespace aliasing).

We would have to support both patterns, I think.

==

This is what I think we should do:

The default - not reference any of them.

Put each of them in a separate newly added FrameworkReferences (Microsoft.WindowsDesktop.App.WPF.Aero, Microsoft.WindowsDesktop.App.WPF.Aero2, Microsoft.WindowsDesktop.App.WPF.Luna etc.) and allow them to be included them one at a time (UseWpfAero, UseWpfAero2 etc.).

And also provide a mechanism for enabling Aliases to each to support the scenario of more than one of them being requested simultaneously. I'm not sure what this would look like...

rladuca · 2019-05-23T18:27:09Z

We've had instances where developers reference the themes directly in order to force certain appearances in controls. A specific instance I recall is forcing Aero themed scrollbar chrome.

Do we need to use FrameworkReferences for this? Is it possible to have them excluded from the default reference set but in the path so a developer can just use a normal Reference? For those already directly referencing them, it means less transformation of their projects.

wjk · 2019-05-23T22:19:08Z

@vatsan-madhavan My 2¢:

While I don't really like your idea of having one theme DLL per framework reference (it's a bit too fine-grained for my taste), I do like the resulting ability to build more fine-grained theming support. (Once .NET Core 3.0 is out the door and behavior-changing PRs are ready to be considered, I — and undoubtedly others — will be all over fixing up the default WPF themes to not look nearly as ugly.)

I would also appreciate a mechanism where a default theme can be selected automatically. Although Directory.Build.props makes it easy to add MSBuild elements to all projects at the same time, I would appreciate projects from earlier WPF-on-Core betas being able to work as before without having to modify my csproj file to add whatever <UseWpfFoo> property is required to select a theme which was done automatically before.

Hope this helps!

nguerrera · 2019-05-25T00:08:13Z

I suggest opening a tracking issue for the theme dlls. This change doesn't make the existing problem any worse so I think we should move forward with it (after code review of course, which I'm doing now.)

nguerrera · 2019-05-25T00:13:42Z

src/Tasks/Microsoft.NET.Build.Tasks/ResolveTargetingPackAssets.cs

+                }
+            }
+
+            ReferencesToAdd = deduplicatedReferences.Distinct() .ToArray();


Why are we calling Distnct() if we've already removed dupes?

src/Tasks/Microsoft.NET.Build.Tasks/ResolveTargetingPackAssets.cs

nguerrera · 2019-05-25T12:17:41Z

src/Tasks/Microsoft.NET.Build.Tasks/ResolveTargetingPackAssets.cs

+
+            //  Filter out duplicate references (which can happen when referencing two different profiles that overlap)
+            List<TaskItem> deduplicatedReferences = DeduplicateItems(referencesToAdd);
+            ReferencesToAdd = deduplicatedReferences.Distinct() .ToArray();


Distinct is still there?

RussKie · 2019-06-20T03:36:42Z

What are our expectation for the docs pertaining to the use of UseWPF and UseWindowsForms properties?

* Update dependencies from https://github.com/aspnet/websdk build 20191021.3 - Microsoft.NET.Sdk.Web - 3.1.100-preview2.19521.3 * Update dependencies from https://github.com/aspnet/websdk build 20191022.3 - Microsoft.NET.Sdk.Web - 3.1.100-preview2.19522.3

dsplaisted added 2 commits May 22, 2019 10:52

Add support for profiles in targeting packs

d3ddc04

Add tests for targeting pack profiles

70326ea

dsplaisted requested review from nguerrera and a team May 23, 2019 00:58

vatsan-madhavan mentioned this pull request May 23, 2019

Support WPF and WinForms specific FrameworkReferences via Profiles dotnet/wpf#746

Closed

2 tasks

nguerrera reviewed May 25, 2019

View reviewed changes

src/Tasks/Microsoft.NET.Build.Tasks/ResolveTargetingPackAssets.cs Show resolved Hide resolved

nguerrera approved these changes May 25, 2019

View reviewed changes

This was referenced May 25, 2019

WPF Theme Assemblies are not usable under the current FrameworkReference design #3265

Closed

Theme assemblies cannot be referenced dotnet/wpf#765

Closed

Apply code review feedback

0e14ca0

dsplaisted merged commit 8f80c53 into dotnet:master May 25, 2019

nguerrera reviewed May 25, 2019

View reviewed changes

vatsan-madhavan mentioned this pull request Jun 4, 2019

Support Microsoft.NetCore.App.Wpf and Microsoft.NetCore.App.WinForms profiles in Microsoft.NET.Sdk.WidnowsDesktop dotnet/wpf#866

Closed

vatsan-madhavan mentioned this pull request Jun 19, 2019

Cannot add ElementHost control in project because there’s no Integration namespace in System.Windows.Forms dotnet/winforms#1144

Closed

nguerrera added this to the 3.0.1xx milestone Jul 15, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Support WPF and WindowsForms-specific FrameworkReferences via profiles #3259

Support WPF and WindowsForms-specific FrameworkReferences via profiles #3259

Uh oh!

dsplaisted commented May 22, 2019 •

edited

Loading

Uh oh!

vatsan-madhavan commented May 23, 2019 •

edited

Loading

Uh oh!

miguep commented May 23, 2019

Uh oh!

vatsan-madhavan commented May 23, 2019

Uh oh!

nguerrera commented May 23, 2019

Uh oh!

nguerrera commented May 23, 2019

Uh oh!

vatsan-madhavan commented May 23, 2019 •

edited

Loading

Uh oh!

rladuca commented May 23, 2019

Uh oh!

wjk commented May 23, 2019

Uh oh!

nguerrera commented May 25, 2019

Uh oh!

nguerrera May 25, 2019

Uh oh!

Uh oh!

nguerrera May 25, 2019

Uh oh!

RussKie commented Jun 20, 2019

Uh oh!

Uh oh!

Support WPF and WindowsForms-specific FrameworkReferences via profiles #3259

Support WPF and WindowsForms-specific FrameworkReferences via profiles #3259

Uh oh!

Conversation

dsplaisted commented May 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vatsan-madhavan commented May 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

miguep commented May 23, 2019

Uh oh!

vatsan-madhavan commented May 23, 2019

Uh oh!

nguerrera commented May 23, 2019

Uh oh!

nguerrera commented May 23, 2019

Uh oh!

vatsan-madhavan commented May 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rladuca commented May 23, 2019

Uh oh!

wjk commented May 23, 2019

Uh oh!

nguerrera commented May 25, 2019

Uh oh!

nguerrera May 25, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nguerrera May 25, 2019

Choose a reason for hiding this comment

Uh oh!

RussKie commented Jun 20, 2019

Uh oh!

Uh oh!

dsplaisted commented May 22, 2019 •

edited

Loading

vatsan-madhavan commented May 23, 2019 •

edited

Loading

vatsan-madhavan commented May 23, 2019 •

edited

Loading