Click on entry at "Check integrity" focus bug fixed #12022

mulla028 · 2024-10-19T01:47:55Z

Closes Click on entry at "Check integrity" should focus the entry and field #11997

Description

This PR aims on a bug fix, when user was clicking on entry at Check Integrity, entry was focused, but not a field in entry editor.

Changes

In src/main/java/org/jabref/gui/entryeditor/EntryEditor.java two lines of code were added to setFocusToField()

inside for-loop forcing to open every tab to render all fields, before selection is made tabbed.getSelectionModel().select(tab);
getScene().getWindow().requestFocus();
Added condition that checks if Field wasn't selected , so it continues to check if ALIAS of that field exists, if ALIAS exists, if so, it replaces Field to it's ALIAS, otherwise nothing happens.

if (!fieldFound) {
    Field aliasField = EntryConverter.FIELD_ALIASES.get(field);
       if (aliasField != null) {
           setFocusToField(aliasField);
        }
 }

This line explicitly brings focus back to the main window containing the Entry Editor. It fixes a field focus bug.

Video of changed UI

Screen.Recording.2024-11-29.at.10.19.05.AM.mov

As you can see on screenshot bug is fixed!

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.

…); to make sure that focus goes to the field even when dialog window is opened

mulla028 · 2024-10-19T01:49:41Z

@koppor
I really enjoyed contributing on your project! Thank you for this opportunity!
If you have any questions or suggestions regarding this PR, just let me know 🫡

koppor

Please re-iterate on the issue description: #11997

Here, especially for you, another description.

Start JabRef
Open chocolate.bib - you find it at https://github.com/JabRef/jabref/blob/main/src/test/resources/testbib/Chocolate.bib
Seelct entry "Cocoa and Cardiovascular Health"
Select tab "General" in Entry editor
Select field DOI
Go to Menu "Quality"
Click on "Check integrity"
Double click on the first entry "Corti_2009"
See that nothing happens

In Step 9, the tab "Requried fiels" and the field "Citatoinkey" should be focused.

mulla028 · 2024-10-19T16:53:38Z

Please re-iterate on the issue description: #11997

Here, especially for you, another description.

Start JabRef

Open chocolate.bib - you find it at https://github.com/JabRef/jabref/blob/main/src/test/resources/testbib/Chocolate.bib

Seelct entry "Cocoa and Cardiovascular Health"

Select tab "General" in Entry editor

Select field DOI

Go to Menu "Quality"

Click on "Check integrity"

Double click on the first entry "Corti_2009"

See that nothing happens

In Step 9, the tab "Requried fiels" and the field "Citatoinkey" should be focused.

Thank you for your feedback! Will do my best!

mulla028 · 2024-10-23T19:49:43Z

@koppor
You mean that after I opened General tab, and then in Check Integrity once I double click on entry it should focus on both fields:

Citation Key
Field?

Siedlerchr · 2024-10-23T19:53:14Z

It should focus on the field that is reported in the integrity check

mulla028 · 2024-10-23T19:56:52Z

It should focus on the field that is reported in the integrity check

Thank you for your response! I'm pretty new to this project. Would it be possible to share a little detailed description? If not, I will do my best to figure all out myself.

mulla028 · 2024-10-23T20:04:06Z

@Siedlerchr
What I don't understand is that current PR supports this feature according on as much I understand.
Following an example by @koppor, he claims that If I click on the first entry Corti2009 nothing happens. However, my end shows me that current field CitationKey focused.
Sharing with you screenshot:

Please let me know if I misunderstand you! Thank you!

Siedlerchr · 2024-10-23T20:21:27Z

That is indeed correct, however, in my tests jumping to another tab only works sometimes:

Open chocolate bib
check integrity
click on an entry with DOI => The required information tab stays open

However, if I now select the general Tab of an entry, I am able to jump to it/again to it
Same applies to pages field

Please check why the tab needs to have been selected first

mulla028 · 2024-10-23T20:24:44Z

That is indeed correct, however, in my tests jumping to another tab only works sometimes:

Open chocolate bib

check integrity

click on an entry with DOI => The required information tab stays open

However, if I now select the general Tab of an entry, I am able to jump to it/again to it Same applies to pages field

