Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,14 @@
import java.util.Locale;
import java.util.Optional;

import javafx.application.Platform;
import javafx.fxml.FXML;
import javafx.scene.control.ButtonType;
import javafx.scene.control.ListView;
import javafx.scene.control.ScrollPane;
import javafx.scene.control.ToggleButton;
import javafx.scene.input.KeyCode;
import javafx.scene.input.KeyEvent;

import org.jabref.gui.DialogService;
import org.jabref.gui.icon.IconTheme;
Expand Down Expand Up @@ -115,6 +117,58 @@ private void initialize() {
memoryStickMode.selectedProperty().bindBidirectional(viewModel.getMemoryStickProperty());

viewModel.setValues();
// Delay execution until JavaFX has completed layout and the Scene object is attached to the DialogPane
Platform.runLater(this::installGlobalCloseBehavior);
}

private void installGlobalCloseBehavior() {
var scene = getDialogPane().getScene();
Copy link
Member

Choose a reason for hiding this comment

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

We avoid var and use explicit types


// Capture events BEFORE child nodes
scene.addEventFilter(KeyEvent.KEY_PRESSED, e -> {
if (!preferences.getKeyBindingRepository().checkKeyCombinationEquality(KeyBinding.CLOSE, e)) {
return;
}

// If any cell-based control is editing (ListView/TableView/TreeView/TreeTableView),
// record that the close input is for canceling edit; DO NOT consume, let the control cancel.
if (isAnyCellControlEditing()) {
return;
}

// No editing in progress --> close dialog
closeDialog();
e.consume();
});
}

private boolean isAnyCellControlEditing() {
var root = getDialogPane();

for (var n : root.lookupAll(".list-view")) {
if (n instanceof javafx.scene.control.ListView<?> lv && lv.getEditingIndex() != -1) {
Copy link
Member

@subhramit subhramit Oct 25, 2025

Choose a reason for hiding this comment

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

Use imports instead of raw library references.

Copy link
Member

Choose a reason for hiding this comment

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

We also avoid abbreviations

return true;
}
}

for (var n : root.lookupAll(".table-view")) {
if (n instanceof javafx.scene.control.TableView<?> tv && tv.getEditingCell() != null) {
return true;
}
}

for (var n : root.lookupAll(".tree-view")) {
if (n instanceof javafx.scene.control.TreeView<?> trv && trv.getEditingItem() != null) {
return true;
}
}

for (var n : root.lookupAll(".tree-table-view")) {
if (n instanceof javafx.scene.control.TreeTableView<?> ttv && ttv.getEditingCell() != null) {
return true;
Copy link
Member

Choose a reason for hiding this comment

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

Similar comments applicable here

}
}
return false;
}

@FXML
Expand Down
2 changes: 2 additions & 0 deletions jabgui/src/main/java/org/jabref/gui/util/BaseDialog.java
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// BaseDialog.java

Copy link
Member

Choose a reason for hiding this comment

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

Rationale behind this comment? Why was this file touched?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rationale behind this comment? Why was this file touched?

In the beginning, when we were trying to understand what triggered the bug, we thought that BaseDialog.java might have some role to play because the ESC was not closing a dialog, although we were wrong, as it was in the Preferences tab. But, we didn't make any changes in the code of this file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, shall we change our implementation? Will changing the code make a difference?

Copy link
Member

Choose a reason for hiding this comment

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

No, but why was this comment added ^

Copy link
Contributor Author

@VishakhaMathur VishakhaMathur Oct 25, 2025

Choose a reason for hiding this comment

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

I am sorry, I think the comment was added by mistake because I am not able to recall the reason behind adding it. It has now been removed in the new commit. I apologize for the oversight.

package org.jabref.gui.util;

import java.util.Optional;
Expand Down
Loading