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

Convert DesignTools projects to Sdk-style #3794

Merged
merged 3 commits into from
Sep 8, 2021

Conversation

Nirmal4G
Copy link
Contributor

@Nirmal4G Nirmal4G commented Feb 28, 2021

Fixes #3805

Notes

We want to force the WindowsDesktop SDK to always use WinFX targets from the SDK instead of using the .NET Framework's inbox version as the SDK version contains fixes around enabling PackageReference. In .NET SDK v5, we added an overridable option to always opt-into this behavior. See dotnet/wpf#2976 and dotnet/wpf#4630. Thus, we'll be using Microsoft.NET.Sdk.WindowsDesktop instead!

<Project Sdk="Microsoft.NET.Sdk.WindowsDesktop" />

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Build related changes
  • Refactoring (no functional changes, no API changes)

What is the current behavior?

  • Design project for UWP Media Controls is missing.
  • Design projects are in legacy project style since it was based on VSLang project system.

What is the new behavior?

  • Added Design Project for UWP Media Controls.
  • Design projects are now in Sdk-style and will now be loaded in CPS.

PR Checklist

Please check if your PR fulfils the following requirements:

  • Tested code with current supported SDKs
  • New major technical changes in the toolkit have or will be added to the Wiki e.g. build changes, source generators, testing infrastructure, sample creation changes, etc...
  • Header has been added to all new source files (run build/UpdateHeaders.bat)
  • Contains NO breaking changes

Other information

  • If you're editing this patch tree, please rebase on latest HEAD and then commit, without updating from the latest HEAD.
  • When merging, please update the commit title to PR title instead of the default Merge pull request #xxxx from repo/branch, and commit message to either PR message or messages of individual commits. The auto-merge bot does this by default.

@ghost
Copy link

ghost commented Feb 28, 2021

Thanks Nirmal4G for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌

@ghost ghost added the bug 🐛 An unexpected issue that highlights incorrect behavior label Feb 28, 2021
@Nirmal4G Nirmal4G changed the title Feature/design tools Update DesignTools projects Feb 28, 2021
@michael-hawker michael-hawker added this to the 7.0 milestone Mar 1, 2021
@michael-hawker
Copy link
Member

@Nirmal4G looks like the build is failing:

