Skip to content

Commit

Permalink
HelpWindow: modify HelpWindow#show() to use Stage#show()
Browse files Browse the repository at this point in the history
opened by typing help in the command box. For instance, the command box 
does not display the result message of subsequent commands. However, 
this does not occur when opening the help window by pressing F1 or 
pressing help from the help menu. 

This occurs due to a combination of using the default Dispatcher used 
by the Guava EventBus system, Dispatcher#perThreadDispatchQueue(), 
along with calling Stage#showAndWait(). Calling Stage#showAndWait(), 
starts a nested event loop to handle subsequent JavaFX events. However, 
the default Dispatcher used by the EventBus does not allow nested 
callback invocations, and instead queues them to be invoked after the 
current invoked callback, the MainWindow#handleShowHelpEvent() 
completes. As such, EventBus events are queued up in the meantime 
resulting in the subscribers of these events having to wait until the 
nested event loop is terminated (i.e. when the callback completes, 
after the help window is closed.) And so, the UI components will only 
receive these events and therefore update after the help window is 
closed.

However, in the case of pressing F1 or pressing help from the help menu, 
MainWindow#handleHelp() is called immediately without posting the
MainWindow#handleShowHelpEvent(). Since this event never enters the 
EventBus's queue, the nested event loop started by Stage#showAndWait() 
does not block it and subsequent EventBus events get dispatched as 
usual.

Let's replace Stage#showAndWait() with Stage#show(), so 
that there will be no entering of a nested loop and hence no blockage 
of the Guava EventBus system.

Likewise, this could be fixed by using the Dispatcher#immediate() 
dispatcher instead of the default dispatcher used by the Guava EventBus 
system. However, replacing Stage#showAndWait() with Stage#show() is 
more suitable in this case as it is sufficient to show the help window 
instead of starting a nested event loop to handle subsequent JavaFX 
events while waiting for the help window to be closed.

JavaFX documentation for Stage:
https://docs.oracle.com/javase/8/javafx/api/javafx/stage/Stage.html

Code and documentation for the Guava Dispatcher:
https://github.com/google/guava/blob/master/guava/src/com/google/common/eventbus/Dispatcher.java

More information on the Guava EventBus system:
https://github.com/google/guava/wiki/EventBusExplained
  • Loading branch information
vivekscl authored and Zhiyuan-Amos committed Feb 14, 2018
1 parent ad1931d commit 433390f
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 2 deletions.
2 changes: 1 addition & 1 deletion src/main/java/seedu/address/ui/HelpWindow.java
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,6 @@ public HelpWindow() {
*/
public void show() {
logger.fine("Showing help page about the application.");
getRoot().showAndWait();
getRoot().show();
}
}
27 changes: 26 additions & 1 deletion src/test/java/systemtests/HelpCommandSystemTest.java
Original file line number Diff line number Diff line change
@@ -1,16 +1,24 @@
package systemtests;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertTrue;
import static seedu.address.testutil.TypicalIndexes.INDEX_FIRST_PERSON;
import static seedu.address.ui.testutil.GuiTestAssert.assertListMatching;

import org.junit.Test;

import guitests.GuiRobot;
import guitests.guihandles.HelpWindowHandle;
import seedu.address.logic.commands.DeleteCommand;
import seedu.address.logic.commands.HelpCommand;
import seedu.address.logic.commands.SelectCommand;
import seedu.address.ui.BrowserPanel;
import seedu.address.ui.StatusBarFooter;

/**
* TODO: This test is incomplete as it is missing test cases.
* A system test class for the help window, which contains interaction with other UI components.
*/
public class HelpCommandSystemTest extends AddressBookSystemTest {
private static final String ERROR_MESSAGE = "ATTENTION!!!! : On some computers, this test may fail when run on "
Expand Down Expand Up @@ -46,6 +54,23 @@ public void openHelpWindow() {
//use command box
executeCommand(HelpCommand.COMMAND_WORD);
assertHelpWindowOpen();

// open help window and give it focus
executeCommand(HelpCommand.COMMAND_WORD);
getMainWindowHandle().focus();

// 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();
assertNotEquals(HelpCommand.SHOWING_HELP_MESSAGE, getResultDisplay().getText());
assertNotEquals(BrowserPanel.DEFAULT_PAGE, getBrowserPanel().getLoadedUrl());
assertListMatching(getPersonListPanel(), getModel().getFilteredPersonList());

// assert that the status bar too is updated correctly while the help window is open
// note: the select command tested above does not update the status bar
executeCommand(DeleteCommand.COMMAND_WORD + " " + INDEX_FIRST_PERSON.getOneBased());
assertNotEquals(StatusBarFooter.SYNC_STATUS_INITIAL, getStatusBarFooter().getSyncStatus());
}

/**
Expand Down

0 comments on commit 433390f

Please sign in to comment.