Please check why the tab needs to have been selected first

Oh now I understand where you are getting at! Thank you!

mulla028 · 2024-11-26T18:45:20Z

@Siedlerchr, @koppor Hello!

Is this issue still relevant? If so, I would love to continue working on it 😄
I had to figure out some staff at college, and wasn't able to finish.
Previous time, when I'd been trying to fix this bug, I couldn't understand if I'd been digging in the right direction. I really want to fix this bug, I took it personal 😆

File that I suspect contains a bug: src/main/java/org/jabref/gui/entryeditor/EntryEditor.java

Siedlerchr · 2024-11-26T20:24:23Z

Sure, that's great to here! You can continue with your PR for this issue

mulla028 · 2024-11-27T21:19:59Z

@Siedlerchr Hello again!
Could you do me a favour, I'm working on this project as part of my Open-source college course. I have already showed that I worked on this issue. Would it be possible to create a new issue related to this bug, assign me, and expand it a little bit, so it could count towards my college work. I would really appreciate it🙏
I'm asking for the reason, that this bug was already expanded a little bit.

koppor · 2024-11-28T12:16:27Z

@mulla028 We need to be fair to all contributors. We try to treat them equally. First, resolve issue #11997 completely. At least one case of it does not work. Thus, maybe the code is buggy and should be changed. If there are follow-up issues, we can create other issues. - But for a single line of code added and not fixing everything, there should be more time spend for a solution. I fully understand that UI programming is hard, because it is very much trial and error. - The hint is to spend time to a minimal working example: Creating a new application from scratch and trying to replicate the issue there - and fixing it there. Then porting back the fix. https://github.com/Siedlerchr/javafxreproducer can be a start.

mulla028 · 2024-11-28T18:50:14Z

@mulla028 We need to be fair to all contributors. We try to treat them equally. First, resolve issue #11997 completely. At least one case of it does not work. Thus, maybe the code is buggy and should be changed. If there are follow-up issues, we can create other issues. - But for a single line of code added and not fixing everything, there should be more time spend for a solution. I fully understand that UI programming is hard, because it is very much trial and error. - The hint is to spend time to a minimal working example: Creating a new application from scratch and trying to replicate the issue there - and fixing it there. Then porting back the fix. https://github.com/Siedlerchr/javafxreproducer can be a start.

Alright, I just tried. If you never try - you never know 😆

mulla028 · 2024-11-28T18:51:28Z

@Siedlerchr @koppor I noticed that #11997 fixed in one of the PRs.

Once updated my branch I witnessed that everything works as you expected:

koppor · 2024-11-28T21:01:58Z

Does not work for aliassed fields:

mulla028 · 2024-11-28T21:03:42Z

Does not work for aliassed fields:

Thank you for your response, will take a look at it, and try to fix it.

koppor · 2024-11-28T21:04:36Z

That is indeed correct, however, in my tests jumping to another tab only works sometimes:

@Siedlerchr Can you refine?

1. Open chocolate bib
2. check integrity
3. click on an entry with DOI => The required information tab stays open

I go to Corti_2009 and select "Other fields" and select "Publisher"

Then, I do "Check integrity"

Then, I click Corti_2009. Works.

Is this a macOS / Windows thing? Timing maybe?

koppor · 2024-11-28T21:11:26Z

Note that double click does not work, it is a single click!

koppor · 2024-11-28T21:11:54Z

Note that double click does not work, it is a single click!

I updated the issue title. - Please update the PR description and add a CHANGELOG.md entry, too.

koppor · 2024-11-28T21:12:54Z

Proposal to proceed:

@mulla028 Adds a CHANGELOG.md entry
@Siedlerchr Checks the issue on macOS again
@mulla028 Works on field aliasses in parallel.

mulla028 · 2024-11-28T21:13:41Z

Note that double click does not work, it is a single click!

Do I have to disable single click, and set focus on double click only, right?

OS: macOS Sequoia 15.1.1

UPD: According to the new issue title. I guess, I have to leave single click option

Siedlerchr · 2024-11-28T21:34:31Z

Might need to verify again but it could be that it got fixed by chance through another commit Amir Mullagaliev ***@***.***> schrieb am Do., 28. Nov. 2024, 22:14:

