fix visual snap of side pane on startup#15553
Open
pgrigolli wants to merge 9 commits intoJabRef:mainfrom
Open
Conversation
github-actions
bot
added
component: ui
status: changes-required
2 tasks
Collaborator
|
Note that your PR will not be reviewed/accepted until you have gone through the mandatory checks in the description and marked each of them them exactly in the format of |
Contributor
Review Summary by QodoFix visual snap of side pane on startup
WalkthroughsDescription• Hide side pane during divider position initialization to prevent visual snap • Apply divider position after stage is fully shown using Platform.runLater • Add listener to update divider position when main stage becomes visible • Update changelog to document the side pane snap fix Diagramflowchart LR
A["Side pane added to split"] -- "Hide split pane" --> B["Platform.runLater"]
B -- "Update divider position" --> C["Show split pane"]
D["Main stage showing"] -- "Listener triggered" --> E["Update divider position"]
File Changes1. jabgui/src/main/java/org/jabref/gui/frame/JabRefFrame.java
|
Contributor
Code Review by Qodo
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Comment on lines
+414
to
+418
| mainStage.showingProperty().addListener((obs, wasShowing, isShowing) -> { | ||
| if (isShowing) { | ||
| updateHorizontalDividerPosition(); | ||
| } | ||
| }); |
Contributor
There was a problem hiding this comment.
2. Leaked divider subscriptions 🐞 Bug ☼ Reliability
initBindings() adds a mainStage.showingProperty listener that calls updateHorizontalDividerPosition(), but JabRefGUI.onShowing() already calls it; because updateHorizontalDividerPosition() overwrites horizontalDividerSubscription without unsubscribing the previous subscription, repeated window show cycles will leak listeners and duplicate preference updates.
Agent Prompt
### Issue description
`updateHorizontalDividerPosition()` installs an EasyBind listener each time it runs, but it does not unsubscribe any prior listener. This PR adds an additional call site (`mainStage.showingProperty` listener), while `JabRefGUI.onShowing()` already calls the method, so subscriptions can stack up across show/hide cycles.
### Issue Context
- `JabRefGUI.onShowing()` already calls `mainFrame.updateHorizontalDividerPosition()`.
- `updateHorizontalDividerPosition()` assigns `horizontalDividerSubscription = ...listenToValues(...)` without cleanup.
### Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/frame/JabRefFrame.java[311-320]
- jabgui/src/main/java/org/jabref/gui/frame/JabRefFrame.java[411-418]
- jabgui/src/main/java/org/jabref/gui/JabRefGUI.java[357-361]
### Suggested fix approach
- Prefer a single invocation path (remove the newly-added `showingProperty` listener, since `JabRefGUI.onShowing()` already handles it), **or**
- Make `updateHorizontalDividerPosition()` idempotent by unsubscribing an existing `horizontalDividerSubscription` before creating a new one (and consider doing the same for vertical for consistency).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Author
There was a problem hiding this comment.
Hey, turns out it was just dead code that I forgot to remove from one try to another. Removed on the last commit!
calixtus
reviewed
Apr 15, 2026
Author
|
Idk why these tests aren't passing. What should I do? |
Member
|
We are looking into it, please be patient. Maybe something with our CI pipeline. |
Member
|
@pgrigolli should be fine now. |
Author
Nice, thanks! What was wrong? |
Member
We had a faulty Windows-specific test which was fixed in one of the recent PRs. |
Contributor
Review Summary by QodoFix side pane visual snap on startup
WalkthroughsDescription• Hide side pane during divider position initialization to prevent visual snap • Use Platform.runLater() to apply divider position after layout is ready • Show side pane only after correct position has been set • Updated CHANGELOG.md with fix description Diagramflowchart LR
A["Side pane added to split"] --> B["Hide horizontal split pane"]
B --> C["Schedule divider position update"]
C --> D["Apply correct divider position"]
D --> E["Show horizontal split pane"]
E --> F["Side pane displays at correct width"]
File Changes1. jabgui/src/main/java/org/jabref/gui/frame/JabRefFrame.java
|
Contributor
Code Review by Qodo
1. CHANGELOG.md adds blank line
|
Contributor
|
Do not mark a PR as ready-for-review if changes are required. |
Member
|
Contributor
Review Summary by QodoFix side pane snap on startup by deferring position update
WalkthroughsDescription• Fixes visual snap of side pane on startup by hiding it during initialization • Applies divider position after layout is ready using Platform.runLater() • Prevents incorrect positioning before showing the pane to user • Updates changelog to document the fix Diagramflowchart LR
A["Side pane added to split"] -- "Hide split pane" --> B["Defer position update"]
B -- "Apply divider position" --> C["Show split pane"]
C -- "User sees correct position" --> D["No visual snap"]
File Changes1. jabgui/src/main/java/org/jabref/gui/frame/JabRefFrame.java
|
Contributor
Code Review by Qodo
1. CHANGELOG.md adds blank line
|
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
4 participants
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.
Related issues and pull requests
Closes #15394
PR Description
When JabRef starts, the side pane was briefly rendered in the wrong position
before snapping to the correct width previously set by the user. The root cause
was that updateHorizontalDividerPosition() was called before the layout was ready,
causing the divider position to not be applied correctly. This was fixed by hiding
the horizontal split pane while the divider position is being applied, and showing
it again only after the correct position has been set, preventing the snap from
being visible to the user.
Steps to test
Checklist
CHANGELOG.mdin a way that can be understood by the average user (if change is visible to the user)Video of the changes working (on Windows 11, developer version)
https://github.com/user-attachments/assets/782147b0-ded8-4b13-ad5c-527f67f63211
I couldn't test it on MacOS, but I tested on Linux (Debian 12) and it wasn't happening there!
Since the issue was first addressed on Windows, it should work now!
Library Entry
