Skip to content

Fix for unnecessary horizontal scroll bar in group panel#8756

Merged
Siedlerchr merged 5 commits into
JabRef:mainfrom
LIM0000:fix-for-issue-8467
May 12, 2022
Merged

Fix for unnecessary horizontal scroll bar in group panel#8756
Siedlerchr merged 5 commits into
JabRef:mainfrom
LIM0000:fix-for-issue-8467

Conversation

@LIM0000

@LIM0000 LIM0000 commented May 4, 2022

Copy link
Copy Markdown
Contributor

Previous discussion for group panel horizontal scroll bar: #8467

Unnecessary scroll bar displays in group panel when user increases or decreases the width of panel.

Proposed solution: Hide the group panel scroll bar from user interface

Fixes #8467

.tree-table-view .scroll-bar:horizontal {
    -fx-opacity: 0;
}
fix-8467.mp4

PR Checklist:

  • 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.

@calixtus

calixtus commented May 6, 2022

Copy link
Copy Markdown
Member

I'm not really happy about just making the scrollbar invisible. Instead I'd ask if you could look into if there is a root cause for displaying the scrollbar? Is there a child node too wide to fit? Are there insets that need to be removed?
If we just make the scrollbar invisble, we also hide it when it is maybe wanted or neccessary...

@calixtus

calixtus commented May 6, 2022

Copy link
Copy Markdown
Member

Thanks for your interest in this issue and your proposal to fix it.

@LIM0000

LIM0000 commented May 6, 2022

Copy link
Copy Markdown
Contributor Author

Thank you for your valuable comment @calixtus.
I agree that hiding the scroll bar is not the best way to fix this issue. There are some discussions about horizontal scrollbar appears in TableView years ago. One of the comment had named a workaround that could fix horizontal scroll bar from appearing and flickering.

1

Solution

I have followed the workaround and glad that it actually fixes our issue by:

  1. mainColumn that needs to be setResizeable(true) to truncate long group names
  2. numberColumn and expansionNodeColumn need to be setResizable(false) as they only need to be in fix size to display their contents
  3. set setMaxWidth, setMinWidth and setPrefWidth for numberColumn and expansionNodeColumn columns

There is a tricky part for the value of width of numberColumn and expansionNodeColumn. Both these columns need to be 40d and 20d respectively. This is because the contents in both the columns have padding in them under GroupTree.css. If a random width value is set to the column, the column will either be too wide or too narrow which causes the display of horizontal scrollbar.

mainColumn.prefWidthProperty().bind(groupTree.widthProperty().subtract(60d).subtract(15)); needs to be added to prevent horizontal scrollbar in table from flickering when table shrinks.

Flickering example when table shrinks:

bug_1.1.mp4

@calixtus

calixtus commented May 6, 2022

Copy link
Copy Markdown
Member

Thanks for looking into this. I believe the solution you found now is the better one.

We have to think about relativity of column width depending on screen resolution and scaling in the future. But I think this should not be fixed in this patch, since scaling fails in other places too. This one looks already good to me and is good improvement.

@calixtus

calixtus commented May 6, 2022

Copy link
Copy Markdown
Member

Is this ready for review now?

@LIM0000

LIM0000 commented May 6, 2022

Copy link
Copy Markdown
Contributor Author

Yes, it is ready for review.

@LIM0000 LIM0000 marked this pull request as ready for review May 6, 2022 14:37
@calixtus calixtus added status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers component: ui labels May 6, 2022
@Siedlerchr Siedlerchr merged commit 6b9b84e into JabRef:main May 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component: ui status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Scrollbar shows up needlessly in group panel, while decreasing the width.

3 participants