…

Note that double click does not work, it is a single click! Do I have to disable single click, and set focus on only double click, right? OS: macOS Sequoia 15.1.1 — Reply to this email directly, view it on GitHub <#12022 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACOFZC73WZ4XUWVSDEGFA32C6BRXAVCNFSM6AAAAABQG7LQUWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKMBWG43DEMBXHA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

mulla028 · 2024-11-28T21:43:51Z

Might need to verify again but it could be that it got fixed by chance through another commit Amir Mullagaliev @.> schrieb am Do., 28. Nov. 2024, 22:14:
…
Note that double click does not work, it is a single click! Do I have to disable single click, and set focus on only double click, right? OS: macOS Sequoia 15.1.1 — Reply to this email directly, view it on GitHub <#12022 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACOFZC73WZ4XUWVSDEGFA32C6BRXAVCNFSM6AAAAABQG7LQUWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKMBWG43DEMBXHA . You are receiving this because you were mentioned.Message ID: @.>

Probably it got fixed in one of these PRs: #12165 , #12034. But I'm not sure...

mulla028 · 2024-11-29T00:37:25Z

I found another problem that prevents me from focusing PAGES field, it happens because the tabs in src/main/java/org/jabref/gui/entryeditor/FieldsEditorTab.java have lazy initialization. Therefore, it means that the fields inside of the tabs aren't initialized until they opened.

e.g., if I try to select entry with PAGES field before I open Optional Fields tab, it won't focus there.
However, once I open that tab and switch to other, and select it again, it will get focused!

Everything that I described is demonstrated in video below!

Screen.Recording.2024-11-28.at.7.50.05.PM.mov

mulla028 · 2024-11-29T00:45:39Z

The reason why Journaltitle isn't focusing, that the field name is JOURNALTITLE, but logic searches for the field named JOURNAL, which doesn't exist in Required Fields tab.

It starts inside of src/main/java/org/jabref/gui/integrity/IntegrityCheckDialog.java#onSelectionChanged:

change.getAddedSubList().stream().findFirst().ifPresent(message ->
                    libraryTab.editEntryAndFocusField(message.entry(), message.field()));

message.field() is getting passed as field argument to the editEntryAndFocusField(), so in case of JOURNAL it passes field not as JOURNALTITLE, but as JOURNAL + message: "journal not found in abbreviation list"

koppor · 2024-11-29T08:12:14Z

I found another problem that prevents me from focusing PAGES field, it happens because the tabs in src/main/java/org/jabref/gui/entryeditor/FieldsEditorTab.java have lazy initialization. Therefore, it means that the fields inside of the tabs aren't initialized until they opened.

Then try to force opening the tab. Maybe it is as simple as wrapping in Plattform.runlater.

koppor · 2024-11-29T08:16:11Z

The reason why Journaltitle isn't focusing, that the field name is JOURNALTITLE, but logic searches for the field named JOURNAL, which doesn't exist in Required Fields tab.

Yeah, first step for the brain teaser.

Second step: Check org.jabref.model.entry.EntryConverter#FIELD_ALIASES.
Third step: Make use of that map when jumping to a field. Hint: If field is not found, to a get on the Map. If this is non-null, try to find that field. If not found, you can do nothing.

Siedlerchr · 2024-11-29T09:19:05Z

There is a method getfieldOrAlias...

jabref/src/main/java/org/jabref/model/entry/BibEntry.java

Line 588 in 04c0df9

