Skip to content

Fix #13402: Make sidebar width proportional to window size and persist across sessions #13405

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

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

SahilBijlani
Copy link

Summary

This PR fixes the issue where the sidebar (SidePane) did not retain its width across application restarts. Previously, even if users resized the sidebar manually, the width would reset on every launch. This change makes the sidebar width proportional to the window size and persists the divider position using preferences.

Closes #13402

Changes Made

  • Modified JabRefFrame to store the sidebar divider position as a proportional value (0.0–1.0) instead of absolute pixels.
  • Persisted the divider position in GuiPreferences when the sidebar is visible.
  • Restored the divider position when the sidebar is shown again and when JabRef is restarted.
  • Handled edge cases (like window width being 0, NaN, or sidebar being hidden) to avoid invalid layout behavior.

Steps to test

  1. Launch JabRef.
  2. Open the Groups pane or any other sidebar component.
  3. Resize the sidebar to your preferred width.
  4. Close JabRef and reopen it.
  5. The sidebar should open at the same proportional width as before.
  6. Try resizing the entire window — sidebar width should still stay proportional.

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 (not applicable; this is a GUI layout change that is hard to unit test)
  • 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: No update needed; behavior is expected and unchanged in functionality.

@SahilBijlani
Copy link
Author

@koppor @Siedlerchr Could you please review this PR? It fixes #13402 by persisting the sidebar width proportionally and across sessions. All checks have passed. Thank you!

@SahilBijlani
Copy link
Author

Hi @koppor @Siedlerchr 👋

All checks have passed ✅ and I’ve addressed all feedback from trag-bot.
This PR fixes #13402 by persisting the sidebar width proportionally across sessions.

Could you please review and approve it when you get a chance?
Thank you so much! 🙏


horizontalSplit.setDividerPositions(savedProportion);

// Subscribe to save user-adjusted sidebar position
Copy link
Author

Choose a reason for hiding this comment

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

done!

CHANGELOG.md Outdated
@@ -9,6 +9,8 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv

## [Unreleased]

- Sidebar width now persists across sessions and resizes proportionally to window width [#13402](https://github.com/JabRef/jabref/issues/13402)
Copy link
Member

Choose a reason for hiding this comment

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

move to the changed section

Copy link

trag-bot bot commented Jun 28, 2025

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

@koppor
Copy link
Member

koppor commented Jun 29, 2025

Contributor uses AI-generated responses for communication - #13402 (comment)

If there is anything strange in this PR, it is OK to close it.

@SahilBijlani
Copy link
Author

SahilBijlani commented Jun 29, 2025 via email

@koppor
Copy link
Member

koppor commented Jun 29, 2025

Appreciate your reply!! But i want to clarify that all the decisions are
made by me (using an ai tool isn’t bad as all other developers do!

Yes. The comments that were posted using your account made the impression that an AI spoke for you.

See https://github.com/JabRef/jabref/blob/main/CONTRIBUTING.md#notes-on-ai-usage

My goal was to improve JabRef..just that

Nice.

My goal is to have a nice free time project with people makeing the impression they are making proper use of our time.

Its Sunday here and I am on the mobile phone only. Thus, no review the next hours.

@SahilBijlani
Copy link
Author

SahilBijlani commented Jun 30, 2025 via email

@subhramit
Copy link
Member

subhramit commented Jun 30, 2025

Hello! i will glad knowing what's the status of the issue that i was assigned? Thank you

Hi, could you take the effort of opening GitHub and checking yourself? You have both the issue link and the PR link, right?
Some background: All the maintainers of the project have day jobs here, and they look after this project out of passion in their free time. We appreciate if our time is not spent in unnecessary things.
Your contribution is truly appreciated, but please enlighten yourself with the bare minimum courtesy and effort that is expected from you with regards to communication and how to check things out, and maintain a thread of conversation on the website.
Maybe check out CONTRIBUTING.md or at least open and read how conversations happen in a few existing PRs.

But i want to clarify that all the decisions are made by me (using an ai tool isn’t bad as all other developers do! My goal was to improve JabRef..just that

Please understand that some of the maintainers here are very senior developers with many years of experience. They all understand and use AI where it is useful, but they are able to do that because they have the knowledge to identify where it is not useful. To get there, it takes a long road of thinking independently and evaluating decisions. The issue arises when one is not able to evaluate the outcome of AI, and also doesn't understand how dehumanizing it is to communicate using that. In your previous PR, you understood nothing that was written in the mandatory checks section and ticked off everything, yet you claim that decisions were made by you.

Usually we close PRs for the reasons mentioned, but the point behind pointing all this out is so that you have another chance and we can have more fruitful exchanges in future, provided you are willing to understand things.

Thanks.

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.

Too many listeners generated. There must be another solution where listeners are instantiated on startup, not on each change of something.

@@ -83,8 +83,9 @@ public class JabRefFrame extends BorderPane implements LibraryTabContainer, UiMe
private enum PanelMode { MAIN_TABLE, MAIN_TABLE_AND_ENTRY_EDITOR }

public static final String FRAME_TITLE = "JabRef";

Copy link
Member

Choose a reason for hiding this comment

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

Why more spaces?

if (Double.isNaN(savedProportion) || savedProportion <= 0 || savedProportion >= 1) {
savedProportion = DEFAULT_SIDEBAR_DIVIDER_POSITION;
}
horizontalSplit.setDividerPositions(savedProportion);
Copy link
Member

Choose a reason for hiding this comment

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

Indent is off

}
}
Copy link
Member

Choose a reason for hiding this comment

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

?

strange indent

@@ -447,6 +463,14 @@ private void initBindings() {
EasyBind.subscribe(preferences.getWorkspacePreferences().hideTabBarProperty(), _ -> updateTabBarVisible());
}

private void initSidebarResizeListener() {
mainStage.widthProperty().addListener((obs, oldVal, newVal) -> {
Copy link
Member

Choose a reason for hiding this comment

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

Why no _ used here?

private void initSidebarResizeListener() {
mainStage.widthProperty().addListener((obs, oldVal, newVal) -> {
if (!sidePane.getChildren().isEmpty()) {
updateHorizontalDividerPosition();
Copy link
Member

Choose a reason for hiding this comment

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

Here, inside, always a new listener is created. This will let the memory grow and grow and grow and grow and grow.

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.

adapt sidebar (groups, etc) proportionally relative to window size
4 participants