"D:\a\1\s\Windows Community Toolkit.sln" (Build target) (1) ->
       "D:\a\1\s\Microsoft.Toolkit.Uwp.UI.Controls.DataGrid.Design\Microsoft.Toolkit.Uwp.UI.Controls.DataGrid.DesignTools.csproj.metaproj" (default target) (32) ->
       "D:\a\1\s\Microsoft.Toolkit.Uwp.UI.Controls.DataGrid.Design\Microsoft.Toolkit.Uwp.UI.Controls.DataGrid.DesignTools.csproj" (default target) (40) ->
       (CoreCompile target) -> 
         D:\a\1\s\Microsoft.Toolkit.Uwp.UI.Controls.DataGrid.Design\Controls\DataGrid.Typedata.cs(11,42): error CS0103: The name 'RootNamespace' does not exist in the current context [D:\a\1\s\Microsoft.Toolkit.Uwp.UI.Controls.DataGrid.Design\Microsoft.Toolkit.Uwp.UI.Controls.DataGrid.DesignTools.csproj]
         D:\a\1\s\Microsoft.Toolkit.Uwp.UI.Controls.DataGrid.Design\Controls\DataGrid.Typedata.cs(12,48): error CS0103: The name 'RootNamespace' does not exist in the current context [D:\a\1\s\Microsoft.Toolkit.Uwp.UI.Controls.DataGrid.Design\Microsoft.Toolkit.Uwp.UI.Controls.DataGrid.DesignTools.csproj]
         D:\a\1\s\Microsoft.Toolkit.Uwp.UI.Controls.DataGrid.Design\Controls\DataGrid.Typedata.cs(13,53): error CS0103: The name 'RootNamespace' does not exist in the current context [D:\a\1\s\Microsoft.Toolkit.Uwp.UI.Controls.DataGrid.Design\Microsoft.Toolkit.Uwp.UI.Controls.DataGrid.DesignTools.csproj]
         D:\a\1\s\Microsoft.Toolkit.Uwp.UI.Controls.DataGrid.Design\Controls\DataGrid.Typedata.cs(14,52): error CS0103: The name 'RootNamespace' does not exist in the current context [D:\a\1\s\Microsoft.Toolkit.Uwp.UI.Controls.DataGrid.Design\Microsoft.Toolkit.Uwp.UI.Controls.DataGrid.DesignTools.csproj]
         D:\a\1\s\Microsoft.Toolkit.Uwp.UI.Controls.DataGrid.Design\Controls\DataGrid.Typedata.cs(15,56): error CS0103: The name 'RootNamespace' does not exist in the current context [D:\a\1\s\Microsoft.Toolkit.Uwp.UI.Controls.DataGrid.Design\Microsoft.Toolkit.Uwp.UI.Controls.DataGrid.DesignTools.csproj]
         D:\a\1\s\Microsoft.Toolkit.Uwp.UI.Controls.DataGrid.Design\Controls\DataGrid.Typedata.cs(16,56): error CS0103: The name 'RootNamespace' does not exist in the current context [D:\a\1\s\Microsoft.Toolkit.Uwp.UI.Controls.DataGrid.Design\Microsoft.Toolkit.Uwp.UI.Controls.DataGrid.DesignTools.csproj]


       "D:\a\1\s\Windows Community Toolkit.sln" (Build target) (1) ->
       "D:\a\1\s\Microsoft.Toolkit.Uwp.UI.Controls.Core.Design\Microsoft.Toolkit.Uwp.UI.Controls.Core.DesignTools.csproj.metaproj" (default target) (31) ->
       "D:\a\1\s\Microsoft.Toolkit.Uwp.UI.Controls.Core.Design\Microsoft.Toolkit.Uwp.UI.Controls.Core.DesignTools.csproj" (default target) (36) ->
         D:\a\1\s\Microsoft.Toolkit.Uwp.UI.Controls.Core.Design\Common\PlatformTypes.cs(6,7): error CS0246: The type or namespace name 'Windows' could not be found (are you missing a using directive or an assembly reference?) [D:\a\1\s\Microsoft.Toolkit.Uwp.UI.Controls.Core.Design\Microsoft.Toolkit.Uwp.UI.Controls.Core.DesignTools.csproj]
         D:\a\1\s\Microsoft.Toolkit.Uwp.UI.Controls.Core.Design\Common\PlatformTypes.cs(7,7): error CS0246: The type or namespace name 'Windows' could not be found (are you missing a using directive or an assembly reference?) [D:\a\1\s\Microsoft.Toolkit.Uwp.UI.Controls.Core.Design\Microsoft.Toolkit.Uwp.UI.Controls.Core.DesignTools.csproj]


       "D:\a\1\s\Windows Community Toolkit.sln" (Build target) (1) ->
       "D:\a\1\s\Microsoft.Toolkit.Uwp.UI.Controls.Markdown.Design\Microsoft.Toolkit.Uwp.UI.Controls.Markdown.DesignTools.csproj.metaproj" (default target) (33) ->
       "D:\a\1\s\Microsoft.Toolkit.Uwp.UI.Controls.Markdown.Design\Microsoft.Toolkit.Uwp.UI.Controls.Markdown.DesignTools.csproj" (default target) (39) ->
         D:\a\1\s\Microsoft.Toolkit.Uwp.UI.Controls.Markdown.Design\Controls\MarkdownTextBlock.Typedata.cs(11,51): error CS0103: The name 'RootNamespace' does not exist in the current context [D:\a\1\s\Microsoft.Toolkit.Uwp.UI.Controls.Markdown.Design\Microsoft.Toolkit.Uwp.UI.Controls.Markdown.DesignTools.csproj]

This is marked as draft, is there more coming or should this be ready to review soon? We'd like to try and close out the release this week if possible.

Thanks!

@Nirmal4G Nirmal4G force-pushed the feature/design-tools branch 2 times, most recently from 6350c9e to 48f7df1 Compare March 2, 2021 18:32
@Nirmal4G
Copy link
Contributor Author

Nirmal4G commented Mar 2, 2021

@michael-hawker

Rebase gone wrong. Sorry about that. My patches were applicable before splitting the controls. Now, it should build fine.

is there more coming or should this be ready to review soon?

There are few patches left to finish the follow-up tasks that were pending previously.

We'd like to try and close out the release this week if possible.

Sure. These are minor issues. Should be done within a day or two!

@michael-hawker
Copy link
Member

Thanks @Nirmal4G! 🦙❤ Build looks good now too! 🎉

Keep us posted, sound like everything's on track. Really appreciate all your help here.

@Nirmal4G Nirmal4G mentioned this pull request Mar 4, 2021
3 tasks
@Nirmal4G Nirmal4G changed the title Update DesignTools projects Update 'DesignTools' projects Mar 4, 2021
@michael-hawker
Copy link
Member

@Nirmal4G think we can get any final updates in by tomorrow (3/10) so we can include them in 7.0?

@michael-hawker michael-hawker marked this pull request as ready for review March 11, 2021 08:07
@michael-hawker michael-hawker mentioned this pull request Mar 11, 2021
@michael-hawker michael-hawker marked this pull request as draft March 11, 2021 15:40
@michael-hawker michael-hawker modified the milestones: 7.0, 7.1 Mar 11, 2021
@michael-hawker
Copy link
Member

Thanks for updating @Nirmal4G. I'm still able to reproduce the HeaderedContentControl not showing up in the DocumentOutline and the MarkdownTextBlock missing from the list.

Should these two issues have been fixed at this point in time yet?

Considering it seem like what we have for now in main is a good first step for this transition, I'm going to not hold the release on waiting for this and move this to 7.1 so we can finish this work without pressure. Thanks!

@Nirmal4G Nirmal4G changed the title Update 'DesignTools' projects Convert 'DesignTools' projects to Sdk-style Mar 14, 2021
@Nirmal4G Nirmal4G marked this pull request as ready for review March 14, 2021 11:39
@michael-hawker
Copy link
Member

@Nirmal4G does this PR need updating now that the underlying one has been merged? Or is it still waiting on another PR of yours?

@Nirmal4G Nirmal4G changed the title Convert 'DesignTools' projects to Sdk-style Convert DesignTools projects to Sdk-style Jun 21, 2021
@Rosuavio
Copy link
Contributor

Rosuavio commented Jul 20, 2021

It depends on build related changes from PR 3894.

#3894

@Nirmal4G
Copy link
Contributor Author

@RosarioPulella It was intentional. I didn't want GitHub to see the link. This PR doesn't require changes from the referenced PR. It was merely built on top of that PR and some build related issues were fixed in that PR.

@Nirmal4G Nirmal4G marked this pull request as ready for review July 20, 2021 21:01
@Rosuavio
Copy link
Contributor

On this branch I started to get this message from VS.
image
I think maybe some default build conf was removed.

@Nirmal4G
Copy link
Contributor Author

It might be because of the AnyCPU which is the default for .NET SDK-style projects. It seems, I missed that patch during rebase from #3894! Good catch, Thanks!!

@Rosuavio
Copy link
Contributor

Rosuavio commented Aug 5, 2021

Build error

...
2021-08-05T07:08:46.9988440Z     31>CSC : error CS1566: Error reading resource 'Microsoft.Toolkit.Uwp.UI.Controls.Media.xml' -- 'Could not find file 'D:\a\1\s\Microsoft.Toolkit.Uwp.UI.Controls.Media\bin\Release\uap10.0.17763\Microsoft.Toolkit.Uwp.UI.Controls.Media.xml'.' [D:\a\1\s\Microsoft.Toolkit.Uwp.UI.Controls.Media.Design\Microsoft.Toolkit.Uwp.UI.Controls.Media.DesignTools.csproj]
2021-08-05T07:08:47.0818055Z          CompilerServer: server - server processed compilation - e5e97082-64bd-4b00-a276-1557cee2c746
2021-08-05T07:08:47.2088149Z     31>Done Building Project "D:\a\1\s\Microsoft.Toolkit.Uwp.UI.Controls.Media.Design\Microsoft.Toolkit.Uwp.UI.Controls.Media.DesignTools.csproj" (default targets) -- FAILED.
...

@Nirmal4G
Copy link
Contributor Author

Nirmal4G commented Aug 15, 2021

I think the build errors out because of the order of compilation of the projects. We should make sure the XML Doc is produced and copied before the DesignTools project is built. I've added a project dependency to the solution file and that fixed the order of the compilation. Thanks!!

- Update Solution to Use CPS for the Design projects.
- Move 'MetadataRegistration.cs' file into Common folder.
- Remove 'AssemblyInfo.cs' file as it's generated by .NET SDK.
The reason we build the Controls library first is that we have a dependency on the XML Doc file it generates.
The Doc file is embedded within the Design library and it uses it's type-info to populate metadata automatically.
Basic DesignTime support on-par with other controls in the toolkit.

Reverts the following commit
- Commit Message: `Media Controls does not have a "DesignTools" project`
- Commit Hash   : `1781e7f4291061c642fedfa5307aa80879838b56`
Copy link
Contributor

@Rosuavio Rosuavio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything is still working from my testing. Good refactor!

@michael-hawker michael-hawker merged commit b276329 into CommunityToolkit:main Sep 8, 2021
@michael-hawker
Copy link
Member

Thanks @Nirmal4G! This definitely makes things a lot simpler now for sure!

@Nirmal4G Nirmal4G deleted the feature/design-tools branch September 8, 2021 18:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 An unexpected issue that highlights incorrect behavior need wiki 📚 needs attention 👋
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compiling the solution with AnyCPU does not work Modernize DesignTools projects
5 participants