Skip to content

Commit

Permalink
Remove dependency on WebView
Browse files Browse the repository at this point in the history
The `help` command is the only command that relies on WebView, a
dependency that weighs in at around 70MB, more than quadrupling the size
of the executable jar. As there are plans to customize the jars for each
individual student during the practical examination, this overhead can
lead to further problems (bandwidth in the lecture hall, storage space,
etc).

Let's remove the dependency on WebView by changing the `help` command to
output a URL to the user guide instead.
  • Loading branch information
j-lum committed Aug 5, 2019
1 parent 9acaf04 commit eae5dcc
Show file tree
Hide file tree
Showing 9 changed files with 8 additions and 194 deletions.
16 changes: 0 additions & 16 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,6 @@ dependencies {
implementation group: 'org.openjfx', name: 'javafx-media', version: javaFxVersion, classifier: 'win'
implementation group: 'org.openjfx', name: 'javafx-media', version: javaFxVersion, classifier: 'mac'
implementation group: 'org.openjfx', name: 'javafx-media', version: javaFxVersion, classifier: 'linux'
implementation group: 'org.openjfx', name: 'javafx-web', version: javaFxVersion, classifier: 'win'
implementation group: 'org.openjfx', name: 'javafx-web', version: javaFxVersion, classifier: 'mac'
implementation group: 'org.openjfx', name: 'javafx-web', version: javaFxVersion, classifier: 'linux'

implementation group: 'com.fasterxml.jackson.core', name: 'jackson-databind', version: '2.7.0'
implementation group: 'com.fasterxml.jackson.datatype', name: 'jackson-datatype-jsr310', version: '2.7.4'
Expand Down Expand Up @@ -158,17 +155,4 @@ task copyStylesheets(type: Copy) {
}
asciidoctor.dependsOn copyStylesheets

task deployOfflineDocs(type: Copy) {
into('src/main/resources/docs')

from ("${asciidoctor.outputDir}/html5") {
include 'stylesheets/*'
include 'images/*'
include 'HelpWindow.html'
}
}

deployOfflineDocs.dependsOn asciidoctor
processResources.dependsOn deployOfflineDocs

defaultTasks 'clean', 'test', 'coverage', 'asciidoctor'
3 changes: 0 additions & 3 deletions docs/HelpWindow.adoc

This file was deleted.

1 change: 0 additions & 1 deletion docs/SettingUp.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ Do not disable them. If you have disabled them, go to `File` > `Settings` > `Plu
. Locate the `build.gradle` file and select it. Click `OK`
. Click `Open as Project`
. Click `OK` to accept the default settings
. Open a console and run the command `gradlew processResources` (Mac/Linux: `./gradlew processResources`). It should finish with the `BUILD SUCCESSFUL` message. +
This will generate all resources required by the application and tests.

== Verifying the setup
Expand Down
9 changes: 6 additions & 3 deletions src/main/java/seedu/address/logic/commands/HelpCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import seedu.address.model.Model;

/**
* Format full help instructions for every command for display.
* Provides a link to the user guide.
*/
public class HelpCommand extends Command {

Expand All @@ -12,10 +12,13 @@ public class HelpCommand extends Command {
public static final String MESSAGE_USAGE = COMMAND_WORD + ": Shows program usage instructions.\n"
+ "Example: " + COMMAND_WORD;

public static final String SHOWING_HELP_MESSAGE = "Opened help window.";
private static final String USER_GUIDE_URL =
"https://github.com/se-edu/addressbook-level3/blob/master/docs/UserGuide.adoc";

public static final String MESSAGE_OUTPUT = "The user guide is available at " + USER_GUIDE_URL;

@Override
public CommandResult execute(Model model) {
return new CommandResult(SHOWING_HELP_MESSAGE, true, false);
return new CommandResult(MESSAGE_OUTPUT);
}
}
85 changes: 0 additions & 85 deletions src/main/java/seedu/address/ui/HelpWindow.java

This file was deleted.

63 changes: 0 additions & 63 deletions src/main/java/seedu/address/ui/MainWindow.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,7 @@

import java.util.logging.Logger;

import javafx.event.ActionEvent;
import javafx.fxml.FXML;
import javafx.scene.control.MenuItem;
import javafx.scene.control.TextInputControl;
import javafx.scene.input.KeyCombination;
import javafx.scene.input.KeyEvent;
import javafx.scene.layout.StackPane;
import javafx.stage.Stage;
import seedu.address.commons.core.GuiSettings;
Expand All @@ -33,14 +28,10 @@ public class MainWindow extends UiPart<Stage> {
// Independent Ui parts residing in this Ui container
private PersonListPanel personListPanel;
private ResultDisplay resultDisplay;
private HelpWindow helpWindow;

@FXML
private StackPane commandBoxPlaceholder;

@FXML
private MenuItem helpMenuItem;

@FXML
private StackPane personListPanelPlaceholder;

Expand All @@ -60,49 +51,12 @@ public MainWindow(Stage primaryStage, Logic logic) {
// Configure the UI
setWindowDefaultSize(logic.getGuiSettings());

setAccelerators();

helpWindow = new HelpWindow();
}

public Stage getPrimaryStage() {
return primaryStage;
}

private void setAccelerators() {
setAccelerator(helpMenuItem, KeyCombination.valueOf("F1"));
}

/**
* Sets the accelerator of a MenuItem.
* @param keyCombination the KeyCombination value of the accelerator
*/
private void setAccelerator(MenuItem menuItem, KeyCombination keyCombination) {
menuItem.setAccelerator(keyCombination);

/*
* TODO: the code below can be removed once the bug reported here
* https://bugs.openjdk.java.net/browse/JDK-8131666
* is fixed in later version of SDK.
*
* According to the bug report, TextInputControl (TextField, TextArea) will
* consume function-key events. Because CommandBox contains a TextField, and
* ResultDisplay contains a TextArea, thus some accelerators (e.g F1) will
* not work when the focus is in them because the key event is consumed by
* the TextInputControl(s).
*
* For now, we add following event filter to capture such key events and open
* help window purposely so to support accelerators even when focus is
* in CommandBox or ResultDisplay.
*/
getRoot().addEventFilter(KeyEvent.KEY_PRESSED, event -> {
if (event.getTarget() instanceof TextInputControl && keyCombination.match(event)) {
menuItem.getOnAction().handle(new ActionEvent());
event.consume();
}
});
}

/**
* Fills up all the placeholders of this window.
*/
Expand Down Expand Up @@ -132,18 +86,6 @@ private void setWindowDefaultSize(GuiSettings guiSettings) {
}
}

/**
* Opens the help window or focuses on it if it's already opened.
*/
@FXML
public void handleHelp() {
if (!helpWindow.isShowing()) {
helpWindow.show();
} else {
helpWindow.focus();
}
}

void show() {
primaryStage.show();
}
Expand All @@ -156,7 +98,6 @@ private void handleExit() {
GuiSettings guiSettings = new GuiSettings(primaryStage.getWidth(), primaryStage.getHeight(),
(int) primaryStage.getX(), (int) primaryStage.getY());
logic.setGuiSettings(guiSettings);
helpWindow.hide();
primaryStage.hide();
}

Expand All @@ -175,10 +116,6 @@ private CommandResult executeCommand(String commandText) throws CommandException
logger.info("Result: " + commandResult.getFeedbackToUser());
resultDisplay.setFeedbackToUser(commandResult.getFeedbackToUser());

if (commandResult.isShowHelp()) {
handleHelp();
}

if (commandResult.isExit()) {
handleExit();
}
Expand Down
18 changes: 0 additions & 18 deletions src/main/resources/view/HelpWindow.fxml

This file was deleted.

3 changes: 0 additions & 3 deletions src/main/resources/view/MainWindow.fxml
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,6 @@
<Menu mnemonicParsing="false" text="File">
<MenuItem mnemonicParsing="false" onAction="#handleExit" text="Exit" />
</Menu>
<Menu mnemonicParsing="false" text="Help">
<MenuItem fx:id="helpMenuItem" mnemonicParsing="false" onAction="#handleHelp" text="Help" />
</Menu>
</MenuBar>

<StackPane VBox.vgrow="NEVER" fx:id="commandBoxPlaceholder" styleClass="pane-with-border">
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package seedu.address.logic.commands;

import static seedu.address.logic.commands.CommandTestUtil.assertCommandSuccess;
import static seedu.address.logic.commands.HelpCommand.SHOWING_HELP_MESSAGE;
import static seedu.address.logic.commands.HelpCommand.MESSAGE_OUTPUT;

import org.junit.jupiter.api.Test;

Expand All @@ -14,7 +14,7 @@ public class HelpCommandTest {

@Test
public void execute_help_success() {
CommandResult expectedCommandResult = new CommandResult(SHOWING_HELP_MESSAGE, true, false);
CommandResult expectedCommandResult = new CommandResult(MESSAGE_OUTPUT);
assertCommandSuccess(new HelpCommand(), model, expectedCommandResult, expectedModel);
}
}

0 comments on commit eae5dcc

Please sign in to comment.