Skip to content

Add hover button to group rows for adding groups#15502

Merged
Siedlerchr merged 10 commits intoJabRef:mainfrom
faneeshh:fix-issue-12289
Apr 9, 2026
Merged

Add hover button to group rows for adding groups#15502
Siedlerchr merged 10 commits intoJabRef:mainfrom
faneeshh:fix-issue-12289

Conversation

@faneeshh
Copy link
Copy Markdown
Contributor

@faneeshh faneeshh commented Apr 6, 2026

Related issues and pull requests

Closes #12289

PR Description

This PR applies the suggested proposal by @koppor showing the "+" button for adding groups on hovering over any group row. The "All Entries" row shows "New Group" and rest of the rows show "Add Subgroup" as expected.

Steps to test

  1. Hover Your mouse over the "All Entries" row and it will show the "Add group" icon.
  2. Same should happen for any subgroup.
2026-04-05.17-33-07.mp4

Checklist

  • I own the copyright of the code submitted and I license it under the MIT license
  • I manually tested my changes in running JabRef (always required)
  • [/] I added JUnit tests for changes (if applicable)
  • I added screenshots in the PR description (if change is visible to the user)
  • [/] I added a screenshot in the PR description showing a library with a single entry with me as author and as title the issue number
  • I described the change in CHANGELOG.md in a way that can be understood by the average user (if change is visible to the user)
  • I checked the user documentation for up to dateness and submitted a pull request to our user documentation repository

@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Add hover button to group rows for adding groups

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Add hover button to group rows for adding new groups
• Display "New Group" for root and "New Subgroup" for subgroups
• Button appears on mouse hover over group rows
• Adjust column widths and styling for new button column
Diagram
flowchart LR
  A["Group Tree View"] -->|"Add Column"| B["Add Subgroup Column"]
  B -->|"Cell Factory"| C["Hover Button"]
  C -->|"Root Group"| D["New Group Button"]
  C -->|"Subgroup"| E["New Subgroup Button"]
  F["CSS Styling"] -->|"Alignment & Padding"| B
Loading

Grey Divider

File Changes

1. jabgui/src/main/java/org/jabref/gui/groups/GroupTreeView.java ✨ Enhancement +51/-3

Add hover button column to group tree view

• Added new addSubgroupColumn TreeTableColumn to display hover button
• Implemented cell factory with button that appears on row hover
• Button shows different tooltips for root ("New group") and subgroups ("New subgroup")
• Adjusted main column width binding to account for new column
• Changed addNewGroup() method visibility from private to public

jabgui/src/main/java/org/jabref/gui/groups/GroupTreeView.java


2. jabgui/src/main/resources/org/jabref/gui/Base.css ⚙️ Configuration changes +13/-0

Style new add subgroup button column

• Added CSS styling for .addSubgroupColumn with center alignment and zero padding
• Added padding rules for .addSubgroupColumn in sub and root row contexts
• Ensures consistent button appearance and spacing across different row types

jabgui/src/main/resources/org/jabref/gui/Base.css


3. jablib/src/main/resources/l10n/JabRef_en.properties 📝 Documentation +1/-0

Add localization for new subgroup label

• Added new localization key New\ subgroup=New subgroup
• Provides translated text for subgroup creation button tooltip

jablib/src/main/resources/l10n/JabRef_en.properties


Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

qodo-free-for-open-source-projects bot commented Apr 6, 2026

Code Review by Qodo

🐞 Bugs (0)   📘 Rule violations (1)   📎 Requirement gaps (0)   🎨 UX Issues (0)
📘\ ⚙ Maintainability (1)

Grey Divider


Action required

1. Subgroup add bypasses constraints🐞
Description
The new hover "+" button always triggers viewModel.addNewSubgroup(...) regardless of whether the
current group permits adding subgroups, bypassing the existing canAddGroupsIn/isEditable gating used
elsewhere. This allows creating subgroups under group types that the UI intentionally disallows
(e.g., automatic groups), leading to inconsistent/unsupported group trees.
Code

jabgui/src/main/java/org/jabref/gui/groups/GroupTreeView.java[R270-293]

