Skip to content

Fix #15397 invisible delete area for internal CSL styles #15399

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.

Sign up for GitHub

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

Merged
Siedlerchr merged 9 commits into from
Apr 12, 2026
+20 −10
Merged

Fix #15397 invisible delete area for internal CSL styles #15399

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
2898a4a
Fix invisible delete button for internal CSL styles
Francis-Lee-210 Mar 24, 2026
5f23cc4
Add changelog entry for internal CSL delete fix
Francis-Lee-210 Mar 24, 2026
55e1d1f
Fix import order in StyleSelectDialogView
Francis-Lee-210 Mar 24, 2026
affabb0
Use ValueTableCellFactory for CSL delete cell
Francis-Lee-210 Mar 25, 2026
693b03a
Merge branch 'main' into fix-15397-internal-csl-delete
Francis-Lee-210 Mar 27, 2026
e5b1a59
Merge branch 'main' into fix-15397-internal-csl-delete
subhramit Mar 27, 2026
a665b63
Fix CSL delete cell tooltip reuse and click handling
Francis-Lee-210 Apr 5, 2026
401fd47
Merge branch 'main' into fix-15397-internal-csl-delete
Francis-Lee-210 Apr 6, 2026
b2a85d4
Avoid dead zone in internal CSL delete column
Francis-Lee-210 Apr 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv
- We improved CSL support with JabRef LibreOffice converter extension. [#14387](https://github.com/JabRef/jabref/issues/14387)
- We fixed an issue where a search group would not be updated in the ui after an entry change [#13378](https://github.com/JabRef/jabref/issues/13378)
- We fixed exceptions occuring when generating citation keys or using certain cleanup operations on macOS [#15366](https://github.com/JabRef/jabref/issues/15366)
- We fixed an issue where the invisible delete button area was shown for internal CSL styles in the OpenOffice/LibreOffice style selection dialog. External CSL styles remain deletable. [#15397](https://github.com/JabRef/jabref/issues/15397)
- We fixed an issue that prevented files without a file extension from being shown in various file select dialogs [#11786](https://github.com/JabRef/jabref/issues/11786)

### Removed
Expand Down
20 changes: 14 additions & 6 deletions jabgui/src/main/java/org/jabref/gui/openoffice/StyleSelectDialogView.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import javafx.scene.control.TabPane;
import javafx.scene.control.TableColumn;
import javafx.scene.control.TableView;
import javafx.scene.input.MouseButton;
import javafx.scene.layout.VBox;

import org.jabref.gui.DialogService;
Expand Down Expand Up @@ -143,14 +144,21 @@ private void setupCslStylesTab() {
cslDeleteColumn.setCellValueFactory(cellData -> cellData.getValue().internalStyleProperty());

new ValueTableCellFactory<CSLStyleSelectViewModel, Boolean>()
.withGraphic(internalStyle -> internalStyle ? null : IconTheme.JabRefIcons.DELETE_ENTRY.getGraphicNode())
.withOnMouseClickedEvent(_ -> _ -> {
CSLStyleSelectViewModel selectedStyle = cslStylesTable.getSelectionModel().getSelectedItem();
if (selectedStyle != null) {
viewModel.deleteCslStyle(selectedStyle.getLayout().citationStyle());
.withGraphic((_, internalStyle) -> internalStyle ? null : IconTheme.JabRefIcons.DELETE_ENTRY.getGraphicNode())
.withOnMouseClickedEvent((style, internalStyle) -> event -> {
if (internalStyle) {
return;
}

event.consume();

if (event.getButton() != MouseButton.PRIMARY) {
return;
}

Comment on lines +153 to +158
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

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

event.consume() is currently executed for all clicks on the delete column cell, including internal styles and non-primary buttons. This prevents the event from bubbling to the row handler (used for double-click selection) and can create a “dead zone” where clicking the (empty) delete column for internal styles won’t select/activate the row. Consider only consuming the event when actually performing the delete (external style + primary click), and letting other clicks propagate normally.

Suggested change
event.consume();
if (internalStyle || (event.getButton() != MouseButton.PRIMARY)) {
return;
}
if (internalStyle || (event.getButton() != MouseButton.PRIMARY)) {
return;
}
event.consume();

Copilot uses AI. Check for mistakes.
viewModel.deleteCslStyle(style.getLayout().citationStyle());
})
.withTooltip(_ -> Localization.lang("Remove style"))
.withTooltip((_, internalStyle) -> internalStyle ? null : Localization.lang("Remove style"))
.install(cslDeleteColumn);

new ViewModelTableRowFactory<CSLStyleSelectViewModel>()
Expand Down
9 changes: 5 additions & 4 deletions jabgui/src/main/java/org/jabref/gui/util/ValueTableCellFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,14 +127,15 @@ protected void updateItem(T item, boolean empty) {
}
if (toTooltip != null) {
String tooltipText = toTooltip.apply(rowItem, item);
Tooltip cellTooltip = null;
Copy link
Copy Markdown
Contributor

@pluto-han pluto-han Apr 5, 2026

Choose a reason for hiding this comment

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

why rename tooltip to celltooltip

Copy link
Copy Markdown
Contributor Author

@Francis-Lee-210 Francis-Lee-210 Apr 6, 2026

Choose a reason for hiding this comment

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

wants to makes it clear this is the tooltip object to apply to this cell, not the factory field.

if (StringUtil.isNotBlank(tooltipText)) {
Screen currentScreen = Screen.getPrimary();
double maxWidth = currentScreen.getBounds().getWidth();
Tooltip tooltip = new Tooltip(tooltipText);
tooltip.setMaxWidth(maxWidth * 2 / 3);
tooltip.setWrapText(true);
setTooltip(tooltip);
cellTooltip = new Tooltip(tooltipText);
cellTooltip.setMaxWidth(maxWidth * 2 / 3);
cellTooltip.setWrapText(true);
}
setTooltip(cellTooltip);
}

if (contextMenuFactory != null) {
Expand Down
Loading