search

srwi / EverythingToolbar

Everything integration for the Windows taskbar.
Other
9.75k stars 422 forks source link

Fixes #481 - Keep selected filter option between application restarts. #503

Closed zeyus closed 3 months ago

zeyus commented 3 months ago

This PR fixes https://github.com/srwi/EverythingToolbar/issues/481 so the selected filter is saved on selection change, and updates the tab and combo to only update if the element is under the mouse cursor or focused.

I implemented it this way because I'm not familiar with C# and couldn't figure out why on Show() it would fire the SelectionChanged event...In the code there should be nothing triggering it (it kept defaulting back to All even though when I was debugging, the Settings.Default.lastFilter showed the correctly selected filter.

Anyway, if you have a suggestion on how to avoid that issue and maybe bind the SelectedIndex directly (I tried that and it still fired on Show()) let me know, and I will update the PR, otherwise, this works. :)

srwi commented 3 months ago

Hi @zeyus, thanks a lot for your contribution. I remember trying this briefly and running into similar problems. The solution feels a little hacky, but then again I also don't know a better solution. One thing that comes to mind: With the focus checks on the ComboBox and TabControl, is it still possible to select filters using the keyboard shortcuts Tab/Shift+Tab?

zeyus commented 3 months ago

Hi @zeyus, thanks a lot for your contribution. I remember trying this briefly and running into similar problems. The solution feels a little hacky, but then again I also don't know a better solution. One thing that comes to mind: With the focus checks on the ComboBox and TabControl, is it still possible to select filters using the keyboard shortcuts Tab/Shift+Tab?

No probs @srwi, least I can do seeing as I'm enjoying using EverythingToolbar :)

I agree it feels a little hacky, although this does specifically check that the selected item change is due to a user interaction rather than the initial population / sync with binding, so conceptually it makes sense.

What I don't understand is that there isn't an obvious way to prevent population/syncing from firing the events (or to bind the handlers after all items are loaded). Maybe it requires way more C# foo than I have haha.

Just confirmed keyboard shortcuts are working as expected (because the tab/combo elements have isFocused == True when tabbing to them).

tab-shift-tab

srwi commented 3 months ago

I agree it feels a little hacky, although this does specifically check that the selected item change is due to a user interaction rather than the initial population / sync with binding, so conceptually it makes sense.

You're right. I think it is fine. Especially since it's well contained within the FilterSelector control.

What I don't understand is that there isn't an obvious way to prevent population/syncing from firing the events (or to bind the handlers after all items are loaded). Maybe it requires way more C# foo than I have haha.

Probably the correct way would be to bind the currently selected filter to both SelectedIndexs with a value converter rather than listening to the property changing, but then I don't really know whether it would help at all with the problem you're describing.

In any case, I really like your solution and it works quite well for me, so I'll merge the PR. Thanks again for your contribution!