Skip to content

feat: Issue add right click option to delete custom bst style #14525

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

Closed
EnzoHirotani wants to merge 3 commits into from
+80 −9
Closed

feat: Issue add right click option to delete custom bst style #14525

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b77987e
feat: add right click option to delete custom bst style (#M2)
EnzoHirotani Dec 5, 2025
f19e84e
Described the change in CHANGELOG.md
EnzoHirotani Dec 5, 2025
03a39db
Merge branch 'main' into feat/RClickToDeleteCustomBSTStyle
koppor Dec 5, 2025
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv

## [Unreleased]

- We added a right-click "Delete" option for custom BST styles in the Entry Preview settings, allowing users to easily remove imported styles. [#14352](https://github.com/JabRef/jabref/issues/14352)
Copy link
Member

@koppor koppor Dec 5, 2025

Choose a reason for hiding this comment

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

Why not following keep-a-changelog format?


### Added

- We added a drop-down menu to those custom fields in the main table for which content selector values exists. [#14087](https://github.com/JabRef/jabref/issues/14087)
Expand Down
38 changes: 32 additions & 6 deletions jabgui/src/main/java/org/jabref/gui/preferences/preview/PreviewTab.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.jabref.gui.util.FileDialogConfiguration;
import org.jabref.gui.util.IconValidationDecorator;
import org.jabref.gui.util.ViewModelListCellFactory;
import org.jabref.logic.bst.BstPreviewLayout;
import org.jabref.logic.l10n.Localization;
import org.jabref.logic.preview.PreviewLayout;
import org.jabref.logic.util.StandardFileType;
Expand Down Expand Up @@ -111,7 +112,7 @@ public String getTabName() {
}

@FXML
private void selectBstFile(ActionEvent event) {
private void selectBstFile(@SuppressWarnings("unused") ActionEvent event) {
FileDialogConfiguration fileDialogConfiguration = new FileDialogConfiguration.Builder()
.addExtensionFilter(StandardFileType.BST)
.withDefaultExtension(StandardFileType.BST)
Expand All @@ -121,6 +122,7 @@ private void selectBstFile(ActionEvent event) {
dialogService.showFileOpenDialog(fileDialogConfiguration).ifPresent(bstFile -> viewModel.addBstStyle(bstFile));
}

@SuppressWarnings("unused")
Copy link
Member

@koppor koppor Dec 5, 2025

Choose a reason for hiding this comment

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

Why this? This is too global?!

Copy link
Member

@calixtus calixtus Dec 8, 2025

Choose a reason for hiding this comment

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

Preferably dont suppress unused at all

public void initialize() {
this.viewModel = new PreviewTabViewModel(dialogService, preferences.getPreviewPreferences(), taskExecutor, stateManager);
lastKeyPressTime = System.currentTimeMillis();
Expand All @@ -145,20 +147,22 @@ public void initialize() {
viewModel.availableSelectionModelProperty().setValue(availableListView.getSelectionModel());
new ViewModelListCellFactory<PreviewLayout>()
.withText(PreviewLayout::getDisplayName)
.withContextMenu(this::createContextMenuForLayout)
.install(availableListView);
availableListView.setOnDragOver(this::dragOver);
availableListView.setOnDragDetected(this::dragDetectedInAvailable);
availableListView.setOnDragDropped(event -> dragDropped(viewModel.availableListProperty(), event));
availableListView.setOnKeyTyped(event -> jumpToSearchKey(availableListView, event));
availableListView.setOnMouseClicked(this::mouseClickedAvailable);
availableListView.getSelectionModel().setSelectionMode(SelectionMode.MULTIPLE);
availableListView.selectionModelProperty().getValue().selectedItemProperty().addListener((observable, oldValue, newValue) ->
availableListView.selectionModelProperty().getValue().selectedItemProperty().addListener((_observable, _oldValue, newValue) ->
Comment on lines -155 to +158
Copy link
Member

@koppor koppor Dec 5, 2025

Choose a reason for hiding this comment

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

???????????????

viewModel.setPreviewLayout(newValue));

chosenListView.itemsProperty().bindBidirectional(viewModel.chosenListProperty());
viewModel.chosenSelectionModelProperty().setValue(chosenListView.getSelectionModel());
new ViewModelListCellFactory<PreviewLayout>()
.withText(PreviewLayout::getDisplayName)
.withContextMenu(this::createContextMenuForLayout)
.setOnDragDropped(this::dragDroppedInChosenCell)
.install(chosenListView);
chosenListView.setOnDragOver(this::dragOver);
Expand All @@ -167,7 +171,7 @@ public void initialize() {
chosenListView.setOnKeyTyped(event -> jumpToSearchKey(chosenListView, event));
chosenListView.setOnMouseClicked(this::mouseClickedChosen);
chosenListView.getSelectionModel().setSelectionMode(SelectionMode.MULTIPLE);
chosenListView.selectionModelProperty().getValue().selectedItemProperty().addListener((observable, oldValue, newValue) ->
chosenListView.selectionModelProperty().getValue().selectedItemProperty().addListener((_observable, _oldValue, newValue) ->
viewModel.setPreviewLayout(newValue));

toRightButton.disableProperty().bind(viewModel.availableSelectionModelProperty().getValue().selectedItemProperty().isNull());
Expand Down Expand Up @@ -198,16 +202,16 @@ public void initialize() {
viewModel.refreshPreview();
});

editArea.textProperty().addListener((obs, oldValue, newValue) ->
editArea.textProperty().addListener((_observable, _oldValue, newValue) ->
editArea.setStyleSpans(0, viewModel.computeHighlighting(newValue)));

editArea.focusedProperty().addListener((observable, oldValue, newValue) -> {
editArea.focusedProperty().addListener((_observable, _oldValue, newValue) -> {
if (!newValue) {
viewModel.refreshPreview();
}
});

searchBox.textProperty().addListener((observable, previousText, searchTerm) -> viewModel.setAvailableFilter(searchTerm));
searchBox.textProperty().addListener((_observable, _previousText, searchTerm) -> viewModel.setAvailableFilter(searchTerm));

readOnlyLabel.visibleProperty().bind(viewModel.selectedIsEditableProperty().not());
resetDefaultButton.disableProperty().bind(viewModel.selectedIsEditableProperty().not());
Expand Down Expand Up @@ -311,4 +315,26 @@ private void mouseClickedChosen(MouseEvent event) {
event.consume();
}
}

private ContextMenu createContextMenuForLayout(PreviewLayout layout) {
// Only show context menu for BST files that were manually added
if (!(layout instanceof BstPreviewLayout) || !viewModel.canRemoveBstStyle(layout)) {
return null;
}

ActionFactory factory = new ActionFactory();
ContextMenu menu = new ContextMenu();
menu.getItems().add(
factory.createMenuItem(StandardActions.DELETE, new SimpleCommand() {
@Override
public void execute() {
viewModel.removeBstStyle(layout);
}
})
);
menu.getItems().forEach(item -> item.setGraphic(null));
menu.getStyleClass().add("context-menu");

return menu;
}
}
49 changes: 46 additions & 3 deletions jabgui/src/main/java/org/jabref/gui/preferences/preview/PreviewTabViewModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.Comparator;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.regex.Matcher;
Expand Down Expand Up @@ -74,6 +75,7 @@ public class PreviewTabViewModel implements PreferenceTabViewModel {
private final ObjectProperty<MultipleSelectionModel<PreviewLayout>> chosenSelectionModelProperty = new SimpleObjectProperty<>(new NoSelectionModel<>());

private final ListProperty<Path> bstStylesPaths = new SimpleListProperty<>(FXCollections.observableArrayList());
private final Set<Path> manuallyAddedBstPaths = new HashSet<>();

private final BooleanProperty selectedIsEditableProperty = new SimpleBooleanProperty(false);
private final ObjectProperty<PreviewLayout> selectedLayoutProperty = new SimpleObjectProperty<>();
Expand All @@ -89,6 +91,7 @@ public class PreviewTabViewModel implements PreferenceTabViewModel {
private ListProperty<PreviewLayout> dragSourceList = null;
private ObjectProperty<MultipleSelectionModel<PreviewLayout>> dragSourceSelectionModel = null;

@SuppressWarnings("unused")
public PreviewTabViewModel(DialogService dialogService,
PreviewPreferences previewPreferences,
TaskExecutor taskExecutor,
Expand All @@ -98,15 +101,15 @@ public PreviewTabViewModel(DialogService dialogService,
this.localDragboard = stateManager.getLocalDragboard();
this.previewPreferences = previewPreferences;

sourceTextProperty.addListener((observable, oldValue, newValue) -> {
sourceTextProperty.addListener((_observable, _oldValue, newValue) -> {
if (selectedLayoutProperty.getValue() instanceof TextBasedPreviewLayout layout) {
layout.setText(sourceTextProperty.getValue());
layout.setText(newValue);
}
});

chosenListValidator = new FunctionBasedValidator<>(
chosenListProperty,
input -> !chosenListProperty.getValue().isEmpty(),
_input -> !chosenListProperty.getValue().isEmpty(),
ValidationMessage.error("%s > %s %n %n %s".formatted(
Localization.lang("Entry preview"),
Localization.lang("Selected"),
Expand Down Expand Up @@ -143,6 +146,7 @@ public void setValues() {
})
.executeWith(taskExecutor);
bstStylesPaths.clear();
manuallyAddedBstPaths.clear(); // Reset the set of manually added files
Copy link
Member

@koppor koppor Dec 5, 2025

Choose a reason for hiding this comment

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

The comment states the obvious. Talk about the WHY

bstStylesPaths.addAll(previewPreferences.getBstPreviewLayoutPaths());
bstStylesPaths.forEach(path -> {
BstPreviewLayout layout = new BstPreviewLayout(path);
Expand Down Expand Up @@ -180,6 +184,7 @@ public void refreshPreview() {
setPreviewLayout(chosenSelectionModelProperty.getValue().getSelectedItem());
}

@SuppressWarnings("SameParameterValue")
Copy link
Member

@koppor koppor Dec 5, 2025

Choose a reason for hiding this comment

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

????

private PreviewLayout findLayoutByName(String name) {
return availableListProperty.getValue().stream().filter(layout -> layout.getName().equals(name))
.findAny()
Expand Down Expand Up @@ -501,7 +506,45 @@ public StringProperty sourceTextProperty() {
public void addBstStyle(Path bstFile) {
BstPreviewLayout bstPreviewLayout = new BstPreviewLayout(bstFile);
bstStylesPaths.add(bstFile);
manuallyAddedBstPaths.add(bstFile); // Mark as manually added
Copy link
Member

@koppor koppor Dec 5, 2025

Choose a reason for hiding this comment

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

The comment states the obvious. Talk about the WHY

availableListProperty().add(bstPreviewLayout);
chosenListProperty().add(bstPreviewLayout);
}

public void removeBstStyle(PreviewLayout layout) {
if (!(layout instanceof BstPreviewLayout bstLayout)) {
Copy link
Member

@koppor koppor Dec 5, 2025

Choose a reason for hiding this comment

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

no assignment to variable needed.

Why isn't the method parameter dirctly BstPreviewLayout?

return;
}

// Find the corresponding path by matching the file name
String layoutName = bstLayout.getName();
Path pathToRemove = bstStylesPaths.stream()
.filter(path -> path.getFileName().toString().equals(layoutName))
.findFirst()
.orElse(null);

// Only remove if it was manually added in this session
if (pathToRemove != null && manuallyAddedBstPaths.contains(pathToRemove)) {
bstStylesPaths.remove(pathToRemove);
manuallyAddedBstPaths.remove(pathToRemove);

// Remove from both lists
availableListProperty().remove(layout);
chosenListProperty().remove(layout);
}
}

public boolean canRemoveBstStyle(PreviewLayout layout) {
if (!(layout instanceof BstPreviewLayout bstLayout)) {
return false;
}

String layoutName = bstLayout.getName();
Path pathToCheck = bstStylesPaths.stream()
.filter(path -> path.getFileName().toString().equals(layoutName))
.findFirst()
.orElse(null);

return pathToCheck != null && manuallyAddedBstPaths.contains(pathToCheck);
}
}