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

WeakEventListener in NetworkConnectionStateTrigger is duplicated #4059

Open
michael-hawker opened this issue Jun 1, 2021 · 1 comment
Open

WeakEventListener in NetworkConnectionStateTrigger is duplicated #4059

michael-hawker opened this issue Jun 1, 2021 · 1 comment
Assignees
@Arlodotexe
Labels
bug 🐛 An unexpected issue that highlights incorrect behavior good first issue Issues identified as good for first-time contributors Hacktoberfest Hacktoberfest is a month-long celebration of open source software help wanted Issues identified as good community contribution opportunities improvements ✨
Milestone
8.0

Comments

@michael-hawker
Copy link
Member

Describe the bug

Looks like NetworkConnectionStateTrigger is using a private copy of WeakEventListener, it's identical to the one we already have in the Toolkit, so we should update the source to use the existing code within the Toolkit.

https://github.com/windows-toolkit/WindowsCommunityToolkit/blob/ebb12e0c38c22ec01849e202f8e477d8c1a67061/Microsoft.Toolkit.Uwp.UI/Triggers/NetworkConnectionStateTrigger.cs#L73

Expected behavior

Use existing code over shadow copy.

https://github.com/windows-toolkit/WindowsCommunityToolkit/blob/ebb12e0c38c22ec01849e202f8e477d8c1a67061/Microsoft.Toolkit.Uwp/Helpers/WeakEventListener.cs#L17
@michael-hawker michael-hawker added bug 🐛 An unexpected issue that highlights incorrect behavior help wanted Issues identified as good community contribution opportunities improvements ✨ good first issue Issues identified as good for first-time contributors labels Jun 1, 2021
@michael-hawker michael-hawker added this to the 7.1 milestone Jun 1, 2021
@ghost ghost removed this from the 7.1 milestone Sep 30, 2021
@michael-hawker michael-hawker added the Hacktoberfest Hacktoberfest is a month-long celebration of open source software label Oct 7, 2021
@puppetsw puppetsw mentioned this issue Jan 10, 2022
Open
11 tasks
@Arlodotexe Arlodotexe added this to the 7.1.3 milestone Sep 15, 2022
@Arlodotexe Arlodotexe moved this to In Progress in Windows Community Toolkit Sep 15, 2022
@Arlodotexe Arlodotexe self-assigned this Sep 15, 2022
@Arlodotexe
Copy link
Member

While closing #4450, we noticed that the code is not identical to what we have in the toolkit. It removes TEventArgs to allow us to do NetworkInformation.NetworkStatusChanged += weakEvent.OnEvent;.

After discussing with @michael-hawker, we won't be removing the private event listener for now. Instead, we'll use the more flexible CommunityToolkit/dotnet#404 when it arrives.

@Arlodotexe Arlodotexe mentioned this issue Sep 20, 2022
Closed
13 tasks
@michael-hawker michael-hawker modified the milestones: 7.1.3, 8.0 Oct 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Arlodotexe Arlodotexe

Labels
bug 🐛 An unexpected issue that highlights incorrect behavior good first issue Issues identified as good for first-time contributors Hacktoberfest Hacktoberfest is a month-long celebration of open source software help wanted Issues identified as good community contribution opportunities improvements ✨
Projects
Windows Community Toolkit
Status: In Progress
Milestone
8.0
Development

No branches or pull requests
2 participants
@Arlodotexe @michael-hawker