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.

Sign up for GitHub

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

Jump to bottom

.NET 5 and Notifications Mega-Bug #3581

Closed
andrewleader opened this issue Nov 25, 2020 · 10 comments
Closed

.NET 5 and Notifications Mega-Bug #3581

andrewleader opened this issue Nov 25, 2020 · 10 comments
Assignees
@andrewleader
Labels
bug 🐛 An unexpected issue that highlights incorrect behavior notifications 🔔
Milestone
7.0

Comments

@andrewleader
Copy link
Contributor

andrewleader commented Nov 25, 2020 •
edited
Loading

This issue exists to capture/organize all information pertaining to upgrading to .NET 5 and issues with the Notifications library resulting from that upgrade.

Issue 1 (Fixed with #3622 ✅): Compiling the "native" (WinMD version for C++) version doesn't work. We temporarily removed the native target from the csproj file.

Issue 2 (Fixed with #3582 ✅): UWP C# min version target has increased to 17763, so can no longer use the new Show() APIs/etc on UWP apps that still target min version 10240.

image

Issue 3 (Fixed with #3622 ✅): Library doesn't work from .NET 5 WebApi project: #3565

Issue 4 (Fixed with #3622 ✅): Library doesn't work from .NET 5 Windows app project: #3579
@andrewleader andrewleader added the bug 🐛 An unexpected issue that highlights incorrect behavior label Nov 25, 2020
@ghost ghost added the needs triage 🔍 label Nov 25, 2020
@ghost
Copy link

ghost commented Nov 25, 2020

Hello andrewleader, thank you for opening an issue with us!

I have automatically added a "needs triage" label to help get things started. Our team will analyze and investigate the issue, and escalate it to the relevant team if possible. Other community members may also look into the issue and provide feedback 🙌

@andrewleader andrewleader self-assigned this Nov 25, 2020
@Kyaa-dost
Copy link
Contributor

@andrewleader Thanks for highlighting the issue and taking the lead ❤️

Let us know regarding updates or any assistance you may require.

@Kyaa-dost Kyaa-dost added this to the 7.0 milestone Nov 25, 2020
@andrewleader andrewleader mentioned this issue Nov 25, 2020
Merged
7 tasks
@jmithy
Copy link

jmithy commented Dec 9, 2020

Hello @andrewleader. Do we have an estimated date for the fix yet? We can't migrate our WPF app to Net 5.0 until this is fixed. Thanks.

@andrewleader
Copy link
Contributor Author

Hey @jmithy, no ETA yet, I was focusing on the C++ issue first but seems like I should possibly prioritize .NET 5.0. I haven't had any time to work on this recently but hopefully Thurs or Fri I will. Do you have any experience here and know what might be the cause? I'm open to any help, but also totally fine if you have no clue why it doesn't work either and simply just want it to work :)

@jmithy
Copy link

jmithy commented Dec 9, 2020 •
edited
Loading

Hello @andrewleader. I'd love to help out but unfortunately I don't have any clue about it.

ghost pushed a commit that referenced this issue Dec 10, 2020
Reset Notifications min version to 10240 (#3582)
ec93987 
<!-- 🚨 Please Do Not skip any instructions and information mentioned below as they are all required and essential to evaluate and test the PR. By fulfilling all the required information you will be able to reduce the volume of questions and most likely help merge the PR faster 🚨 -->

## Partially addresses #3581 issue 2
<!-- Add the relevant issue number after the "#" mentioned above (for ex: Fixes #1234) which will automatically close the issue once the PR is merged. -->

<!-- Add a brief overview here of the feature/bug & fix. -->

## PR Type
What kind of change does this PR introduce?
<!-- Please uncomment one or more that apply to this PR. -->

- Build or CI related changes


## What is the current behavior?
<!-- Please describe the current behavior that you are modifying, or link to a relevant issue. -->

Min version of UWP package in master incorrectly got bumped to 17763

![image](https://user-images.githubusercontent.com/13246069/100283018-86fbb080-2f21-11eb-8622-78a7707d2599.png)


## What is the new behavior?
<!-- Describe how was this issue resolved or changed? -->

Min version of UWP package is 10240. Had to make sure the DefaultTargetMinVersion and TargetMinVersion properties were set in the Notifications project (so the overall default of 17763 didn't get used).

![image](https://user-images.githubusercontent.com/13246069/100290310-e2cd3600-2f2f-11eb-9fd8-e05a1263dcf4.png)



## PR Checklist

Please check if your PR fulfills the following requirements:

- [x] Tested code with current [supported SDKs](../readme.md#supported)
- [x] Pull Request has been submitted to the documentation repository [instructions](..\contributing.md#docs). Link: <!-- docs PR link -->
- [x] Sample in sample app has been added / updated (for bug fixes / features)
    - [x] Icon has been created (if new sample) following the [Thumbnail Style Guide and templates](https://github.com/windows-toolkit/WindowsCommunityToolkit-design-assets)
- [x] Tests for the changes have been added (for bug fixes / features) (if applicable)
- [x] Header has been added to all new source files (run *build/UpdateHeaders.bat*)
- [x] Contains **NO** breaking changes

<!-- If this PR contains a breaking change, please describe the impact and migration path for existing applications below. 
     Please note that breaking changes are likely to be rejected within minor release cycles or held until major versions. -->


## Other information
@andrewleader
Copy link
Contributor Author

@jmithy .NET 5 should be working by end of day today hopefully! Not sure when the next prerelease package will be released, but we should at least have a package built that you can download and locally reference by today!

Notes to self: My .NET 5 changes seem to have broken keeping the UWP package version at 10240 :P

@andrewleader
Copy link
Contributor Author

Hey @jmithy we've got .NET 5 working in a pull request, you can download the built NuGet package from here: https://dev.azure.com/dotnet/WindowsCommunityToolkit/_build/results?buildId=43110&view=artifacts&pathAsName=false&type=publishedArtifacts

@100RH
Copy link

100RH commented Dec 16, 2020

Hello @andrewleader,
We are building a WinForms component that is using the Notifications Toolkit and we are glad to see that it can build in .NET 5!
Do you have an idea, when it's going to be officially released?
Best regards,
Stoyan

@andrewleader
Copy link
Contributor Author

andrewleader commented Dec 16, 2020 •
edited
Loading

Hey @100RH, glad to see you're going to use notifications in WinForms! We're tentatively targeting a January release (we should at least have another preview out early Jan).

I'd love to hear if the new NuGet package (from that build artifacts link) works for you and would love to hear any feedback you have! Have you looked through the updated notification features in 7.0, which let you send notifications without setting up a Start menu shortcut like previously? The preview docs are here.

@andrewleader
Copy link
Contributor Author

All these fixes have been merged into master, closing! Hopefully we get an updated preview released on NuGet soon!

@ghost ghost locked as resolved and limited conversation to collaborators Mar 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@andrewleader andrewleader

Labels
bug 🐛 An unexpected issue that highlights incorrect behavior notifications 🔔
Projects
None yet
Milestone
7.0
Development

No branches or pull requests
4 participants
@andrewleader @jmithy @Kyaa-dost @100RH