Refresh fulltext search results when database change#13384
Merged
Siedlerchr merged 6 commits intoJabRef:mainfrom Jun 23, 2025
Merged
Refresh fulltext search results when database change#13384Siedlerchr merged 6 commits intoJabRef:mainfrom
Siedlerchr merged 6 commits intoJabRef:mainfrom
Conversation
…rewrote shouldShow in FileAnnotationTab as requested in JabRef#13279
forcing a search refresh only when the fulltext flag is on, removing and re-adding the flag from the query when the database is switched.
Member
|
Related PR: #11268 |
koppor
reviewed
Jun 22, 2025
Comment on lines
+113
to
+116
| // We need to call this when the database is switched during a fulltext search since | ||
| // the listener on the searchQueryProperty will not fire if the query doesn't change | ||
| // (this causes searchResults in FullTextResultsTab to be empty) | ||
| // https://github.com/JabRef/jabref/issues/13241 |
Member
There was a problem hiding this comment.
Please fix the indnet.
Use Three slashes for JavaDOC method comments.
|
@trag-bot didn't find any issues in the code! ✅✨ |
Siedlerchr
approved these changes
Jun 23, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #13241
We found out the issue was that the
searchResultsin theFullTextResultsTabwould be empty after switching databases, making the tab appear blank. This happened because the listener (and consequently other listeners) on theSearchQueryinsideMainTableDataModelwasn't firing since it didn't detect a different query (oldValue == newValue).As other maintainers suggested, we could have fixed this by emptying the query and then refilling it, but that risked thread-concurrency issues and breaking functions relying on
SearchQuerynot being empty. Another solution was toggling the fulltext flag when switching databases, but that caused extra loading times and weird UI behavior.The solution I went with: just remove the
FULLTEXTflag from thesearchQuery's flag list. This triggers the search refresh. No need to manually re-add the flag because the UI listeners will put it back automatically in the same event cycle. Yeah it's a workaround, but it's cheaper and faster than a full investigation and major refactoring.Mandatory checks
CHANGELOG.mddescribed in a way that is understandable for the average user (if change is visible to the user)