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

ThemeListener not disposed in sample #3555

Open
Marv51 opened this issue Nov 5, 2020 · 3 comments · May be fixed by #4361
Open

ThemeListener not disposed in sample #3555

Marv51 opened this issue Nov 5, 2020 · 3 comments · May be fixed by #4361
Labels
documentation 📃 helpers ✋ In-PR 🚀 sample app 🖼
Milestone
8.0

Comments

@Marv51
Copy link
Contributor

Marv51 commented Nov 5, 2020

The sample ThemeListenerPage does not implement any kind of disposing for the (IDisposable)ThemeListener.

I believe this should be fixed? Might also be a good idea to update the code sample in the docs.
@ghost ghost added the needs triage 🔍 label Nov 5, 2020
@ghost
Copy link

ghost commented Nov 5, 2020

Hello Marv51, 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 🙌

@Marv51 Marv51 changed the title ThemeListener not disposed ThemeListener not disposed in sample Nov 5, 2020
@michael-hawker
Copy link
Member

Thanks @Marv51, yeah we don't really show how to dispose this. Though I'm wondering @Sergio0694 @azchohfi should this helper be an instance or something vs. creating multiples? Thoughts?

Doc link for reference.

@michael-hawker michael-hawker added this to the 7.0 milestone Nov 5, 2020
@azchohfi
Copy link
Contributor

azchohfi commented Nov 5, 2020

I think the implementation itself is fine. Its only the sample app that is not disposing it properly, so we should update that.

@Kyaa-dost Kyaa-dost modified the milestones: 7.0, 7.1 Dec 8, 2020
@michael-hawker michael-hawker modified the milestones: 7.1, 7.2/8.0? Nov 2, 2021
@XAML-Knight XAML-Knight linked a pull request Nov 2, 2021 that will close this issue
Open
11 tasks
@ghost ghost added the In-PR 🚀 label Nov 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
documentation 📃 helpers ✋ In-PR 🚀 sample app 🖼
Projects
None yet
Milestone
8.0
Development

Successfully merging a pull request may close this issue.

ThemeListener Refactor & Disposal XAML-Knight/WindowsCommunityToolkit
4 participants
@Marv51 @azchohfi @michael-hawker @Kyaa-dost