Fixed search result focus handling by ZodGaz · Pull Request #13174 · JabRef/jabref

ZodGaz · 2025-05-27T17:25:26Z

Closes #12865
Implemented handleFocus() in FulltextSearchResultsTab , now search results tab shows correctly after a fulltext search, and disappear after the fulltextsearch flag is disabled; fixed also an issue where sometimes the search results tab would be empty after a successful search.

Mandatory checks

I own the copyright of the code submitted and I license it under the MIT license
Change in CHANGELOG.md described in a way that is understandable for the average user (if change is visible to the user)
Tests created for changes (if applicable)
Manually tested changed features in running JabRef (always required)
Screenshots added in PR description (if change is visible to the user)
Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

…r a fulltext search; fixed also an issue where sometimes the search results tab would be empty after a successful search

…ltext search; fixed also an issue where sometimes the search results tab would be empty after a successful search

subhramit · 2025-05-27T19:33:02Z

@koppor determine issue number check failed https://github.com/JabRef/jabref/actions/runs/15281990665/job/42983040404?pr=13174

koppor · 2025-05-27T20:49:27Z

@koppor determine issue number check failed JabRef/jabref/actions/runs/15281990665/job/42983040404?pr=13174

Contributor used Closes [#12856](https://github.com/JabRef/jabref/issues/12865). One out of 10 persons use this strange format. One year ago, we had a help text on it that people should just add the number, or the link - and not both.

NOTE THAT THE LINK POINTS TO ANOITHER ISSUE

Option A: We try to parse also these kind of things - and also report errors if the link text does not match the issue number
Option B: Refine the comment text 😅

Update I needed to fix the job name - 5b5cba8

koppor · 2025-05-27T21:03:55Z

        }
        this.entry = entry;
        updateSearch();
+        notifyAboutFocus(entry);


Are you sure that this is needed? - I think, Platform.runLater(entryEditor::adaptVisibleTabs) could be directly called - or another method adaptVisisbleTabs() containing this call could be used. focus is wrong IMHO.

Actually is not needed, just tested it. Sorry I'm just starting to get familiar with JavaFX.

subhramit · 2025-05-27T21:10:59Z

Option A: We try to parse also these kind of things - and also report errors if the link text does not match the issue number

Option B: Refine the comment text 😅

None, 1 out of 10 is a bearable edge case. I think this is rarer than 1 out of 10.

…abs; fixed incorrect spacing

koppor · 2025-06-01T14:06:38Z

