Skip to content
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

Fix for issue 11428: auto-completion in search bar now works without tab switching #12060

Closed

Conversation

xiaojie2008
Copy link

@xiaojie2008 xiaojie2008 commented Oct 22, 2024

Purpose

see #11428
The fix ensures that the auto-completion is set up correctly when a database is initially opened, using Platform.runLater to ensure that the completion binder is updated in the next event loop cycle.
closes #11428

Description

I pulled the latest version 5.15 on Windows 11, and the issue still persists.
The root cause lies in the jabref/src/main/java/org/jabref/gui/frame/JabRefFrame.java, specifically in the initBindings() method.
The code responsible for updating the search auto-completion (globalSearchBar.setAutoCompleter(libraryTab.getAutoCompleter());) is placed inside a listener that only triggers when the user switches between tabs (Lines 363-383 in 8cf8959). This leads to the bug where the auto-completion is not activated when opening a database for the first time.
image

Solution

I noticed that there is another listener in the initBindings() method, responsible for handling the selected tab and executing corresponding operations. By placing the globalSearchBar.setAutoCompleter(libraryTab.getAutoCompleter()); inside this listener, the problem can be resolved. Whether the user opens a new database tab or switches to an already open one, this listener gets triggered since both actions select a tab. As a result, this ensures that the search bar auto-completion gets updated appropriately.
image

Outcome

Since the JabRefFrame class is part of the UI, it is not suitable for code-based testing. Therefore, I have attached relevant screenshots below for reference.
image
Figure 1: Type sm and the auto-complete pop-up window will appear.

image
Figure 2: I opened a new database and the autocomplete popup is still active.

image
Figure 3: I switched to the first database and the auto-completion feature still works.

Issue to be clarified

There seems to be a problem with the auto-completion logic. I'm not sure if it's because my testing method is wrong (to be honest, I don't really understand the auto-completion logic), or if this problem is related to the current search being restructured.
This problem mainly occurs when I only enter the first letter of the name, and the completion information given is inaccurate, such as Figure 2.

I think it may not be a problem with my code, because I used the version of the code in main branch (8cf8959) to test, and found that only the first letter of the name is entered, and the same problem as Figure 3 and the following problem will appear.
image

Sometimes when I enter the first two letters of a name, it doesn’t give 100% accurate completion information, such as the screenshot below
image

@LoayGhreeb if possible, could you please take a look into this? I would greatly appreciate your help in identifying the cause. Thank you so much!

Mandatory checks

  • I own the copyright of the code submitted and I licence it under the MIT license
  • Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • 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.

The fix ensures that the auto-completion is set up correctly when a database is initially opened, using `Platform.runLater` to ensure that the completion binder is updated in the next event loop cycle

Issue: JabRef#11428
See: JabRef#11428
@Siedlerchr
Copy link
Member

Hm, I tested your PR and for me under mac it still does only work when I switch tabs
I run JabRef from Idea, it opens with the chocolate bib and autcompletion does not work.

However, in the debugger I see that the setup code is executed, but it still does not work

@xiaojie2008
Copy link
Author

Hm, I tested your PR and for me under mac it still does only work when I switch tabs I run JabRef from Idea, it opens with the chocolate bib and autcompletion does not work.

However, in the debugger I see that the setup code is executed, but it still does not work

Thank you for your feedback. I'm very sorry that I didn't test my code on a Mac before. I will check the problem based on what you said.

@LoayGhreeb
Copy link
Collaborator

I'm very sorry that I didn't test my code on a Mac before. I will check the problem based on what you said.

The same behavior occurs on Windows: if you open Chocolate.bib, the auto-completion won't work until you switch to another tab.

Unfortunately, the changes you made won't solve the problem because the main issue lies with the dummy database context.

Here's the current flow:

  1. Jabref starts and remembers that Chocolate.bib was opened.
  2. A new tab for this library is created (see LibraryTab#createLibraryTab).
  3. This method creates a dummy database context and initializes the tab components with it.
    1. See that the searchAutoCompleter is created using this dummy context, which is empty.
    2. We use the dummy context because loading the real context may take time depending on the library size. To avoid blocking the UI, the tab is shown with a loading progress indicator, and once loading is done, the tab components are recreated with the real context (this might be the main issue --using the dummy context).
  4. After the library tab constructor finishes, the tab is added to the tabContainer (see OpenDatabaseAction#openTheFile).
  5. The listener in JabRefFrame of the selected tab is triggered and sets the search auto-completer, which was created with the dummy context.
  6. Once the background loading task finishes, the tab components are recreated with the real database context (LibraryTab#onDatabaseLoadingSucceed).
  7. While the correct searchAutoCompleter is created, the selected tab listener isn't triggered because the tab itself hasn't changed, so the search bar isn't updated with the correct auto-completer. (This update should get triggered in JabRefFrame to apply the right auto-completer).

I hope this flow helps in identifying the root cause of the issue. In my opinion, the real fix would be to replace the dummy context logic and create the tab only after the loading task finishes.

As a hotfix, I think adding searchAutoCompleter to the StateManager as a property and handling it exactly like the activeSearchQuery property in StateManager, could work.

@LoayGhreeb LoayGhreeb marked this pull request as draft October 23, 2024 10:42
@LoayGhreeb LoayGhreeb added the status: changes required Pull requests that are not yet complete label Oct 23, 2024
@xiaojie2008
Copy link
Author

I'm very sorry that I didn't test my code on a Mac before. I will check the problem based on what you said.

The same behavior occurs on Windows: if you open Chocolate.bib, the auto-completion won't work until you switch to another tab.

Unfortunately, the changes you made won't solve the problem because the main issue lies with the dummy database context.

Here's the current flow:

  1. Jabref starts and remembers that Chocolate.bib was opened.

  2. A new tab for this library is created (see LibraryTab#createLibraryTab).

  3. This method creates a dummy database context and initializes the tab components with it.

    1. See that the searchAutoCompleter is created using this dummy context, which is empty.
    2. We use the dummy context because loading the real context may take time depending on the library size. To avoid blocking the UI, the tab is shown with a loading progress indicator, and once loading is done, the tab components are recreated with the real context (this might be the main issue --using the dummy context).
  4. After the library tab constructor finishes, the tab is added to the tabContainer (see OpenDatabaseAction#openTheFile).

  5. The listener in JabRefFrame of the selected tab is triggered and sets the search auto-completer, which was created with the dummy context.

  6. Once the background loading task finishes, the tab components are recreated with the real database context (LibraryTab#onDatabaseLoadingSucceed).

  7. While the correct searchAutoCompleter is created, the selected tab listener isn't triggered because the tab itself hasn't changed, so the search bar isn't updated with the correct auto-completer. (This update should get triggered in JabRefFrame to apply the right auto-completer).

I hope this flow helps in identifying the root cause of the issue. In my opinion, the real fix would be to replace the dummy context logic and create the tab only after the loading task finishes.

As a hotfix, I think adding searchAutoCompleter to the StateManager as a property and handling it exactly like the activeSearchQuery property in StateManager, could work.

Thank you very much for your detailed feedback. I am very sorry that I did not notice the Chocolate.bib file before. I was using other bib format files. I will refer to the process you gave, check and modify the code.

@koppor
Copy link
Member

koppor commented Dec 9, 2024

Closing this issue due to inactivity 💤

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
search status: changes required Pull requests that are not yet complete ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Auto-Completion in search bar not working until switching tabs
5 participants