Skip to content
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.

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

Implemented error console in JavaFX #1383

Merged
merged 15 commits into from
Sep 19, 2016
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `#
- Redesigned about dialog.
- Redesigned key bindings dialog.
- Redesigned journal abbreviations dialog.
- Redesigned error console.

### Changed
- Added integrity check for fields with BibTeX keys, e.g., `crossref` and `related`, to check that the key exists
Expand Down
28 changes: 23 additions & 5 deletions src/main/java/net/sf/jabref/gui/FXAlert.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import javafx.fxml.FXMLLoader;
import javafx.scene.control.Alert;
import javafx.scene.image.Image;
import javafx.stage.Modality;
import javafx.stage.Stage;

import net.sf.jabref.Globals;
Expand Down Expand Up @@ -71,26 +72,39 @@ public void windowGainedFocus(WindowEvent e) {
}
};

public FXAlert(AlertType type, String title, Image image, boolean isModal) {
Copy link
Member

Choose a reason for hiding this comment

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

I would leave all the old constructors and call the new overloads with isModal=true since this looks like the default behavior. For example:
public FXAlert(AlertType type, String title) { this(type, title, true); }.

this(type, title, isModal);
setDialogIcon(image);
}

public FXAlert(AlertType type, String title, Image image) {
this(type, title);
this(type, title, true);
setDialogIcon(image);
}

public FXAlert(AlertType type, String title, boolean isModal) {
this(type, isModal);
setTitle(title);
}

public FXAlert(AlertType type, String title) {
this(type);
setTitle(title);
}

public FXAlert(AlertType type) {
public FXAlert(AlertType type, boolean isModal) {
super(type);
Stage fxDialogWindow = getDialogWindow();

fxDialogWindow.setOnCloseRequest(evt -> this.close());
if (isModal) {
initModality(Modality.APPLICATION_MODAL);
} else {
initModality(Modality.NONE);
}
fxDialogWindow.setOnShown(evt -> {
setSwingWindowsEnabledAndFocusable(false);
setSwingWindowsEnabledAndFocusable(!isModal);
setLocationRelativeToMainWindow();
});

fxDialogWindow.setOnHiding(evt -> setSwingWindowsEnabledAndFocusable(true));

fxDialogWindow.setOnCloseRequest(evt -> this.close());
Expand All @@ -103,6 +117,10 @@ public FXAlert(AlertType type) {
});
}

public FXAlert(AlertType type) {
this(type, true);
}

public void setDialogStyle(String pathToStyleSheet) {
getDialogPane().getScene().getStylesheets().add(pathToStyleSheet);
}
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/net/sf/jabref/gui/FXDialogs.java
Original file line number Diff line number Diff line change
Expand Up @@ -119,14 +119,14 @@ public static Optional<ButtonType> showCustomButtonDialogAndWait(AlertType type,
*/
public static Optional<ButtonType> showCustomDialogAndWait(String title, DialogPane contentPane,
ButtonType... buttonTypes) {
FXAlert alert = new FXAlert(AlertType.NONE, title);
FXAlert alert = new FXAlert(AlertType.NONE, title, true);
Copy link
Member

Choose a reason for hiding this comment

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

true is default, right? If so, remove it.

alert.setDialogPane(contentPane);
alert.getButtonTypes().setAll(buttonTypes);
return alert.showAndWait();
}

private static FXAlert createDialog(AlertType type, String title, String content) {
FXAlert alert = new FXAlert(type, title);
FXAlert alert = new FXAlert(type, title, true);
alert.setHeaderText(null);
alert.setContentText(content);
return alert;
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/net/sf/jabref/gui/JabRefFrame.java
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ public void actionPerformed(ActionEvent e) {
private final AbstractAction manageJournals = new ManageJournalsAction();
private final AbstractAction databaseProperties = new DatabasePropertiesAction();
private final AbstractAction bibtexKeyPattern = new BibtexKeyPatternAction();
private final AbstractAction errorConsole = new ErrorConsoleAction(this, Globals.getStreamEavesdropper(), GuiAppender.CACHE);
private final AbstractAction errorConsole = new ErrorConsoleAction(Globals.getStreamEavesdropper(), GuiAppender.CACHE);

private final AbstractAction cleanupEntries = new GeneralAction(Actions.CLEANUP,
Localization.menuTitle("Cleanup entries") + ELLIPSES,
Expand Down
58 changes: 6 additions & 52 deletions src/main/java/net/sf/jabref/gui/actions/ErrorConsoleAction.java
Original file line number Diff line number Diff line change
@@ -1,16 +1,13 @@
package net.sf.jabref.gui.actions;

import java.awt.Dimension;
import java.awt.event.ActionEvent;

import javax.swing.AbstractAction;
import javax.swing.Action;
import javax.swing.JFrame;
import javax.swing.JOptionPane;
import javax.swing.JScrollPane;
import javax.swing.JTabbedPane;
import javax.swing.JTextArea;

import javafx.application.Platform;

import net.sf.jabref.gui.errorconsole.ErrorConsoleView;
import net.sf.jabref.logic.error.StreamEavesdropper;
import net.sf.jabref.logic.l10n.Localization;
import net.sf.jabref.logic.logging.Cache;
Expand All @@ -24,57 +21,14 @@
*/
public class ErrorConsoleAction extends AbstractAction {

private final JFrame frame;
private final StreamEavesdropper streamEavesdropper;
private final Cache cache;

public ErrorConsoleAction(JFrame frame, StreamEavesdropper streamEavesdropper, Cache cache) {
super(Localization.menuTitle("Show error console"));
this.streamEavesdropper = streamEavesdropper;
this.cache = cache;
public ErrorConsoleAction(StreamEavesdropper streamEavesdropper, Cache cache) {
super(Localization.menuTitle("View event log"));
putValue(Action.SHORT_DESCRIPTION, Localization.lang("Display all error messages"));
this.frame = frame;
}

@Override
public void actionPerformed(ActionEvent e) {
displayErrorConsole(frame);
}

private void displayErrorConsole(JFrame parent) {
JTabbedPane tabbed = new JTabbedPane();

addTextArea(tabbed, Localization.lang("Log"), cache.get());
addTextArea(tabbed, Localization.lang("Exceptions"), streamEavesdropper.getErrorMessages(),
Localization.lang("No exceptions have occurred."));
addTextArea(tabbed, Localization.lang("Output"), streamEavesdropper.getOutput());

tabbed.setPreferredSize(new Dimension(500, 500));

JOptionPane.showMessageDialog(parent, tabbed,
Localization.lang("Program output"), JOptionPane.ERROR_MESSAGE);
Platform.runLater(() -> new ErrorConsoleView().show());
}

/**
* @param tabbed the tabbed pane to add the tab to
* @param output the text to display in the tab
* @param ifEmpty Text to output if textbox is emtpy. may be null
*/
private static void addTextArea(JTabbedPane tabbed, String title, String output, String ifEmpty) {
JTextArea ta = new JTextArea(output);
ta.setEditable(false);
if ((ifEmpty != null) && (ta.getText().isEmpty())) {
ta.setText(ifEmpty);
}
JScrollPane sp = new JScrollPane(ta);
tabbed.addTab(title, sp);
}

/**
* @param tabbed the tabbed pane to add the tab to
* @param output the text to display in the tab
*/
private static void addTextArea(JTabbedPane tabbed, String title, String output) {
addTextArea(tabbed, title, output, null);
}
}
142 changes: 142 additions & 0 deletions src/main/java/net/sf/jabref/gui/errorconsole/ErrorConsoleView.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
/* Copyright (C) 2016 JabRef contributors.
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation; either version 2 of the License, or
(at your option) any later version.

This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.

You should have received a copy of the GNU General Public License along
with this program; if not, write to the Free Software Foundation, Inc.,
51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
*/
package net.sf.jabref.gui.errorconsole;

import javafx.collections.ObservableList;
import javafx.fxml.FXML;
import javafx.scene.control.Alert.AlertType;
import javafx.scene.control.Button;
import javafx.scene.control.ButtonBar;
import javafx.scene.control.DialogPane;
import javafx.scene.control.Label;
import javafx.scene.control.ListCell;
import javafx.scene.control.ListView;
import javafx.scene.text.Text;
import javafx.stage.Stage;
import javafx.util.Callback;

import net.sf.jabref.gui.FXAlert;
import net.sf.jabref.gui.IconTheme;
import net.sf.jabref.logic.error.LogMessageWithPriority;
import net.sf.jabref.logic.error.MessagePriority;
import net.sf.jabref.logic.l10n.Localization;
import net.sf.jabref.logic.logging.LogMessage;

import com.airhacks.afterburner.views.FXMLView;

public class ErrorConsoleView extends FXMLView {

private final ErrorConsoleViewModel errorViewModel = new ErrorConsoleViewModel();

@FXML
private Button closeButton;
@FXML
private Button copyLogButton;
@FXML
private Button createIssueButton;
@FXML
private ListView<LogMessageWithPriority> allMessage;
Copy link
Member

Choose a reason for hiding this comment

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

add plural s

@FXML
private Label descriptionLabel;

public ErrorConsoleView() {
super();
bundle = Localization.getMessages();
}

public void show() {
FXAlert errorConsole = new FXAlert(AlertType.ERROR, Localization.lang("Event log"), false);
DialogPane pane = (DialogPane) this.getView();
errorConsole.setDialogPane(pane);
errorConsole.setResizable(true);
errorConsole.show();
}

@FXML
private void initialize() {
ButtonBar.setButtonData(copyLogButton, ButtonBar.ButtonData.LEFT);
Copy link
Member

Choose a reason for hiding this comment

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

Please move this to FXML: http://stackoverflow.com/a/32427404/873661

ButtonBar.setButtonData(createIssueButton, ButtonBar.ButtonData.LEFT);

ObservableList<LogMessageWithPriority> masterData = LogMessage.getInstance().messagesProperty();
Copy link
Member

Choose a reason for hiding this comment

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

The view shouldn't know about the datasource (in this case LogMessage) but ask the viewmodel. Could you please add a pass-through getter in errorViewModel.
Also inline masterData, i.e. allMessages.setItems(errorViewModel.getMessages())

listViewStyle();
allMessage.setItems(masterData);
descriptionLabel.setGraphic(IconTheme.JabRefIcon.CONSOLE.getGraphicNode());
}

@FXML
private void copyLogButton() {
errorViewModel.copyLog();
}

@FXML
private void createIssueButton() {
errorViewModel.reportIssue();
}

@FXML
private void closeErrorDialog() {
Stage stage = (Stage) closeButton.getScene().getWindow();
stage.close();
}

/**
* Style the list view with icon and message color
*/
private void listViewStyle() {
// Handler for listCell appearance (example for exception Cell)
allMessage.setCellFactory(
new Callback<ListView<LogMessageWithPriority>, ListCell<LogMessageWithPriority>>() {
@Override
public ListCell<LogMessageWithPriority> call(
ListView<LogMessageWithPriority> listView) {
return new ListCell<LogMessageWithPriority>() {

@Override
public void updateItem(LogMessageWithPriority logMessageWithPriority, boolean empty) {
super.updateItem(logMessageWithPriority, empty);
if (logMessageWithPriority != null) {
setText(logMessageWithPriority.getMessage());
Text graphic = new Text();
graphic.getStyleClass().add("icon");

MessagePriority prio = logMessageWithPriority.getPriority();
switch (prio) {
case HIGH:
getStyleClass().add("exception");
graphic.setText(IconTheme.JabRefIcon.INTEGRITY_FAIL.getCode());
Copy link
Member

Choose a reason for hiding this comment

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

setGraphic(IconTheme....getGraphicNode())

break;
case MEDIUM:
getStyleClass().add("output");
graphic.setText(IconTheme.JabRefIcon.INTEGRITY_WARN.getCode());
break;
case LOW:
getStyleClass().add("log");
graphic.setText(IconTheme.JabRefIcon.INTEGRITY_INFO.getCode());
break;
default:
break;
}
setGraphic(graphic);
} else {
setText(null);
setGraphic(null);
}
}
};
}
});
}
}
Loading