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

Fixed and completed implementation of multitargeted project dependency system #196

Closed
wants to merge 26 commits into from

Conversation

Arlodotexe
Copy link
Member

@Arlodotexe Arlodotexe commented Jul 13, 2022

This PR:

@Arlodotexe Arlodotexe added enhancement Improvement to an existing feature dev loop ➰ For issues that impact the core dev-loop of building experiments templating labels Jul 13, 2022
@michael-hawker
Copy link
Member

@Arlodotexe looks like a build failure for the template. Can you explain what the Library projects (fixed a major bug) bug was here?

Copy link
Member

@michael-hawker michael-hawker left a comment

Choose a reason for hiding this comment

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

I think my biggest question is I don't see where the Dependencies.props is getting referenced for inclusion anywhere?

common/Labs.Uno.UI.Dependencies.props Show resolved Hide resolved
common/Labs.TargetFrameworks.All.props Outdated Show resolved Hide resolved
common/Labs.Sample.props Show resolved Hide resolved
common/Labs.WinAppSdk.props Outdated Show resolved Hide resolved
common/Labs.Head.props Outdated Show resolved Hide resolved
common/Scripts/UseUnoWinUI.ps1 Show resolved Hide resolved
@Arlodotexe
Copy link
Member Author

Arlodotexe commented Jul 13, 2022

Can you explain what the Library projects (fixed a major bug) bug was here?

@michael-hawker The switch between WinUI 2 and 3 in #139 used the property WinUIMajorVersion to decide between package references. However, in the MSBuild step that evaluates property values, TargetFramework is not defined (see here for my notes).

@michael-hawker
Copy link
Member

We need a guard for uno around this:

  D:\a\Labs-Windows\Labs-Windows\template\lab\samples\ProjectTemplate.Wasm\obj\Debug\net5.0\g\XamlCodeGenerator\ToolkitDocumentationRenderer_aee9f0e9748a2e19cff6f84be8470785.g.cs(819,6): warning Uno0001: Windows.UI.Xaml.Controls.TextBlock.IsTextSelectionEnabled is not implemented in Uno [D:\a\Labs-Windows\Labs-Windows\template\lab\samples\ProjectTemplate.Wasm\ProjectTemplate.Wasm.csproj]

@Arlodotexe
Copy link
Member Author

Arlodotexe commented Jul 14, 2022

It's working locally, strange. I'll resolve the compiler errors soon.

So far in this PR, I've focused on implementing the feature using things I know 100% will work as expected, which resulted in needing to use 3 files instead of just 1 for PackageReferences.

However, using my notes here, I might have a way to combine them all into 1 file again while avoiding specific MSBuild quirks that have caused issues in the past. I'm testing it locally first to make sure it works.

@Arlodotexe
Copy link
Member Author

Arlodotexe commented Jul 14, 2022

Oddly, even though the syntax was exactly the same as our usage on TextBlock in ToolkitSampleRenderer:
image

-- and even though the property IsTextSelectionEnabled definitely exists on both TextBlock and MarkdownTextBlock, using the win: prefix here breaks UWP builds.

@Arlodotexe
Copy link
Member Author

Still working to resolve the build errors, locally before pushing.
Though I'm somehow getting a different error from what the CI is getting...

@Arlodotexe
Copy link
Member Author

We'll be using #199 instead

@Arlodotexe Arlodotexe closed this Jul 19, 2022
Martin1994 pushed a commit to Martin1994/Labs-Windows that referenced this pull request Sep 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dev loop ➰ For issues that impact the core dev-loop of building experiments enhancement Improvement to an existing feature templating
Projects
None yet
2 participants