Skip to content

Conversation

@070jhz
Copy link
Contributor

@070jhz 070jhz commented Jun 4, 2025

Closes #12921

the type-check predicate can behave inconsistently during the interaction, likely a timing/synchronization issue caused by how the UI updates the tab list during drag-and-drop while the filtered list is reacting to changes.
directly mapping over the original tabs list with a lambda that accoutns for possible non-LibraryTab entries solves the issue. this results in openDatabaseList possibly holding null values but since it is a private member not accessed elsewhere and we bind a filtered version to StateManager::openDatabases, it should not cause any further bugs.

a more robust solution could be implemented with listeners, but it might be overkill ?

Steps to test

  1. Disable the setting to "Hide Tab bar when single library is present"
  2. Create a library (Or create 2 if you skipped step 1)
  3. Click and drag the Welcome tab and move it beside the newly created library
  4. Exception no longer occurs

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.

@070jhz 070jhz marked this pull request as ready for review June 4, 2025 08:42
@koppor
Copy link
Member

koppor commented Jun 4, 2025

This is a JavaFX issue. @Siedlerchr how to report?

Update: Moved to #12921 (comment) as it concerns more the original issue than this work around

Copy link
Member

@koppor koppor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code nees to be adapted, since the FilteredList is not right at all due to a JavaFX bug.

return null;
}
});
EasyBind.bindContent(stateManager.getOpenDatabases(), new FilteredList<>(openDatabaseList, Objects::nonNull));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think, the list will be wrong on tab change. Maybe some custom code needs to be placed here? IDK.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a specific case you had in mind ? i manually tested tab changes (opening, closing, rearranging) and openDatabaseList seems to accurately reflect the tab state.

@Siedlerchr
Copy link
Member

Siedlerchr commented Jun 4, 2025

you can file a bug report https://bugreport.java.com/bugreport/
It will take a while until it is processed
For company you can use JabRef e.V.

@koppor
Copy link
Member

koppor commented Jun 4, 2025

you can file a bug report bugreport.java.com/bugreport

Submitted a JavaFX error report - internal review ID : 9078592

@070jhz
Copy link
Contributor Author

070jhz commented Jun 5, 2025

Code nees to be adapted, since the FilteredList is not right at all due to a JavaFX bug.

exception no longer occurs with no FilteredList usage, with the lists accurately reflecting the state of the tabs. i added some listener code to filter out the nulls from the mapping and keep it in sync with stateManager::openDatabaseList

Copy link
Member

@koppor koppor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some code simplificatoin comments.

calixtus and others added 2 commits June 7, 2025 19:40
Co-authored-by: Oliver Kopp <[email protected]>
@calixtus calixtus enabled auto-merge June 7, 2025 17:43
@trag-bot
Copy link

trag-bot bot commented Jun 7, 2025

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

@calixtus calixtus added this pull request to the merge queue Jun 7, 2025
Merged via the queue into JabRef:main with commit bae13e1 Jun 7, 2025
1 check passed
@koppor koppor mentioned this pull request Jul 5, 2025
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Trying to rearrange the welcome tab causes an exception

5 participants