-
Notifications
You must be signed in to change notification settings - Fork 45
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
Feature/quick search #66
base: master
Are you sure you want to change the base?
Conversation
* 1st version of adding system icon for files * added SystemIconsService * continued implementation * changed reminder tag * added support for extension who use uwp apps * added support also to indexing using , * added wip * added support to resolve lnk files * fixed some lnk issues * moved some code to new service IShellLinksService * added fallback to builtin icons, when there is no shell icon * updated comments * renamed enum value * added ShellIconsCacheService * implemented cache * added todos * added ImageModel and csproj to implement it * cleanup * moved System.Drawing.Common to windows project * started adding filter * started adding settings for icons * added comment * some imporvments * fix * added caching of value * added some notes in settings dialog * renamed IconsService to IconsSettingsService * continued rename * added check of Windows * cleanup * cleanup * renamed 'SystemIcon' to 'ShellIcon' * moved strings to resources * fixed comment * cleanup and made code more clear * changed code to be more clear * cleanup * fixed - dont crash on linux * some cleanup and comments * added key down on filters also selects next item * moved code to new service 'quick-search * added stubs for TextInput event * cleanup * add usage of TextInput instead of KeyToChar * added settings dialogs * renamed folder * renamed namespace * renamed file * renamed interface * renmaed enum * renamed member * renamed arg * renamed file * renamed class * added comment * fixed text on label * added explenation * fixed comment * fixed todo * added todo * removed comment * updated comments * updated comment * fixed: make sure to scroll to item * refactoring in GetSelected * fixed going to next row, using same letter repeatedly * refactoring and fix of select of first item * also reset selected index on clear of search * cleanup * fixed icon of keyboard in settings (before was copy of some existing icon) * added help and fixed look of settings, almost no need for groupbox now * added binding to disable row * cleanup * cleanup from unused style classes * also go back when shift pressed * cleanup * moved code to static function so will be more clear * some fixes * more fix * added clear search when switcing folders * added comment * added comment * added comment and cleanup * fixed all build error after sync from fork * removed duplicate of file which was moved to other folder * cleanup * cleanup 2 * cleanup and comments * added comments * removed un-needed file * added comments + cleanup * cleanup 3 * added comments + cleanup 2 * remove unused file * added comment * cleanup
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #66 +/- ##
==========================================
- Coverage 92.89% 91.42% -1.48%
==========================================
Files 185 192 +7
Lines 4830 4967 +137
Branches 388 403 +15
==========================================
+ Hits 4487 4541 +54
- Misses 288 368 +80
- Partials 55 58 +3 ☔ View full report in Codecov by Sentry. |
|
||
void ClearSearch(); | ||
|
||
bool Enabled(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds like this should be a property and not a method
public record QuickSearchFileModel | ||
{ | ||
public string Name { get; init; } | ||
public object Tag { get; init; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this property object
?
public void OnCharDown(char c, | ||
bool isShiftDown, | ||
List<QuickSearchFileModel> files, | ||
out bool handled) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why out
instead of returning bool
from the method?
} | ||
|
||
public void OnDataGridKeyDownCallback(Key key) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ViewModels by definition should not have code that looks like code behind callback method. This method should be renamed to represent the business logic it contains
// "cycle from last to first" | ||
// reset, so and start again from first | ||
// Done in 2 'half' loops, in sake of effiency. | ||
if (!isShiftDown) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like with this logic it will iterate over the files twice?
I will be working on refactoring this PR. It will take me at least a couple of days. Few bugs that I've noticed:
TODOs left:
|
Update: I managed to rewrite files filtering code but still need to address selection logic changes. I'll keep the code locally for now because it's working only partially at the moment |
Pushed a refactored revision, here is what's left:
|
Filters out item as you time.
Can be controlled via settings
as was requested here - #59