+        // "Add subgroup" button shown on row hover
+        addSubgroupColumn.setCellValueFactory(cellData -> cellData.getValue().valueProperty());
+        addSubgroupColumn.setCellFactory(col -> {
+            Button button = IconTheme.JabRefIcons.ADD.asButton();
+            button.setVisible(false);
+            StackPane pane = new StackPane(button);
+
+            TreeTableCell<GroupNodeViewModel, GroupNodeViewModel> cell = new TreeTableCell<>() {
+                @Override
+                protected void updateItem(GroupNodeViewModel group, boolean empty) {
+                    super.updateItem(group, empty);
+                    if (empty || group == null) {
+                        setGraphic(null);
+                    } else {
+                        if (group.isRoot()) {
+                            button.setTooltip(new Tooltip(Localization.lang("New group")));
+                        } else {
+                            button.setTooltip(new Tooltip(Localization.lang("New subgroup")));
+                        }
+                        setGraphic(pane);
+                        button.setOnAction(event -> viewModel.addNewSubgroup(
+                                group,
+                                group.isRoot() ? GroupDialogHeader.GROUP : GroupDialogHeader.SUBGROUP));
+                    }
Evidence
The hover button’s action is set for every non-empty row without checking any permission predicates.
In contrast, the existing GROUP_SUBGROUP_ADD action is executable only when the group is editable
and can accept subgroups (or is root). The model explicitly returns false for canAddGroupsIn() for
several automatic group types, meaning the hover button is enabling a path the rest of the UI
disables.

jabgui/src/main/java/org/jabref/gui/groups/GroupTreeView.java[270-293]
jabgui/src/main/java/org/jabref/gui/groups/GroupTreeView.java[759-773]
jabgui/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java[505-530]
jabgui/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java[561-587]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new hover add-subgroup button in `GroupTreeView` always calls `viewModel.addNewSubgroup(...)` even when the target group should not accept subgroups (per `isEditable()` / `canAddGroupsIn()`). This bypasses the same constraints enforced by the context-menu action enablement.
### Issue Context
`ContextAction` for `GROUP_SUBGROUP_ADD` is executable only when `(group.isEditable() && group.canAddGroupsIn()) || group.isRoot()`. The hover button should apply the same rule (either hide the button, disable it, and/or no-op the handler when the rule is false).
### Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/groups/GroupTreeView.java[270-307]
- jabgui/src/main/java/org/jabref/gui/groups/GroupTreeView.java[759-773]
- jabgui/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java[505-530]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. New subgroup localization duplicated📘
Description
The PR introduces a new user-facing localization key New subgroup even though an existing similar
key Add subgroup already exists, increasing translation burden and risking inconsistent UI
wording. Prefer reusing the existing key unless there is a strong UX reason to distinguish them.
Code

jabgui/src/main/java/org/jabref/gui/groups/GroupTreeView.java[R284-288]

+                        if (group.isRoot()) {
+                            button.setTooltip(new Tooltip(Localization.lang("New group")));
+                        } else {
+                            button.setTooltip(new Tooltip(Localization.lang("New subgroup")));
+                        }
Evidence
GroupTreeView adds a tooltip using Localization.lang("New subgroup") and the PR adds a new `New
subgroup entry in JabRef_en.properties, despite an existing Add subgroup` localization key
already present in the same section, violating the rule to reuse existing localization strings and
avoid near-duplicates.

AGENTS.md
jabgui/src/main/java/org/jabref/gui/groups/GroupTreeView.java[284-288]
jablib/src/main/resources/l10n/JabRef_en.properties[567-572]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The tooltip introduces a near-duplicate localization string (`New subgroup`) even though an existing similar string (`Add subgroup`) already exists.
## Issue Context
Per localization rules, existing strings should be reused when appropriate to avoid translation churn and inconsistent wording.
## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/groups/GroupTreeView.java[284-288]
- jablib/src/main/resources/l10n/JabRef_en.properties[567-572]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Trivial hover comment added 📘
Description
A new comment restates what the code already clearly does (hover button behavior), adding noise
without explaining rationale. Comments should focus on intent/why rather than repeating the
implementation.
Code

jabgui/src/main/java/org/jabref/gui/groups/GroupTreeView.java[270]

+        // "Add subgroup" button shown on row hover
Evidence
The added comment // "Add subgroup" button shown on row hover simply describes the immediately
following code and does not add rationale, which violates the guideline to avoid trivial comments.

AGENTS.md
jabgui/src/main/java/org/jabref/gui/groups/GroupTreeView.java[270-270]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A new trivial comment restates the code behavior without adding rationale.
## Issue Context
Project comment guidance prefers explaining intent/why rather than repeating what the code does.
## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/groups/GroupTreeView.java[270-270]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Advisory comments

4. Unnecessary public method exposure🐞
Description
The PR changes addNewGroup() from private to public even though the shown usage remains internal to
GroupTreeView. This widens the class API surface without an in-code justification.
Code

jabgui/src/main/java/org/jabref/gui/groups/GroupTreeView.java[R691-693]

+    public void addNewGroup() {
 viewModel.addNewGroupToRoot();
}
Evidence
The only call site visible in this class is the local bottom-bar button handler; making the method
public is not required for that usage.

jabgui/src/main/java/org/jabref/gui/groups/GroupTreeView.java[193-198]
jabgui/src/main/java/org/jabref/gui/groups/GroupTreeView.java[691-693]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`addNewGroup()` was changed to `public` but is invoked from within `GroupTreeView` itself.
### Issue Context
If there is no external consumer (e.g., tests or other components) that must call it, keeping it `private` preserves encapsulation.
### Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/groups/GroupTreeView.java[691-693]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@faneeshh
Copy link
Copy Markdown
Contributor Author

faneeshh commented Apr 6, 2026

Let me know if the UI and hover behavior feels right. There was a lot of context around the issue but I'm guessing this was the planned approach.

# Grouping
Groups=Groups
New\ group=New group
New\ subgroup=New subgroup
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I was also confused whether to reuse the existing "Add subgroup" key or introduce a new one but I went with New subgroup to stay consistent with the existing New group key used by the bottom button.

this.setCenter(groupTree);

mainColumn.prefWidthProperty().bind(groupTree.widthProperty().subtract(80d).subtract(15d));
mainColumn.prefWidthProperty().bind(groupTree.widthProperty().subtract(80d).subtract(28d).subtract(15d));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Magic numbers need some explanation. Best as constants.

Copy link
Copy Markdown
Member

@subhramit subhramit Apr 6, 2026

Choose a reason for hiding this comment

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

Also, why subtract thrice?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Implying, this might need both a constant as well as a comment explaining

@calixtus
Copy link
Copy Markdown
Member

calixtus commented Apr 6, 2026

Placement of the add button looks awkward.
Move somehow to the right, next to the badge

@Siedlerchr
Copy link
Copy Markdown
Member

please test with long group names

@faneeshh
Copy link
Copy Markdown
Contributor Author

faneeshh commented Apr 7, 2026

Placement of the add button looks awkward. Move somehow to the right, next to the badge

image

How about this or should I keep it to the left of the badge but closer.

@faneeshh
Copy link
Copy Markdown
Contributor Author

faneeshh commented Apr 7, 2026

please test with long group names

Works perfectly fine.

@subhramit
Copy link
Copy Markdown
Member

The text of the subgroup titles are somehow not centered, they're leaning towards the top

@faneeshh
Copy link
Copy Markdown
Contributor Author

faneeshh commented Apr 7, 2026

The text of the subgroup titles are somehow not centered, they're leaning towards the top

It's fixed.

image

Copy link
Copy Markdown
Member

@Siedlerchr Siedlerchr left a comment

Choose a reason for hiding this comment

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

lgtm

@Siedlerchr Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Apr 9, 2026
@Siedlerchr Siedlerchr added this pull request to the merge queue Apr 9, 2026
@github-actions github-actions bot added the status: to-be-merged PRs which are accepted and should go into the merge-queue. label Apr 9, 2026
Merged via the queue into JabRef:main with commit b2bc718 Apr 9, 2026
66 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component: groups component: ui status: no-bot-comments status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers status: to-be-merged PRs which are accepted and should go into the merge-queue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"Add group" should additionally be a button "on hover"

4 participants