Improve document viewer

CHANGELOG.md

@@ -53,6 +53,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv
 - We fixed an issue where the Pubmed/Medline Plain importer would not respect the user defined keyword separator [#11413](https://github.com/JabRef/jabref/issues/11413)
 - We fixed an issue where the value of "Override default font settings" was not applied on startup [#11344](https://github.com/JabRef/jabref/issues/11344)
 - We fixed an issue where DatabaseChangeDetailsView was not scrollable when reviewing external metadata changes [#11220](https://github.com/JabRef/jabref/issues/11220)
+- We fixed an issue where clicking on a page number in the search results tab opens a wrong file in the document viewer. [#11432](https://github.com/JabRef/jabref/pull/11432)
 
 ### Removed
 

src/main/java/org/jabref/gui/documentviewer/DocumentViewModel.java

@@ -5,14 +5,10 @@
 import javafx.collections.ObservableList;
 
 public abstract class DocumentViewModel {
- private IntegerProperty maxPages = new SimpleIntegerProperty();
+ private final IntegerProperty maxPages = new SimpleIntegerProperty();
 
  public abstract ObservableList<DocumentPageViewModel> getPages();
 
- public int getMaxPages() {
- return maxPages.get();
- }
-
  public IntegerProperty maxPagesProperty() {
  return maxPages;
  }

src/main/java/org/jabref/gui/documentviewer/DocumentViewer.fxml

@@ -46,7 +46,7 @@
  <bottom>
  <ToolBar>
  <HBox alignment="CENTER" spacing="5.0" HBox.hgrow="ALWAYS">
- <Button onAction="#previousPage" styleClass="icon-button">
+ <Button fx:id="previousButton" onAction="#previousPage" styleClass="icon-button">
  <graphic>
  <JabRefIconView glyph="PREVIOUS_LEFT"/>
  </graphic>
@@ -57,7 +57,7 @@
  <TextField fx:id="currentPage" minWidth="30.0" prefWidth="40.0"/>
  <Label text="of"/>
  <Label fx:id="maxPages"/>
- <Button onAction="#nextPage" styleClass="icon-button">
+ <Button fx:id="nextButton" onAction="#nextPage" styleClass="icon-button">
  <graphic>
  <JabRefIconView glyph="NEXT_RIGHT"/>
  </graphic>

src/main/java/org/jabref/gui/documentviewer/DocumentViewerView.java

@@ -2,15 +2,18 @@
 
 import javafx.event.ActionEvent;
 import javafx.fxml.FXML;
+import javafx.scene.control.Button;
 import javafx.scene.control.ButtonBar;
 import javafx.scene.control.ButtonType;
 import javafx.scene.control.ComboBox;
 import javafx.scene.control.Label;
 import javafx.scene.control.ScrollBar;
 import javafx.scene.control.TextField;
 import javafx.scene.control.ToggleButton;
+import javafx.scene.control.ToggleGroup;
 import javafx.scene.layout.BorderPane;
 import javafx.stage.Modality;
+import javafx.stage.Stage;
 
 import org.jabref.gui.StateManager;
 import org.jabref.gui.util.BaseDialog;
@@ -29,10 +32,13 @@ public class DocumentViewerView extends BaseDialog<Void> {
  @FXML private ScrollBar scrollBar;
  @FXML private ComboBox<LinkedFile> fileChoice;
  @FXML private BorderPane mainPane;
+ @FXML private ToggleGroup toggleGroupMode;
  @FXML private ToggleButton modeLive;
  @FXML private ToggleButton modeLock;
  @FXML private TextField currentPage;
  @FXML private Label maxPages;
+ @FXML private Button nextButton;
+ @FXML private Button previousButton;
 
  @Inject private StateManager stateManager;
  @Inject private TaskExecutor taskExecutor;
@@ -65,18 +71,22 @@ private void initialize() {
  setupModeButtons();
  }
 
- private void setupModeButtons() throws RuntimeException {
- viewModel.liveModeProperty().addListener((observable, oldValue, newValue) -> {
- modeLive.setSelected(newValue);
+ private void setupModeButtons() {
+ // make sure that always one toggle is selected
+ toggleGroupMode.selectedToggleProperty().addListener((observable, oldToggle, newToggle) -> {
+ if (newToggle == null) {
+ oldToggle.setSelected(true);
+ }
  });
 
- modeLive.setOnAction(event -> {
- if (!viewModel.liveModeProperty().get()) {
+ modeLive.selectedProperty().addListener((observable, oldValue, newValue) -> {
+ if (newValue) {
  viewModel.setLiveMode(true);
  }
  });
- modeLock.setOnAction(event -> {
- if (viewModel.liveModeProperty().get()) {
+
+ modeLock.selectedProperty().addListener((observable, oldValue, newValue) -> {
+ if (newValue) {
  viewModel.setLiveMode(false);
  }
  });
@@ -92,6 +102,11 @@ private void setupPageControls() {
  viewModel.currentPageProperty().bindBidirectional(integerFormatter.valueProperty());
  currentPage.setTextFormatter(integerFormatter);
  maxPages.textProperty().bind(viewModel.maxPagesProperty().asString());
+ previousButton.setDisable(true);
+ viewModel.currentPageProperty().addListener((observable, oldValue, newValue) -> {
+ nextButton.setDisable(newValue == viewModel.maxPagesProperty().get());
+ previousButton.setDisable(newValue == 1);
+ });
  }
 
  private void setupFileChoice() {
@@ -103,8 +118,14 @@ private void setupFileChoice() {
  (observable, oldValue, newValue) -> viewModel.switchToFile(newValue));
  // We always want that the first item is selected after a change
  // This also automatically selects the first file on the initial load
- fileChoice.itemsProperty().addListener(
- (observable, oldValue, newValue) -> fileChoice.getSelectionModel().selectFirst());
+ Stage stage = (Stage) getDialogPane().getScene().getWindow();
+ fileChoice.itemsProperty().addListener((observable, oldValue, newValue) -> {
+ if (newValue.isEmpty()) {
+ stage.close();
+ } else {
+ fileChoice.getSelectionModel().selectFirst();
+ }
+ });
  fileChoice.itemsProperty().bind(viewModel.filesProperty());
  }
 
@@ -119,8 +140,12 @@ private void setupViewer() {
  mainPane.setCenter(viewer);
  }
 
- public void setLiveMode(boolean liveMode) {
- modeLive.setSelected(liveMode);
+ public void disableLiveMode() {
+ modeLock.setSelected(true);
+ }
+
+ public void switchToFile(LinkedFile file) {
+ fileChoice.getSelectionModel().select(file);
  }
 
  public void gotoPage(int pageNumber) {

src/main/java/org/jabref/gui/documentviewer/DocumentViewerViewModel.java

@@ -4,8 +4,9 @@
 import java.nio.file.Path;
 import java.util.List;
 import java.util.Objects;
+import java.util.Set;
+import java.util.stream.Collectors;
 
-import javafx.application.Platform;
 import javafx.beans.property.BooleanProperty;
 import javafx.beans.property.IntegerProperty;
 import javafx.beans.property.ListProperty;
@@ -19,6 +20,7 @@
 
 import org.jabref.gui.AbstractViewModel;
 import org.jabref.gui.StateManager;
+import org.jabref.gui.util.UiTaskExecutor;
 import org.jabref.logic.util.io.FileUtil;
 import org.jabref.model.entry.BibEntry;
 import org.jabref.model.entry.LinkedFile;
@@ -38,7 +40,7 @@ public class DocumentViewerViewModel extends AbstractViewModel {
  private final PreferencesService preferencesService;
  private final ObjectProperty<DocumentViewModel> currentDocument = new SimpleObjectProperty<>();
  private final ListProperty<LinkedFile> files = new SimpleListProperty<>();
- private final BooleanProperty liveMode = new SimpleBooleanProperty();
+ private final BooleanProperty liveMode = new SimpleBooleanProperty(true);
  private final ObjectProperty<Integer> currentPage = new SimpleObjectProperty<>();
  private final IntegerProperty maxPages = new SimpleIntegerProperty();
 
@@ -48,7 +50,7 @@ public DocumentViewerViewModel(StateManager stateManager, PreferencesService pre
 
  this.stateManager.getSelectedEntries().addListener((ListChangeListener<? super BibEntry>) c -> {
  // Switch to currently selected entry in live mode
- if (isLiveMode()) {
+ if (liveMode.get()) {
  setCurrentEntries(this.stateManager.getSelectedEntries());
  }
  });
@@ -61,7 +63,7 @@ public DocumentViewerViewModel(StateManager stateManager, PreferencesService pre
  });
 
  // we need to wrap this in run later so that the max pages number is correctly shown
- Platform.runLater(() -> maxPages.bindBidirectional(
+ UiTaskExecutor.runInJavaFXThread(() -> maxPages.bind(
  EasyBind.wrapNullable(currentDocument).selectProperty(DocumentViewModel::maxPagesProperty)));
  setCurrentEntries(this.stateManager.getSelectedEntries());
  }
@@ -78,10 +80,6 @@ public IntegerProperty maxPagesProperty() {
  return maxPages;
  }
 
- private boolean isLiveMode() {
- return liveMode.get();
- }
-
  public ObjectProperty<DocumentViewModel> currentDocumentProperty() {
  return currentDocument;
  }
@@ -91,18 +89,13 @@ public ListProperty<LinkedFile> filesProperty() {
  }
 
  private void setCurrentEntries(List<BibEntry> entries) {
- if (!entries.isEmpty()) {
- BibEntry firstSelectedEntry = entries.getFirst();
- setCurrentEntry(firstSelectedEntry);
- }
- }
-
- private void setCurrentEntry(BibEntry entry) {
- stateManager.getActiveDatabase().ifPresent(database -> {
- List<LinkedFile> linkedFiles = entry.getFiles();
+ if (entries.isEmpty()) {
+ files.clear();
+ } else {
+ Set<LinkedFile> linkedFiles = entries.stream().map(BibEntry::getFiles).flatMap(List::stream).collect(Collectors.toSet());
  // We don't need to switch to the first file, this is done automatically in the UI part
  files.setValue(FXCollections.observableArrayList(linkedFiles));
- });
+ }
  }
 
  private void setCurrentDocument(Path path) {
@@ -121,23 +114,28 @@ public void switchToFile(LinkedFile file) {
  stateManager.getActiveDatabase()
  .flatMap(database -> file.findIn(database, preferencesService.getFilePreferences()))
  .ifPresent(this::setCurrentDocument);
+ currentPage.set(1);
  }
  }
 
- public BooleanProperty liveModeProperty() {
- return liveMode;
- }
-
  public void showPage(int pageNumber) {
- currentPage.set(pageNumber);
+ if (pageNumber >= 1 && pageNumber <= maxPages.get()) {
+ currentPage.set(pageNumber);
+ } else {
+ currentPage.set(1);
+ }
  }
 
  public void showNextPage() {
- currentPage.set(getCurrentPage() + 1);
+ if (getCurrentPage() < maxPages.get()) {
+ currentPage.set(getCurrentPage() + 1);
+ }
  }
 
  public void showPreviousPage() {
- currentPage.set(getCurrentPage() - 1);
+ if (getCurrentPage() > 1) {
+ currentPage.set(getCurrentPage() - 1);
+ }
  }
 
  public void setLiveMode(boolean value) {

src/main/java/org/jabref/gui/documentviewer/ShowDocumentViewerAction.java

@@ -11,14 +11,18 @@
 import static org.jabref.gui.actions.ActionHelper.needsEntriesSelected;
 
 public class ShowDocumentViewerAction extends SimpleCommand {
+ private final DialogService dialogService = Injector.instantiateModelOrService(DialogService.class);
+ private DocumentViewerView documentViewerView;
 
  public ShowDocumentViewerAction(StateManager stateManager, PreferencesService preferences) {
  this.executable.bind(needsEntriesSelected(stateManager).and(ActionHelper.isFilePresentForSelectedEntry(stateManager, preferences)));
  }
 
  @Override
  public void execute() {
- DialogService dialogService = Injector.instantiateModelOrService(DialogService.class);
- dialogService.showCustomDialog(new DocumentViewerView());
+ if (documentViewerView == null) {
+ documentViewerView = new DocumentViewerView();
+ }
+ dialogService.showCustomDialog(documentViewerView);
  }
 }