Skip to content
This repository has been archived by the owner on Mar 5, 2023. It is now read-only.

Ui: Make the ResultDisplay red on command failure #799

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@
public class NewResultAvailableEvent extends BaseEvent {

public final String message;
public final boolean isSuccessful;

public NewResultAvailableEvent(String message) {
public NewResultAvailableEvent(String message, boolean isSuccessful) {
this.message = message;
this.isSuccessful = isSuccessful;
}

@Override
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/seedu/address/ui/CommandBox.java
Original file line number Diff line number Diff line change
Expand Up @@ -107,14 +107,14 @@ private void handleCommandEntered() {
// process result of the command
commandTextField.setText("");
logger.info("Result: " + commandResult.feedbackToUser);
raise(new NewResultAvailableEvent(commandResult.feedbackToUser));
raise(new NewResultAvailableEvent(commandResult.feedbackToUser, true));

} catch (CommandException | ParseException e) {
initHistory();
// handle command failure
setStyleToIndicateCommandFailure();
logger.info("Invalid command: " + commandTextField.getText());
raise(new NewResultAvailableEvent(e.getMessage()));
raise(new NewResultAvailableEvent(e.getMessage(), false));
}
}

Expand Down
33 changes: 32 additions & 1 deletion src/main/java/seedu/address/ui/ResultDisplay.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import javafx.application.Platform;
import javafx.beans.property.SimpleStringProperty;
import javafx.beans.property.StringProperty;
import javafx.collections.ObservableList;
import javafx.fxml.FXML;
import javafx.scene.control.TextArea;
import javafx.scene.layout.Region;
Expand All @@ -18,6 +19,8 @@
*/
public class ResultDisplay extends UiPart<Region> {

public static final String ERROR_STYLE_CLASS = "error";

private static final Logger logger = LogsCenter.getLogger(ResultDisplay.class);
private static final String FXML = "ResultDisplay.fxml";

Expand All @@ -35,7 +38,35 @@ public ResultDisplay() {
@Subscribe
private void handleNewResultAvailableEvent(NewResultAvailableEvent event) {
logger.info(LogsCenter.getEventHandlingLogMessage(event));
Platform.runLater(() -> displayed.setValue(event.message));
Platform.runLater(() -> {
displayed.setValue(event.message);

if (event.isSuccessful) {
Copy link
Contributor

Choose a reason for hiding this comment

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

can we ternary this? :P

Copy link
Member Author

@yamgent yamgent Jan 27, 2018

Choose a reason for hiding this comment

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

Ternary operators are only meant for assigning values though.

setStyleToIndicateCommandSuccess();
} else {
setStyleToIndicateCommandFailure();
}
});
}

/**
* Sets the {@code ResultDisplay} style to use the default style.
*/
private void setStyleToIndicateCommandSuccess() {
resultDisplay.getStyleClass().remove(ERROR_STYLE_CLASS);
}

/**
* Sets the {@code ResultDisplay} style to indicate a failed command.
*/
private void setStyleToIndicateCommandFailure() {
ObservableList<String> styleClass = resultDisplay.getStyleClass();

if (styleClass.contains(ERROR_STYLE_CLASS)) {
return;
}

styleClass.add(ERROR_STYLE_CLASS);
}

}
9 changes: 9 additions & 0 deletions src/test/java/guitests/guihandles/ResultDisplayHandle.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package guitests.guihandles;

import java.util.List;

import javafx.scene.control.TextArea;

/**
Expand All @@ -19,4 +21,11 @@ public ResultDisplayHandle(TextArea resultDisplayNode) {
public String getText() {
return getRootNode().getText();
}

/**
* Returns the list of style classes present in the result display.
*/
public List<String> getStyleClass() {
return getRootNode().getStyleClass();
}
}
20 changes: 20 additions & 0 deletions src/test/java/seedu/address/ui/CommandBoxTest.java
Original file line number Diff line number Diff line change
@@ -1,25 +1,33 @@
package seedu.address.ui;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;

import java.util.ArrayList;

import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;

import guitests.guihandles.CommandBoxHandle;
import javafx.scene.input.KeyCode;
import seedu.address.commons.events.ui.NewResultAvailableEvent;
import seedu.address.logic.Logic;
import seedu.address.logic.LogicManager;
import seedu.address.logic.commands.ListCommand;
import seedu.address.model.Model;
import seedu.address.model.ModelManager;
import seedu.address.ui.testutil.EventsCollectorRule;

public class CommandBoxTest extends GuiUnitTest {

private static final String COMMAND_THAT_SUCCEEDS = ListCommand.COMMAND_WORD;
private static final String COMMAND_THAT_FAILS = "invalid command";

@Rule
public final EventsCollectorRule eventsCollectorRule = new EventsCollectorRule();

private ArrayList<String> defaultStyleOfCommandBox;
private ArrayList<String> errorStyleOfCommandBox;

Expand Down Expand Up @@ -127,22 +135,34 @@ public void handleKeyPress_startingWithDown() {

/**
* Runs a command that fails, then verifies that <br>
* - {@code NewResultAvailableEvent} is posted
* - the text remains <br>
* - the command box's style is the same as {@code errorStyleOfCommandBox}.
*/
private void assertBehaviorForFailedCommand() {
commandBoxHandle.run(COMMAND_THAT_FAILS);

assertFalse(((NewResultAvailableEvent) eventsCollectorRule.eventsCollector.getMostRecent()).isSuccessful);
assertTrue(eventsCollectorRule.eventsCollector.getSize() == 1);
eventsCollectorRule.eventsCollector.reset();

assertEquals(COMMAND_THAT_FAILS, commandBoxHandle.getInput());
assertEquals(errorStyleOfCommandBox, commandBoxHandle.getStyleClass());
}

/**
* Runs a command that succeeds, then verifies that <br>
* - {@code NewResultAvailableEvent} is posted
* - the text is cleared <br>
* - the command box's style is the same as {@code defaultStyleOfCommandBox}.
*/
private void assertBehaviorForSuccessfulCommand() {
commandBoxHandle.run(COMMAND_THAT_SUCCEEDS);

assertTrue(((NewResultAvailableEvent) eventsCollectorRule.eventsCollector.getMostRecent()).isSuccessful);
assertTrue(eventsCollectorRule.eventsCollector.getSize() == 1);
eventsCollectorRule.eventsCollector.reset();

assertEquals("", commandBoxHandle.getInput());
assertEquals(defaultStyleOfCommandBox, commandBoxHandle.getStyleClass());
}
Expand Down
39 changes: 35 additions & 4 deletions src/test/java/seedu/address/ui/ResultDisplayTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
import static org.junit.Assert.assertEquals;
import static seedu.address.testutil.EventsUtil.postNow;

import java.util.ArrayList;
import java.util.List;

import org.junit.Before;
import org.junit.Test;

Expand All @@ -11,7 +14,14 @@

public class ResultDisplayTest extends GuiUnitTest {

private static final NewResultAvailableEvent NEW_RESULT_EVENT_STUB = new NewResultAvailableEvent("Stub");
private static final NewResultAvailableEvent NEW_RESULT_SUCCESS_EVENT_STUB =
new NewResultAvailableEvent("success", true);

private static final NewResultAvailableEvent NEW_RESULT_FAILURE_EVENT_STUB =
new NewResultAvailableEvent("failure", false);

private List<String> defaultStyleOfResultDisplay;
private List<String> errorStyleOfResultDisplay;

private ResultDisplayHandle resultDisplayHandle;

Expand All @@ -22,17 +32,38 @@ public void setUp() {

resultDisplayHandle = new ResultDisplayHandle(getChildNode(resultDisplay.getRoot(),
ResultDisplayHandle.RESULT_DISPLAY_ID));

defaultStyleOfResultDisplay = new ArrayList<>(resultDisplayHandle.getStyleClass());

errorStyleOfResultDisplay = new ArrayList<>(defaultStyleOfResultDisplay);
errorStyleOfResultDisplay.add(ResultDisplay.ERROR_STYLE_CLASS);
}

@Test
public void display() {
// default result text
guiRobot.pauseForHuman();
assertEquals("", resultDisplayHandle.getText());
assertEquals(defaultStyleOfResultDisplay, resultDisplayHandle.getStyleClass());

// new result received
postNow(NEW_RESULT_EVENT_STUB);
// receiving new results
assertResultDisplay(NEW_RESULT_SUCCESS_EVENT_STUB);
assertResultDisplay(NEW_RESULT_FAILURE_EVENT_STUB);
}

/**
* Posts the {@code event} to the {@code EventsCenter}, then verifies that <br>
* - the text on the result display matches the {@code event}'s message <br>
* - the result display's style is the same as {@code defaultStyleOfResultDisplay} if event is successful,
* {@code errorStyleOfResultDisplay} otherwise.
*/
private void assertResultDisplay(NewResultAvailableEvent event) {
postNow(event);
guiRobot.pauseForHuman();
assertEquals(NEW_RESULT_EVENT_STUB.message, resultDisplayHandle.getText());

List<String> expectedStyleClass = event.isSuccessful ? defaultStyleOfResultDisplay : errorStyleOfResultDisplay;

assertEquals(event.message, resultDisplayHandle.getText());
assertEquals(expectedStyleClass, resultDisplayHandle.getStyleClass());
}
}
4 changes: 2 additions & 2 deletions src/test/java/systemtests/AddCommandSystemTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ private void assertCommandSuccess(String command, Model expectedModel, String ex
executeCommand(command);
assertApplicationDisplaysExpected("", expectedResultMessage, expectedModel);
assertSelectedCardUnchanged();
assertCommandBoxShowsDefaultStyle();
assertCommandBoxAndResultDisplayShowsDefaultStyle();
assertStatusBarUnchangedExceptSyncStatus();
}

Expand All @@ -250,7 +250,7 @@ private void assertCommandFailure(String command, String expectedResultMessage)
executeCommand(command);
assertApplicationDisplaysExpected(command, expectedResultMessage, expectedModel);
assertSelectedCardUnchanged();
assertCommandBoxShowsErrorStyle();
assertCommandBoxAndResultDisplayShowsErrorStyle();
assertStatusBarUnchanged();
}
}
19 changes: 15 additions & 4 deletions src/test/java/systemtests/AddressBookSystemTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import seedu.address.testutil.TypicalPersons;
import seedu.address.ui.BrowserPanel;
import seedu.address.ui.CommandBox;
import seedu.address.ui.ResultDisplay;

/**
* A system test class for AddressBook, which provides access to handles of GUI components and helper methods
Expand All @@ -56,6 +57,9 @@ public abstract class AddressBookSystemTest {
private static final List<String> COMMAND_BOX_ERROR_STYLE =
Arrays.asList("text-input", "text-field", CommandBox.ERROR_STYLE_CLASS);

private List<String> defaultStyleOfResultDisplay;
private List<String> errorStyleOfResultDisplay;

private MainWindowHandle mainWindowHandle;
private TestApp testApp;
private SystemTestSetupHelper setupHelper;
Expand All @@ -71,6 +75,11 @@ public void setUp() {
testApp = setupHelper.setupApplication(this::getInitialData, getDataFileLocation());
mainWindowHandle = setupHelper.setupMainWindowHandle();

defaultStyleOfResultDisplay = mainWindowHandle.getResultDisplay().getStyleClass();

errorStyleOfResultDisplay = mainWindowHandle.getResultDisplay().getStyleClass();
errorStyleOfResultDisplay.add(ResultDisplay.ERROR_STYLE_CLASS);

waitUntilBrowserLoaded(getBrowserPanel());
assertApplicationStartingStateIsCorrect();
}
Expand Down Expand Up @@ -236,17 +245,19 @@ protected void assertSelectedCardUnchanged() {
}

/**
* Asserts that the command box's shows the default style.
* Asserts that the command box and result display shows the default style.
*/
protected void assertCommandBoxShowsDefaultStyle() {
protected void assertCommandBoxAndResultDisplayShowsDefaultStyle() {
assertEquals(COMMAND_BOX_DEFAULT_STYLE, getCommandBox().getStyleClass());
assertEquals(defaultStyleOfResultDisplay, getResultDisplay().getStyleClass());
}

/**
* Asserts that the command box's shows the error style.
* Asserts that the command box and result display shows the error style.
*/
protected void assertCommandBoxShowsErrorStyle() {
protected void assertCommandBoxAndResultDisplayShowsErrorStyle() {
assertEquals(COMMAND_BOX_ERROR_STYLE, getCommandBox().getStyleClass());
assertEquals(errorStyleOfResultDisplay, getResultDisplay().getStyleClass());
}

/**
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/systemtests/ClearCommandSystemTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ private void assertCommandSuccess(String command) {
private void assertCommandSuccess(String command, String expectedResultMessage, Model expectedModel) {
executeCommand(command);
assertApplicationDisplaysExpected("", expectedResultMessage, expectedModel);
assertCommandBoxShowsDefaultStyle();
assertCommandBoxAndResultDisplayShowsDefaultStyle();
assertStatusBarUnchangedExceptSyncStatus();
}

Expand All @@ -95,7 +95,7 @@ private void assertCommandFailure(String command, String expectedResultMessage)
executeCommand(command);
assertApplicationDisplaysExpected(command, expectedResultMessage, expectedModel);
assertSelectedCardUnchanged();
assertCommandBoxShowsErrorStyle();
assertCommandBoxAndResultDisplayShowsErrorStyle();
assertStatusBarUnchanged();
}
}
4 changes: 2 additions & 2 deletions src/test/java/systemtests/DeleteCommandSystemTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ private void assertCommandSuccess(String command, Model expectedModel, String ex
assertSelectedCardUnchanged();
}

assertCommandBoxShowsDefaultStyle();
assertCommandBoxAndResultDisplayShowsDefaultStyle();
assertStatusBarUnchangedExceptSyncStatus();
}

Expand All @@ -192,7 +192,7 @@ private void assertCommandFailure(String command, String expectedResultMessage)
executeCommand(command);
assertApplicationDisplaysExpected(command, expectedResultMessage, expectedModel);
assertSelectedCardUnchanged();
assertCommandBoxShowsErrorStyle();
assertCommandBoxAndResultDisplayShowsErrorStyle();
assertStatusBarUnchanged();
}
}
4 changes: 2 additions & 2 deletions src/test/java/systemtests/EditCommandSystemTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ private void assertCommandSuccess(String command, Model expectedModel, String ex
executeCommand(command);
expectedModel.updateFilteredPersonList(PREDICATE_SHOW_ALL_PERSONS);
assertApplicationDisplaysExpected("", expectedResultMessage, expectedModel);
assertCommandBoxShowsDefaultStyle();
assertCommandBoxAndResultDisplayShowsDefaultStyle();
if (expectedSelectedCardIndex != null) {
assertSelectedCardChanged(expectedSelectedCardIndex);
} else {
Expand All @@ -301,7 +301,7 @@ private void assertCommandFailure(String command, String expectedResultMessage)
executeCommand(command);
assertApplicationDisplaysExpected(command, expectedResultMessage, expectedModel);
assertSelectedCardUnchanged();
assertCommandBoxShowsErrorStyle();
assertCommandBoxAndResultDisplayShowsErrorStyle();
assertStatusBarUnchanged();
}
}
4 changes: 2 additions & 2 deletions src/test/java/systemtests/FindCommandSystemTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ private void assertCommandSuccess(String command, Model expectedModel) {

executeCommand(command);
assertApplicationDisplaysExpected("", expectedResultMessage, expectedModel);
assertCommandBoxShowsDefaultStyle();
assertCommandBoxAndResultDisplayShowsDefaultStyle();
assertStatusBarUnchanged();
}

Expand All @@ -189,7 +189,7 @@ private void assertCommandFailure(String command, String expectedResultMessage)
executeCommand(command);
assertApplicationDisplaysExpected(command, expectedResultMessage, expectedModel);
assertSelectedCardUnchanged();
assertCommandBoxShowsErrorStyle();
assertCommandBoxAndResultDisplayShowsErrorStyle();
assertStatusBarUnchanged();
}
}
2 changes: 1 addition & 1 deletion src/test/java/systemtests/HelpCommandSystemTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public void openHelpWindow() {
// assert that while the help window is open the UI updates correctly for a command execution
executeCommand(SelectCommand.COMMAND_WORD + " " + INDEX_FIRST_PERSON.getOneBased());
assertEquals("", getCommandBox().getInput());
assertCommandBoxShowsDefaultStyle();
assertCommandBoxAndResultDisplayShowsDefaultStyle();
assertNotEquals(HelpCommand.SHOWING_HELP_MESSAGE, getResultDisplay().getText());
assertNotEquals(BrowserPanel.DEFAULT_PAGE, getBrowserPanel().getLoadedUrl());
assertListMatching(getPersonListPanel(), getModel().getFilteredPersonList());
Expand Down
Loading