-
-
Notifications
You must be signed in to change notification settings - Fork 397
Fix history results clear logic & Make sure results cleared #3525
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
Changes from all commits
3718796
54d50cf
5784f87
3484d08
021b451
6e473c8
c8989c3
3436ac9
08447da
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,6 +34,7 @@ public partial class MainViewModel : BaseModel, ISavable, IDisposable | |
private bool _isQueryRunning; | ||
private Query _lastQuery; | ||
private bool _previousIsHomeQuery; | ||
private bool _needClearResults; | ||
private string _queryTextBeforeLeaveResults; | ||
private string _ignoredQueryText; // Used to ignore query text change when switching between context menu and query results | ||
|
||
|
@@ -1246,19 +1247,7 @@ private async Task QueryResultsAsync(bool searchDelay, bool isReQuery = false, b | |
|
||
if (query == null) // shortcut expanded | ||
{ | ||
App.API.LogDebug(ClassName, $"Clear query results"); | ||
|
||
// Hide and clear results again because running query may show and add some results | ||
Results.Visibility = Visibility.Collapsed; | ||
Results.Clear(); | ||
|
||
// Reset plugin icon | ||
PluginIconPath = null; | ||
PluginIconSource = null; | ||
SearchIconVisibility = Visibility.Visible; | ||
|
||
// Hide progress bar again because running query may set this to visible | ||
ProgressBarVisibility = Visibility.Hidden; | ||
ClearResults(); | ||
return; | ||
} | ||
|
||
|
@@ -1284,8 +1273,6 @@ private async Task QueryResultsAsync(bool searchDelay, bool isReQuery = false, b | |
// Update the query's IsReQuery property to true if this is a re-query | ||
query.IsReQuery = isReQuery; | ||
|
||
|
||
|
||
ICollection<PluginPair> plugins = Array.Empty<PluginPair>(); | ||
if (currentIsHomeQuery) | ||
{ | ||
|
@@ -1342,6 +1329,7 @@ private async Task QueryResultsAsync(bool searchDelay, bool isReQuery = false, b | |
|
||
// plugins are ICollection, meaning LINQ will get the Count and preallocate Array | ||
|
||
var resultsCleared = false; | ||
Task[] tasks; | ||
if (currentIsHomeQuery) | ||
{ | ||
|
@@ -1376,6 +1364,24 @@ private async Task QueryResultsAsync(bool searchDelay, bool isReQuery = false, b | |
// nothing to do here | ||
} | ||
|
||
// If the query is cancelled, part of results may be added to the results already. | ||
// But we should not clear results now | ||
// because typing very fast will cause many calls to Results.Clear() which leads to flickering issue | ||
// Instead, we should clear the results next time we update the results | ||
if (currentCancellationToken.IsCancellationRequested) | ||
{ | ||
_needClearResults = true; | ||
return; | ||
} | ||
|
||
// If QueryTaskAsync or QueryHistoryTask is not called which means that results are not cleared | ||
// we need to clear the results | ||
if (!resultsCleared) | ||
{ | ||
ClearResults(); | ||
return; | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this really necessary? This could cause unexplained/unexpected flicker There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We must need it. This is used to make sure results are cleared when there are not any update tasks. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Think about one situation. Home page feature is off and you just change query text from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When you say not any update task are you referring to one example where you toggle off home page in settings and results are not cleared? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let me give you one demo video There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jjw24 It seems that I have issues when uploading video here. Please see more in Discord. |
||
if (currentCancellationToken.IsCancellationRequested) return; | ||
|
||
// this should happen once after all queries are done so progress bar should continue | ||
|
@@ -1389,6 +1395,23 @@ private async Task QueryResultsAsync(bool searchDelay, bool isReQuery = false, b | |
} | ||
|
||
// Local function | ||
void ClearResults() | ||
{ | ||
App.API.LogDebug(ClassName, $"Clear query results"); | ||
|
||
// Hide and clear results again because running query may show and add some results | ||
Results.Visibility = Visibility.Collapsed; | ||
Results.Clear(); | ||
|
||
// Reset plugin icon | ||
PluginIconPath = null; | ||
PluginIconSource = null; | ||
SearchIconVisibility = Visibility.Visible; | ||
|
||
// Hide progress bar again because running query may set this to visible | ||
ProgressBarVisibility = Visibility.Hidden; | ||
} | ||
|
||
async Task QueryTaskAsync(PluginPair plugin, CancellationToken token) | ||
{ | ||
App.API.LogDebug(ClassName, $"Wait for querying plugin <{plugin.Metadata.Name}>"); | ||
|
@@ -1439,6 +1462,7 @@ await PluginManager.QueryHomeForPluginAsync(plugin, query, token) : | |
var shouldClearExistingResults = ShouldClearExistingResults(query, currentIsHomeQuery); | ||
_lastQuery = query; | ||
_previousIsHomeQuery = currentIsHomeQuery; | ||
resultsCleared = true; | ||
|
||
if (!_resultsUpdateChannelWriter.TryWrite(new ResultsForUpdate(resultsCopy, plugin.Metadata, query, | ||
token, reSelect, shouldClearExistingResults))) | ||
|
@@ -1458,8 +1482,14 @@ void QueryHistoryTask(CancellationToken token) | |
|
||
App.API.LogDebug(ClassName, $"Update results for history"); | ||
|
||
// Indicate if to clear existing results so to show only ones from plugins with action keywords | ||
var shouldClearExistingResults = ShouldClearExistingResults(query, currentIsHomeQuery); | ||
_lastQuery = query; | ||
_previousIsHomeQuery = currentIsHomeQuery; | ||
resultsCleared = true; | ||
|
||
if (!_resultsUpdateChannelWriter.TryWrite(new ResultsForUpdate(results, _historyMetadata, query, | ||
token))) | ||
token, reSelect, shouldClearExistingResults))) | ||
{ | ||
App.API.LogError(ClassName, "Unable to add item to Result Update Queue"); | ||
} | ||
|
@@ -1558,6 +1588,13 @@ private async Task BuildQueryAsync(IEnumerable<BaseBuiltinShortcutModel> builtIn | |
/// <returns>True if the existing results should be cleared, false otherwise.</returns> | ||
private bool ShouldClearExistingResults(Query query, bool currentIsHomeQuery) | ||
{ | ||
// If the results are not cleared temporarily, we need to clear this time | ||
if (_needClearResults) | ||
{ | ||
_needClearResults = false; | ||
return true; | ||
} | ||
|
||
// If previous or current results are from home query, we need to clear them | ||
if (_previousIsHomeQuery || currentIsHomeQuery) | ||
{ | ||
|
Uh oh!
There was an error while loading. Please reload this page.