public Optional<String> getFieldOrAlias(Field field) {

koppor · 2024-11-29T09:54:55Z

There is a method getfieldOrAlias...

jabref/src/main/java/org/jabref/model/entry/BibEntry.java

Line 588 in 04c0df9

public Optional<String> getFieldOrAlias(Field field) {

This is the other way round!

There: Give me the content of the field
Here: Give me the field belonging to a content

mulla028 · 2024-11-29T12:42:58Z

@koppor @Siedlerchr Thank you for your feedback! Will try to make everything work!

mulla028 · 2024-11-29T12:59:53Z

Alright, I fixed focus for journaltitle field, take a look at it. Trying to fix pages field

github-actions

Your code currently does not meet JabRef's code guidelines.
We use Checkstyle to identify issues.
Please carefully follow the setup guide for the codestyle.
Afterwards, please run checkstyle locally and fix the issues.

…ecting one

mulla028 · 2024-11-29T15:20:48Z

@koppor @Siedlerchr
Issue fixed:

Screen.Recording.2024-11-29.at.10.19.05.AM.mov

Siedlerchr · 2024-11-29T16:18:03Z

Please test with multiple open libraries as well

Siedlerchr · 2024-11-29T16:22:05Z

src/main/java/org/jabref/gui/entryeditor/EntryEditor.java

@@ -468,6 +468,7 @@ public void setFocusToField(Field field) {
        Field actualField = field;
        boolean fieldFound = false;
            for (Tab tab : tabbed.getTabs()) {
+                tabbed.getSelectionModel().select(tab);


This looks not right without a condition. What happens with multiple open libraries?
Actually there is a condition below. Why is that not triggered?

It works well with multiple opened libraries, just tested.
What condition specifically are you talking about , the very first one inside of for-loop?

Answering the question, why that is not triggered, for the reason, that fieldEditorTab has a lazy initialization, so it doesn't render the fields unless tab opened at least once, that's why I added line 471. I updated description, at the top of this PR, there I described all the changes.

mulla028 · 2024-11-29T16:25:53Z

Please test with multiple open libraries as well

Just have tested, works with other libraries that include MONTH, YEAR, LOCATION, and FILE fields

mulla028 · 2024-11-29T16:36:52Z

Do you have any other concerns, imo - the only change that I should do is to remove line 468

mulla028 · 2024-11-29T16:40:56Z

If you have concerns regarding iteration through the tabbed tabs before the condition, it doesn't include every library. It only iterates through the library that is currently in use.

Siedlerchr · 2024-11-29T16:58:21Z

Tested it, works fine, also with custom field

koppor

Code is OK enough.

I left comments for a follow-up. But we merge now to free energy for finishing other PRs.

koppor · 2024-11-29T17:06:18Z

src/main/java/org/jabref/gui/entryeditor/EntryEditor.java

@@ -463,11 +465,26 @@ private void fetchAndMerge(EntryBasedFetcher fetcher) {

    public void setFocusToField(Field field) {
        UiTaskExecutor.runInJavaFXThread(() -> {
+        Field actualField = field;


This is not necessary as you use recursion in line 484.

koppor · 2024-11-29T17:07:01Z

src/main/java/org/jabref/gui/entryeditor/EntryEditor.java

+                    // This line explicitly brings focus back to the main window containing the Entry Editor.
+                    getScene().getWindow().requestFocus();


I would have wrapped that into Platform.runlater, too (and before the runLater above)

koppor · 2024-11-29T17:07:13Z

src/main/java/org/jabref/gui/entryeditor/EntryEditor.java

+                    // This line explicitly brings focus back to the main window containing the Entry Editor.
+                    getScene().getWindow().requestFocus();
+                    fieldFound = true;
+                    break;


Could be a return.

koppor · 2024-11-29T17:07:28Z

src/main/java/org/jabref/gui/entryeditor/EntryEditor.java

+                    break;
+                }
+            }
+            if (!fieldFound) {


If guard can be removed if you use return in 481.

mulla028 · 2024-11-29T17:18:23Z

@koppor @Siedlerchr
I would love to continue working on this project, could you please suggest me issues that I may take?

Siedlerchr · 2024-11-29T17:33:32Z

@mulla028 That's great to hear and thanks for coming back and finishing this.
We have enough issues for everyone! I would sugget you check out some of the good first issues https://github.com/orgs/JabRef/projects/5 (fee to take)
or you can look at other issues:
https://github.com/orgs/JabRef/projects/7/views/2

mulla028 · 2024-11-29T20:20:32Z

Thank you so much!
I really enjoyed working on this project!
I really appreciate your guidelines and patience 😄

Two lines added: comment and the getScene().getWindow().requestFocus(…

173666b

…); to make sure that focus goes to the field even when dialog window is opened

koppor marked this pull request as draft October 19, 2024 09:22

koppor requested changes Oct 19, 2024

View reviewed changes

koppor added the status: changes required Pull requests that are not yet complete label Oct 19, 2024

Merge branch 'main' into issue-11997

e195313

mulla028 changed the title ~~Double click on entry at "Check integrity" focus bug fixed~~ Click on entry at "Check integrity" focus bug fixed Nov 28, 2024

Focus bug added to the Changelog's Fix field

10f5f10

setFocusToField() fixed focus for JournalTitle Field

f7727dc

github-actions bot reviewed Nov 29, 2024

View reviewed changes

mulla028 added 2 commits November 29, 2024 08:24

Checkstyle Applied

d8c4aec

Added line in setFocusToField() to force opening every tab before sel…

9e4390c

…ecting one

mulla028 marked this pull request as ready for review November 29, 2024 15:20

Siedlerchr reviewed Nov 29, 2024

View reviewed changes

Siedlerchr approved these changes Nov 29, 2024

View reviewed changes

Siedlerchr added status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers and removed status: changes required Pull requests that are not yet complete labels Nov 29, 2024

koppor approved these changes Nov 29, 2024

View reviewed changes

koppor added this pull request to the merge queue Nov 29, 2024

Merged via the queue into JabRef:main with commit 37593bf Nov 29, 2024
24 checks passed

		// This line explicitly brings focus back to the main window containing the Entry Editor.
		getScene().getWindow().requestFocus();

Click on entry at "Check integrity" focus bug fixed #12022

Click on entry at "Check integrity" focus bug fixed #12022

Conversation

mulla028 commented Oct 19, 2024 • edited Loading

Description

Changes

Video of changed UI

Mandatory checks

mulla028 commented Oct 19, 2024

koppor left a comment

Choose a reason for hiding this comment

mulla028 commented Oct 19, 2024

mulla028 commented Oct 23, 2024

Siedlerchr commented Oct 23, 2024

mulla028 commented Oct 23, 2024

mulla028 commented Oct 23, 2024 • edited Loading

Siedlerchr commented Oct 23, 2024

mulla028 commented Oct 23, 2024

mulla028 commented Nov 26, 2024 • edited Loading

Siedlerchr commented Nov 26, 2024

mulla028 commented Nov 27, 2024 • edited Loading

koppor commented Nov 28, 2024

mulla028 commented Nov 28, 2024

mulla028 commented Nov 28, 2024 • edited Loading

koppor commented Nov 28, 2024

mulla028 commented Nov 28, 2024

koppor commented Nov 28, 2024

koppor commented Nov 28, 2024

koppor commented Nov 28, 2024

koppor commented Nov 28, 2024

mulla028 commented Nov 28, 2024 • edited Loading

Siedlerchr commented Nov 28, 2024 via email

mulla028 commented Nov 28, 2024 • edited Loading

mulla028 commented Nov 29, 2024 • edited Loading

mulla028 commented Nov 29, 2024

koppor commented Nov 29, 2024

koppor commented Nov 29, 2024

Siedlerchr commented Nov 29, 2024

koppor commented Nov 29, 2024

mulla028 commented Nov 29, 2024

mulla028 commented Nov 29, 2024

github-actions bot left a comment

Choose a reason for hiding this comment

mulla028 commented Nov 29, 2024

Siedlerchr commented Nov 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mulla028 commented Nov 29, 2024

mulla028 commented Nov 29, 2024

mulla028 commented Nov 29, 2024

Siedlerchr commented Nov 29, 2024

koppor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mulla028 commented Nov 29, 2024

Siedlerchr commented Nov 29, 2024

mulla028 commented Nov 29, 2024 • edited Loading

mulla028 commented Oct 19, 2024 •

edited

Loading

mulla028 commented Oct 23, 2024 •

edited

Loading

mulla028 commented Nov 26, 2024 •

edited

Loading

mulla028 commented Nov 27, 2024 •

edited

Loading

mulla028 commented Nov 28, 2024 •

edited

Loading

mulla028 commented Nov 28, 2024 •

edited

Loading

mulla028 commented Nov 28, 2024 •

edited

Loading

mulla028 commented Nov 29, 2024 •

edited

Loading

mulla028 commented Nov 29, 2024 •

edited

Loading