Add App Filtering Page and Session Validation (ref #633)#652
Conversation
- Add dedicated "App Filter" page - Add "App Filtering" menu card and navigation item - Add app filtering properties to settings storage - Add app filtering keys to en-US dictionary - Update main menu to use `GetActiveMediaSession()` over `GetFocusedSession()` - Update control click handlers to use `GetActiveMediaSession()`
- Update formatting from K&R to BSD (C# standard)
- Add taskbar & flyout refresh once a filter has been updated
github-actions
Bot
added
MainWindow / Media Flyout
|
A quick comment about the formatting, I come from a Java/C++ background so I had originally formatted it with braces on the same line. I changed it in one of the commits, but if there is still an issue with it let me know and I will try my best to fix it. Thanks! |
|
Found a small bug regarding pause/play, I will try fixing it today. |
- Update `PlayPause_Click()` in both `MainWindow.xaml.cs` and `TaskbarWidgetControl.xaml.cs` to use `TryTogglePlayPauseAsync()` over the previous keyboard approach to respect app filtering - Update UI logic to check `PlaybackStatus` over `Controls.IsPauseEnabled` to prevent desync from filters NOTE: ControlPlayPause opacity has been moved outside of the conditional due to making more sense there.
|
Fixed the bug above, I changed the old implementation that relied on manually sending a keyboard input. |
unchihugo
left a comment
unchihugo
left a comment
There was a problem hiding this comment.
Hi @galagyy, solid implementation so far! Just a few things I've noticed throughout that we should adjust before merging. I've noted it in the comments.
I'd like to discuss the implementation of the filter lists:
Currently, names in the filter lists aren't sanitized - we should have this feature utilize our MediaPlayerData class. Methods in here can find the sanitized title + icon for apps/media players, especially with the changes I've added myself in the Volume Flyout branch here.
Could you adapt the current code to use the GetMediaPlayerData() method in MediaPlayerData instead of ExtractAppName()? That would be great!
Once the current issues/questions are addressed, we can continue with the merge :)
| if (taskbarSession == null) | ||
| { | ||
| taskbarWindow?.UpdateUi("-", "-", null, GlobalSystemMediaTransportControlsSessionPlaybackStatus.Closed); | ||
| return; |
There was a problem hiding this comment.
I believe the early return here is justified unless I'm mistaken, and adding the else clause isn't necessary either. I think we should bring the structure back to a similar state of what it was before (early return if null, no else needed).
What happens now when focusedSession == null:
taskbarSession = GetActiveMediaSession(); // first check, null
update taskbar ui, but don't return
focusedSession = GetActiveMediaSession(); // a second time, null again
if (focusedSession == null) return; // returns after calling same method again
Additionally, variable taskbarSession is the same as focusedSession. We should keep the focusedSession naming!
|
|
||
| var thumbnail = BitmapHelper.GetThumbnail(songInfo.Thumbnail); | ||
| BitmapHelper.GetDominantColors(1); | ||
| taskbarWindow?.UpdateUi(songInfo.Title, songInfo.Artist, thumbnail, playbackInfo.PlaybackStatus, playbackInfo.Controls); |
There was a problem hiding this comment.
If I'm not mistaken, this change is unnecessary right? Correct me if I'm wrong!
There was a problem hiding this comment.
Yes, you are correct! I have gone ahead and fixed it.
| @@ -0,0 +1,141 @@ | |||
| // Copyright � 2024-2026 The FluentFlyout Authors | |||
There was a problem hiding this comment.
For some reason, the copyright symbol is a question mark here.
- Removed the `ExtractAppName()` function in favor of using the already-existing `getMediaPlayerData()` function
- Reintroduced early return statement - Renamed `taskbarSession` back to `focusedSession` - Removed unneeded else indentation
- Replaced malformed unicode with proper copyright unicode
|
Hello, thank you for the prompt review! I have addressed all the issues you pointed out earlier:
I also had noticed that all the other tabs have an image describing their function with this specifically standing out as one without; would you like me to try to find an image suitable for it? Thank you! |
unchihugo
left a comment
unchihugo
left a comment
There was a problem hiding this comment.
Hi, thanks for the quick refactor, though could you answer my questions I've noted in the code review? Just asking since I'm a bit busy and can't load up the build right now :)
I also had noticed that all the other tabs have an image describing their function with this specifically standing out as one without; would you like me to try to find an image suitable for it?
I was thinking of having this be a subpage from the System page instead (kind of like the Advanced Settings button there). Therefore we wouldn't need an image for it either currently!
PS: have you tested whether the manual adding works?
| if (mainWindow?.mediaManager != null) | ||
| { | ||
| var apps = mainWindow.mediaManager.CurrentMediaSessions.Values | ||
| .Select(s => MediaPlayerData.getMediaPlayerData(s.Id).Item1) |
There was a problem hiding this comment.
Could you explain why you have to specify Item1 here?
There was a problem hiding this comment.
I chose to specifically pick Item1 because the getMediaPlayerData() function returns both (string, ImageSource) and I specifically need only the title of the application.
|
Hello, thanks for the response! I will try moving it to the Systems page as you specified earlier. Regarding manual adding, it has worked with the apps I've tried (e.g. Chrome, Edge, Firefox, etc.) but I haven't come across an app that wouldn't be displayed on the default dropdown yet; I will try to test that case soon. I will have the changes done hopefully by the end of today; I will try adding pictures or a video if you are unable to build it currently. |
|
Okay, that makes sense - will wait for updates! |
|
Currently I have these for the system, I will post further images/videos in some time. I've removed the tab for App Filtering in the left-side hamburger menu and placed it here instead. The UI for the actual App Filtering page looks a little something like this: I was able to confirm that manually adding an app works for all apps that I had tested from. I am working on removing it from the main page and tidying up the code a bit. |
|
That's good! I'm thinking about the user experience with the current UI - it took me a bit to figure out what was happening myself, so I think we should definitely improve UX before we ship this to average users. We can brainstorm this after you're ready. A lot of users on GitHub called this feature something like whitelist/blacklist instead of filtering. |
|
Hello! Yes, the UX needs to be improved, if you have any ideas I am open to them as I am not very good at UX myself. I'm about to commit the changes where the button is removed from the hamburger menu and main menu; it should hopefully be a matter of naming and UX now. |
- Move the app filtering feature to the system menu
- Fix `.exe` parsing when adding custom application - Inverted if statements for easier readability - Changed type casting to use the `as` keyword for readability
- Update app comparison to compare manual additions against dropdown selections - Move `.exe` comparison within new comparison method
|
Hello, Thank you for the comment! I will attempt to fix those issues today. |
|
Hey, sorry to intrude again. The PR is looking good but if you'd allow me to make one final suggestion, I'm wondering if there should be a Label next to the mode dropdown ( 
The "App Language" case is very good in my opinion because it also adds a smal subtext with an explanation. I think a good Label for this element could be "Execution Mode" |
|
That's a great point, I like the "App Language" case you brought up; I'll try changing it to that in some time after I address the improper app name issue. On that case, do you have an idea of what the explanation could be? |
|
With the image above, I've changed it to what @darklinkpower suggested with a short message explaining it, although I can change it to a more comprehensive one if everyone believes that would be better. 
|
- Change the whitelist and blacklist dropdown to have a label and descriptor alongside the dropdown - Change page formatting to be better organized - Remove parenthetical explanation of each option
- Reduce `CornerRadius` of the allowed and blocked apps elements from 8 to 4 to ensure it is synonymous with the rest of the project
|
We could try "Filtering Mode", since I think it sounds clearer than Execution Mode. As for the subtitle: the current one sounds a bit technical and I'm not sure if the average user will understand what it means. Any other ideas for that? Btw, when you add strings to the dictionary and we merge the PR to master, all translators will work with those merged strings, that's why I believe it's important to get the strings right before merging :) |
- Fix improper app name showing up as a result of background workers
That's a good point with the strings, I have a few ideas of what we could change it to in order to make it more user friendly. I agree with changing the title to "Filter Mode," as that is more representative of what it's actually doing. For the description, I was thinking we could change it to something simple such as "Choose which filter list is active," although we could make it more informative if everyone feels that's better. I was also thinking of changing the dropdown options from Whitelist/Blacklist to Allow List/Block List as other applications (Google's UI, Apple's UI, etc.) have started using this to be more user friendly.
With regards to this, I believe I have fixed the issue as it no longer replicates on my own machine (I removed a piece checking for background workers when I shouldn't have); do you have the names of the other apps that caused this issue? Thanks, |
I think "Filtering Mode" sounds great. Regarding the subtitle, how about:
In my opinion it pairs well with the top "Enable Filtering" toggle, although I'd change this one to simply "Control which applications can display media" to explain the feature at a high level, whereas the dropdown goes into more specifics. 
|
|
Thanks for the suggestions, this is what it's looking like right now: 
I'm working on making it (the Allowed/Excluded Apps sections) a dropdown as suggested before. |
- Change dropdown order to have the allow list as the first option - Change the English to be more user friendly
- Add dropdown to "Allow App" and 'Exclude App" sections
@unchihugo All the changes you had pointed out should be complete now. I have tested it with a fair share of apps, but I will try testing/looking for more edge cases. 
|
unchihugo
left a comment
unchihugo
left a comment
There was a problem hiding this comment.
Hi @galagyy, I'm ready to merge after these changes:
- Could you move the
MediaPlayerData.cschanges out of this current branch? They don’t seem relevant to the PR (unless I'm mistaken, I'd like to hear your thoughts), so separating them would be helpful. - Also, I think it’s better to keep blacklist as the default rather than switching to whitelisting :)
- Change default dropdown to be "blacklist"
|
Hello, Thanks for the prompt reply! I've gone ahead and made the blacklist the default mode as pointed out. Regarding the changes to I noticed that certain apps would fail to filter with the old code. Since some apps handle audio through background workers, the old check against As a result of the above I feel the changes are needed specifically for the app filtering, but I could try testing again with the old implementation as I originally tested a few commits ago; I do doubt the code would work though with the caveats pointed above. |
- Fix dotnet style formatting
|
Thanks for the follow-up! I see the issue now. I'll have a look into this. |
- Change `.gitattributes` to have a consistent EOL for all `*.cs` files
|
I'm trying to fix the dotnet format issue but I'm unable to figure out why it's failing; I looked at the EOF issue the formatter had mentioned but am unable to find it on my end. |
I just pushed commit c1ec46c to master to fix the issue. You can pull the changes to your branch and it should fix the check! |
|
I've gone ahead and pulled the changes, but I still get this particular error: I've checked the newline and it appears to be fine, I'm not too sure what it's asking me to resolve. |
There was a problem hiding this comment.
Hi @galagyy, I just tested the changes and it's great - we're nearing the finish now and I'm excited to get this feature in the app with you. I found a breaking issue though:
The new MediaPlayerData.cs is incredibly CPU resource intensive (I believe it's searching through way too many processes now), resulting in ~3-5 second freezes when the GetAndCacheMediaPlayerData method tries to find a new media player. On my end, when I reverted the changes to that file back to master, it works flawlessly. Perhaps it would be better for us to revert the changes to this file and start a new PR for background process tracking.
Another thing I've noticed is that GetActiveMediaSession() in MainWindow.xaml.cs searches for the focused session session first, then falls back to the first playing session. Previously it would not check if the status was playing: this is a change not related to the PR, and I'm thinking we should simply return validSessions.FirstOrDefault() instead of check whether the session is playing like how it used to work, and open a now PR for this as well.
I'm sorry I'm dragging this PR, but I definitely believe that after these two changes we can safely merge the PR to master!
EDIT: I'll handle the formatting later, it's okay to leave it!
|
Hello! No worries for dragging out the PR request, in the end it's better for everyone! I've learned a lot about C# from this PR. With regards to the issues, I will try reverting them and making sure it works properly with the code. |
I'm probably missing something but I think it's an easy fix. Right now the issues I see in the loop are:
To fix this it may be enough to leave the loop early when a process with the max possible priority is found. 
Here's how I'd modify the method, apologies if formatting is incorrect or if there's something wrong, I wrote it with a text editor and might have missed something: private sealed class ProcessMatch
{
public required Process Process { get; init; }
public required string Path { get; init; }
public required int Priority { get; init; }
}
public static (string, ImageSource?) GetAndCacheMediaPlayerData(string mediaPlayerId)
{
if (mediaPlayerCache.TryGetValue(mediaPlayerId, out var cachedInfo)
|| mediaPlayerIdVariants.TryGetValue(mediaPlayerId, out var variantKey)
&& mediaPlayerCache.TryGetValue(variantKey, out cachedInfo))
{
return (cachedInfo.Title, cachedInfo.Icon);
}
string mediaTitle = mediaPlayerId;
ImageSource? mediaIcon = null;
// get sanitized media title name
string[] mediaSessionIdVariants = mediaPlayerId.Split('.');
// remove common non-informative substrings
var variants = mediaSessionIdVariants.Select(variant =>
variant.Replace("com", "", StringComparison.OrdinalIgnoreCase)
.Replace("github", "", StringComparison.OrdinalIgnoreCase)
.Replace("exe", "", StringComparison.OrdinalIgnoreCase)
.Trim()
).Where(variant => !string.IsNullOrWhiteSpace(variant)).ToList();
// add original id to the end of the array to ensure at least one variant
variants.Add(mediaPlayerId);
Process[] processes;
// use cache to avoid frequent process enumeration
if (cachedProcesses == null || (DateTime.Now - lastCacheTime).TotalSeconds > CACHE_DURATION_SECONDS)
{
cachedProcesses = Process.GetProcesses();
lastCacheTime = DateTime.Now;
}
ProcessMatch? bestProcessMatchInfo = null;
foreach (var p in cachedProcesses)
{
try
{
// we no longer instantly filter out processes without a main window handle,
// to support background media processes (chrome, edge, etc. may spawn background
// workers).
var module = p.MainModule;
if (module == null)
{
continue;
}
string path = module.FileName;
bool matches = false;
foreach (var variant in variants)
{
if (path.Contains(variant, StringComparison.OrdinalIgnoreCase))
{
matches = true;
break;
}
}
if (!matches)
{
continue;
}
// prioritize processes that have a main window, as they represent the primary app (e.g., Microsoft Edge) rather than a background worker
int priority = p.MainWindowHandle != IntPtr.Zero ? 1 : 0;
if (bestProcessMatchInfo == null || priority > bestProcessMatchInfo.Priority)
{
bestProcessMatchInfo = new ProcessMatch
{
Process = p,
Path = path,
Priority = priority
};
// Break early if maximum priority was obtained
if (priority == 1)
{
break;
}
}
}
catch (Win32Exception)
{
}
catch (InvalidOperationException)
{
}
}
if (bestProcessMatchInfo == null)
{
return (mediaTitle, mediaIcon);
}
try
{
// prioritize ProductName for the user-facing app name, but fall back to `FileDescription` or `MainWindowTitle` if `ProductName` is not available
var versionInfo = bestProcessMatchInfo.Process.MainModule?.FileVersionInfo;
mediaTitle =
!string.IsNullOrWhiteSpace(versionInfo?.ProductName)
? versionInfo.ProductName
: !string.IsNullOrWhiteSpace(versionInfo?.FileDescription)
? versionInfo.FileDescription
: bestProcessMatchInfo.Process.MainWindowTitle;
}
catch
{
}
// check cache again because we have the sanitized title
if (mediaPlayerCache.TryGetValue(mediaTitle, out cachedInfo))
{
// map the original id to the sanitized title for future lookups
mediaPlayerIdVariants[mediaPlayerId] = mediaTitle;
return (cachedInfo.Title, cachedInfo.Icon);
}
mediaIcon = GetIconFromPath(bestProcessMatchInfo.Path);
mediaPlayerCache[mediaPlayerId] = new CachedMediaPlayerInfo
{
Title = mediaTitle,
Icon = mediaIcon,
ProcessId = bestProcessMatchInfo.Process.Id
};
return (mediaTitle, mediaIcon);
}The idea is to break early from the loop if a match is found and move expensive calls outside the loop, so they are only processed for a single one of them. This also removes a few LINQ processing calls although it's not like they would make any realistic difference. |
|
Hello! Sorry for the short hiatus, I'll work on this in a little bit; something had came up in the past week. |
3 participants
Summary
This PR introduces an app filtering feature. I had seen a feature request, specifically #633, which discusses an idea for a feature similar to this.
This allows the user to create a custom "Allow list" and "Block list" where users may pick which apps they want affecting the flyout and taskbar player.
Motivation
As mentioned above, feature request #633 introduced an idea of a feature similar to this. This request has more features they want added so I do not suggest closing it yet.
Type of Change
What Changed
IsSessionAllowed(), to validate the current session. This function has replaced the previousGetFocusedSession().Additional Information
Originally I had the filter list housed under the taskbar section; I have moved it to its own section due to how it affects both the taskbar and flyout.
NOTE: I have also gone ahead and added the copyright disclaimer on top of the files added, but I am unsure if I was supposed to do that.
Checklist