@ZodGaz please fix your issue link - [#12856](https://github.com/JabRef/jabref/issues/12865). 12856 is not 12865

ZodGaz · 2025-06-01T15:06:19Z

@ZodGaz please fix your issue link - [#12856](https://github.com/JabRef/jabref/issues/12865). 12856 is not 12865

done, thank you

koppor

Besides minor formatting comments, looksgood

koppor

As usual? CHANGELOG.md entry missing.

ZodGaz · 2025-06-02T14:15:38Z

As usual? CHANGELOG.md entry missing.

Done, let me know if everything else is needed

Co-authored-by: Oliver Kopp <kopp.dev@gmail.com>

trag-bot · 2025-06-02T14:22:42Z

@trag-bot didn't find any issues in the code! ✅✨

koppor · 2025-06-02T14:25:37Z

I think, this solves the issue.

I am wondering if the tab "fulltext search results" should be shown if search was triggered. However, this is more hard and we need to discuss internally @Siedlerchr

…or-test * upstream/main: (102 commits) Try to fix output Improve AI preferences UI and templates (JabRef#13202) Bump jablib/src/main/abbrv.jabref.org from `6926b83` to `333c2f1` (JabRef#13216) Bump jablib/src/main/resources/csl-styles from `8a2317a` to `c3df987` (JabRef#13215) Fixed search result focus handling (JabRef#13174) New Crowdin updates (JabRef#13214) Add Pseudonymization to CLI (JabRef#13158) Try parallel gource build Update gource.yml Fix position of checkout Preapre: Enable gradle configuration cache (JabRef#13212) Add yml as YAML extension (JabRef#13213) Fix wrong detection of issue numbers (JabRef#13211) Miscellaneous refactoring - II (JabRef#13197) Run Windows tests only on main (and on demand) (JabRef#13210) Fix porcelain for consistency check (JabRef#13209) Use setup-jbang action (instead of custom call of .sh script) (JabRef#13208) Add link to JabRef guru (JabRef#13207) Switch to gradlex for modularity (JabRef#13112) feat(ci-cd): change issue URL pattern (JabRef#13206) ...

koppor · 2025-06-03T14:51:26Z

@ZodGaz can you please check? I think your PR introduced issue #13241

You can check by git revert {commit id}

subhramit · 2025-06-03T15:00:05Z

        tabs.add(sourceTab);
        tabs.add(new LatexCitationsTab(preferences, dialogService, stateManager, directoryMonitor));
-        tabs.add(new FulltextSearchResultsTab(stateManager, preferences, dialogService, taskExecutor));
+        tabs.add(new FulltextSearchResultsTab(stateManager, preferences, dialogService, taskExecutor, this));


This seems like the flaw - there is only one entry editor, so the state needs to refresh in each tab

Still happens before my PR, tested just now

koppor · 2025-06-03T15:21:39Z

-        content.getChildren().clear();
        stateManager.activeSearchQuery(SearchType.NORMAL_SEARCH).get().ifPresent(searchQuery -> {
            SearchResults searchResults = searchQuery.getSearchResults();
            if (searchResults != null && entry != null) {


Maybe add a breakpoint here and see when it is called?

This is the moment I come back from the switched library to where I do the fulltext search:

This is every-time I try to open a search results tab (that happens to be blank)

If I reset the fulltext search flag straight after coming back from the switched library this does not happens. So basically entry is null when I come back from the switched one and then searchResults is always null while I'm in the library where I do the search (after coming back) .

ZodGaz

sorry for late response, didn't know I was the one that has to confirm the pending review

ZodGaz · 2025-06-03T15:16:36Z

-        content.getChildren().clear();
        stateManager.activeSearchQuery(SearchType.NORMAL_SEARCH).get().ifPresent(searchQuery -> {
            SearchResults searchResults = searchQuery.getSearchResults();
            if (searchResults != null && entry != null) {


I think after coming back from the switched tab one of these is always null for some reason until toggling the search flag off and on again. (line 97-102)

ZodGaz · 2025-06-03T15:34:14Z

-        content.getChildren().clear();
        stateManager.activeSearchQuery(SearchType.NORMAL_SEARCH).get().ifPresent(searchQuery -> {
            SearchResults searchResults = searchQuery.getSearchResults();
            if (searchResults != null && entry != null) {


This is the moment I come back from the switched library to where I do the fulltext search:

This is every-time I try to open a search results tab (that happens to be blank)

If I reset the fulltext search flag straight after coming back from the switched library this does not happens. So basically entry is null when I come back from the switched one and then searchResults is always null while I'm in the library where I do the search (after coming back) .

ZodGaz added 2 commits May 27, 2025 19:05

fixed issue JabRef#12856, now search results tab shows correctly afte…

84df999

…r a fulltext search; fixed also an issue where sometimes the search results tab would be empty after a successful search

fixed issue 12856, now search results tab shows correctly after a ful…

89e806e

…ltext search; fixed also an issue where sometimes the search results tab would be empty after a successful search

Siedlerchr changed the title ~~fixed issue #12856~~ Fixed search result focus handling May 27, 2025

koppor mentioned this pull request May 27, 2025

Refine text for referencing #13176

Merged

1 task

koppor reviewed May 27, 2025

View reviewed changes

Comment thread jabgui/src/main/java/org/jabref/gui/frame/MainMenu.java Outdated

koppor reviewed May 27, 2025

View reviewed changes

removed wrong focus use, replaced with a direct call to adaptVisibleT…

870c92d

…abs; fixed incorrect spacing

ZodGaz requested a review from koppor June 1, 2025 13:46

koppor requested changes Jun 1, 2025

View reviewed changes

Comment thread jabgui/src/main/java/org/jabref/gui/entryeditor/fileannotationtab/FulltextSearchResultsTab.java Outdated

Addressing review's comment regarding consistency

2ecfc89

koppor approved these changes Jun 2, 2025

View reviewed changes

koppor requested changes Jun 2, 2025

View reviewed changes

Update CHANGELOG.md

45fb34e

koppor reviewed Jun 2, 2025

View reviewed changes

Comment thread CHANGELOG.md Outdated

ZodGaz and others added 2 commits June 2, 2025 16:19

Update CHANGELOG.md

d71688d

Co-authored-by: Oliver Kopp <kopp.dev@gmail.com>

Merge branch 'main' into fix-for-issue-12865

97b4157

koppor approved these changes Jun 2, 2025

View reviewed changes

koppor enabled auto-merge June 2, 2025 14:25

koppor added this pull request to the merge queue Jun 2, 2025

Merged via the queue into JabRef:main with commit 36382da Jun 2, 2025
1 check passed

github-actions Bot mentioned this pull request Jun 2, 2025

"Search results" tab should be shown during fulltext search #12865

Closed

ZodGaz deleted the fix-for-issue-12865 branch June 2, 2025 19:22

subhramit reviewed Jun 3, 2025

View reviewed changes

koppor reviewed Jun 3, 2025

View reviewed changes

ZodGaz commented Jun 6, 2025

View reviewed changes

Uh oh!

Conversation

ZodGaz commented May 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Mandatory checks

Uh oh!

subhramit commented May 27, 2025

Uh oh!

koppor commented May 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

subhramit commented May 27, 2025

Uh oh!

koppor commented Jun 1, 2025

Uh oh!

ZodGaz commented Jun 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

koppor left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

koppor left a comment

Choose a reason for hiding this comment

Uh oh!

ZodGaz commented Jun 2, 2025

Uh oh!

Uh oh!

trag-bot Bot commented Jun 2, 2025

Uh oh!

koppor commented Jun 2, 2025

Uh oh!

Uh oh!

koppor commented Jun 3, 2025 • edited by subhramit Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ZodGaz left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ZodGaz commented May 27, 2025 •

edited

Loading

koppor commented May 27, 2025 •

edited

Loading

ZodGaz commented Jun 1, 2025 •

edited

Loading

koppor commented Jun 3, 2025 •

edited by subhramit

Loading

ZodGaz left a comment •

edited

Loading