Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

Multi Target UWP to 1809 and 2004 #1247

Closed
wants to merge 43 commits into from

Conversation

inforithmics
Copy link
Contributor

@inforithmics inforithmics commented Apr 29, 2021

Description of Change

Bugs Fixed

Probably Fixes

API Changes

Adds Target Windows 17763 / 1809
Keeps Target Windows 19031 / 2004

Behavioral Changes

works now again on Windows 17763 / 1809

PR Checklist

  • Has tests (if omitted, state reason in description)
    Has already tests
  • Has samples (if omitted, state reason in description)
    Has already samples
  • Rebased on top of main at time of PR
  • Changes adhere to coding standard

add windows 1809 / 10.0.18362 target
@inforithmics inforithmics changed the title Update Xamarin.CommunityToolkit.csproj Multi Target UWP 1809 / 2004 Apr 29, 2021
@inforithmics inforithmics changed the title Multi Target UWP 1809 / 2004 Multi Target UWP to 1809 and 2004 Apr 29, 2021
@inforithmics inforithmics changed the title Multi Target UWP to 1809 and 2004 Multi Target UWP to 1809 and 2004 (WIP) Apr 29, 2021
@inforithmics
Copy link
Contributor Author

inforithmics commented Apr 29, 2021

Don't merge yet, seems not to work with Windows 1809

Target 1809 instead of 2004
@inforithmics
Copy link
Contributor Author

I picked the wrong uap version number now it should really target 1809 and 2004

@inforithmics
Copy link
Contributor Author

@inforithmics inforithmics changed the title Multi Target UWP to 1809 and 2004 (WIP) Multi Target UWP to 1809 and 2004 Apr 29, 2021
@jfversluis
Copy link
Member

OK, so. This fixes the mentioned issues as it will sort out the SDK that needs to be targeted and it will automatically pick up on the shadows not being there for the popups for instance?

And this closes #942 and #1238 ?

@inforithmics
Copy link
Contributor Author

inforithmics commented Apr 29, 2021

As I mentioned in.

#1238 (comment)

When you Target for Min Windows 1809 Then the Transparent Background won't work because of this.

#if UWP_18362
if (Element.Color == Color.Transparent)
     flyoutStyle.Setters.Add(new Windows.UI.Xaml.Setter(FlyoutPresenter.IsDefaultShadowEnabledProperty, false));
#endif

The 1809 Target won't Compile the Code and therefore it won't be executed which means no Transparent Background for the Popup.

But it would solve the UWP Targeting issues. and close #942 and #1238?

If the Transparent Background should work

#if UWP_18362
if (Element.Color == Color.Transparent)
     flyoutStyle.Setters.Add(new Windows.UI.Xaml.Setter(FlyoutPresenter.IsDefaultShadowEnabledProperty, false));
#else
// Fixing Transparent Background for 1809 ....
#endif

would be needed

@jfversluis
Copy link
Member

Right, I noticed that compiler directive in there, yet it breaks right now for people targeting different versions if I understand correctly

@inforithmics
Copy link
Contributor Author

That's why I did the pull request #942
Where it Targets the 1809 But makes the newer properties available. But they need to be guarded by Api Checks

@inforithmics
Copy link
Contributor Author

Maybe Multi Targeting and deciding by case by case basis what to do is saver than having runtime crashes.

For example in the develop branch

https://github.com/xamarin/XamarinCommunityToolkit/blob/develop/src/CommunityToolkit/Xamarin.CommunityToolkit/Effects/Shadow/PlatformShadowEffect.uwp.cs

View.ActualSize is used which is not availble under 1809 with compilation for 1809 and 2004 this would immediatly be detected.

Here I mentioned a possible Fix for 1809 with transparent background, But I have to this it First in Release (Linking Mode if it still works).

#1238 (comment)

Trying to fix Transparant Background in Popup for 1809 Target
Fix build
Revert Sdk.extras upgrade
@inforithmics
Copy link
Contributor Author

inforithmics commented Apr 30, 2021

I tested the changes and it works for an UWP App in the Debug Mode. But if I Native Compile it (Release) Popup does not work. Although the original Popup doesn't work in Release Mode. I have to investigate further what the problem could be. Maybe the wrong Release compilation Settings.

@inforithmics
Copy link
Contributor Author

inforithmics commented Apr 30, 2021

A fun fact I discovered is that the Xamarin.CommunityToolkit Nuget Package and Xamarin.CommunityToolkit Project behave differently in Version Resolution Management.

  1. Original Nuget Targets 19031 / 2004 does not work in UWP App Targeting Min Version of 1809. So a Project using the nuget package doesn't work.
  2. Original Project Targets 9031 / 2004 works in UWP App Targeting Min Version of 1809. So the Test Project works because it references the Xamarin.CommunityToolkit Project File.

@inforithmics
Copy link
Contributor Author

I removed the Release Compilation Crash Fixes from this Pull Request, because they are reviewed in this Pull Request.

#1303

@bijington
Copy link
Contributor

@inforithmics thank you for keeping this PR up to date. To give a brief heads up we are currently unsure what the best course of action is in terms of supporting an older version of UWP and also the implications of targeting multiple versions.

I plan to investigate this over the next week or so in order to hopefully help us all come to a decision on whether this is something we should do. This also relates to #1684

@bijington bijington closed this Jan 11, 2022
@bijington
Copy link
Contributor

Changes made in #1684 were considered safer

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants