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

Update TaskbarIcon.Declarations.cs #60

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

Tetley22
Copy link

Added support for dynamic, in-memory icons to be assigned at the xaml level. This allows better support for MVVM design.

For a small wpf project I created, I wanted the ability to generate icons at runtime, while maintaining MVVM design pattern. The only way I could figure out how to assign dynamic in-memory icons was to break MVVM and access the TaskbarIcon object directly in the ViewModel code. So I added this ability which worked well for my needs, and thought I'd share if you are interested in it.

Usage in xaml:

        <tb:TaskbarIcon
            x:Name ="ToolbarIcon"
            **IconInMemory="{Binding TrayIcon}"**
           ... />

In ViewModel (where TrayIcon is set via dynamic icon generation):
public Icon TrayIcon
{
get
{
return _trayIcon;
}
set
{
_trayIcon = value;
NotifyPropertyChanged(nameof(TrayIcon));
}
}

hardcodet and others added 2 commits April 22, 2021 13:22
Long-overdue contributor recognition.
Added support for dynamic, in-memory icons to be assigned at the xaml level. This allows better support for MVVM design.
@Lakritzator Lakritzator changed the base branch from master to develop November 29, 2021 21:11
@Lakritzator
Copy link
Collaborator

Thank you for sharing this PR, sorry for the delay but I have a lot of my plate.

I see a bit of an issue with this change, an Icon is a IDisposable, so it needs to be disposed otherwise a memory leak will happen. The OnIconInMemoryPropertyChanged will overwrite it, and not allow it to be disposed.

The issue is that sometimes an icon needs to be disposed and sometimes not (as you will need it), and for these scenarios we might want to have a better solution. I know there were some other issues with icons, I need to consolidate all of that to come up with one... bear with me.

@Lakritzator Lakritzator added the needs clarification More context needed to understand the problem. label Sep 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs clarification More context needed to understand the problem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants