Skip to content

Commit

Permalink
[1432] Error message: invalid keyboard shortcuts
Browse files Browse the repository at this point in the history
  • Loading branch information
@m133225
m133225 committed Apr 6, 2016
2 parents 0803563 + e4cafeb commit 850a1bb
Show file tree
Hide file tree
Showing 9 changed files with 59 additions and 367 deletions.
18 changes: 0 additions & 18 deletions config/findbugs/excludeFilter.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,24 +59,6 @@
<Bug pattern="ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD" />
</Match>

<Match>
<Class name="ui.components.KeyboardShortcuts" />
<Bug pattern="MS_CANNOT_BE_FINAL" />
<Or>
<Field name="closeIssue" />
<Field name="reopenIssue" />
<Field name="downIssue" />
<Field name="leftPanel" />
<Field name="markAsRead" />
<Field name="markAsUnread" />
<Field name="rightPanel" />
<Field name="scrollDown" />
<Field name="scrollToBottom" />
<Field name="scrollToTop" />
<Field name="scrollUp" />
<Field name="upIssue" />
</Or>
</Match>
<Match>
<Class name="tests.PreferencesTests" />
<Bug pattern="RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT" />
Expand Down
9 changes: 0 additions & 9 deletions src/main/java/prefs/Preferences.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -293,13 +293,4 @@ public void setMarkedReadAt(String repoId, int issue, LocalDateTime timeReadAt)
public Optional<LocalDateTime> getMarkedReadAt(String repoId, int issue) {
return sessionConfig.getMarkedReadAt(repoId, issue);
}

public Map<String, String> getKeyboardShortcuts() {
return sessionConfig.getKeyboardShortcuts();
}

public void setKeyboardShortcuts(Map<String, String> keyboardShortcuts) {
sessionConfig.setKeyboardShortcuts(keyboardShortcuts);
save();
}
}
1 change: 0 additions & 1 deletion src/main/java/ui/UI.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,6 @@ private void initPreApplicationState() {

TestController.setUI(this, getParameters());
prefs = TestController.loadApplicationPreferences();
KeyboardShortcuts.loadKeyboardShortcuts(prefs);

eventBus = new EventBus();
if (TestController.isTestMode()) {
Expand Down
181 changes: 25 additions & 156 deletions src/main/java/ui/components/KeyboardShortcuts.java
View file
Original file line number Diff line number Diff line change
@@ -1,62 +1,46 @@
package ui.components;

import javafx.application.Platform;
import javafx.scene.input.KeyCode;
import javafx.scene.input.KeyCodeCombination;
import javafx.scene.input.KeyCombination;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import prefs.Preferences;
import util.DialogMessage;

import java.util.HashMap;
import java.util.Collections;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;

/**
* a central place to specify keyboard shortcuts
* <p>
* Classes that currently have keyboard shortcut code:
* ui.components.NavigableListView
* ui.issuepanel.PanelControl
* ui.listpanel.ListPanel
* ui.MenuControl
* <p>
* Utility Class:
* util.KeyPress
* This class handles the keyboard shortcuts used by the different UI components
*/
public final class KeyboardShortcuts {
public static final KeyCodeCombination MARK_AS_READ =
new KeyCodeCombination(KeyCode.E);
public static final KeyCodeCombination MARK_AS_UNREAD =
new KeyCodeCombination(KeyCode.U);

private static final Logger logger = LogManager.getLogger(KeyboardShortcuts.class.getName());

private static Map<String, String> keyboardShortcuts = null;
private static Set<KeyCodeCombination> assignedKeys = null;

// customizable keyboard shortcuts
// ui.listpanel.ListPanel
public static KeyCodeCombination markAsRead;
public static KeyCodeCombination markAsUnread;

public static KeyCodeCombination closeIssue;
public static KeyCodeCombination reopenIssue;
public static final KeyCodeCombination CLOSE_ISSUE =
new KeyCodeCombination(KeyCode.X);
public static final KeyCodeCombination REOPEN_ISSUE =
new KeyCodeCombination(KeyCode.O);

public static KeyCodeCombination scrollToTop;
public static KeyCodeCombination scrollToBottom;
public static KeyCodeCombination scrollUp;
public static KeyCodeCombination scrollDown;
public static final KeyCodeCombination SCROLL_TO_TOP =
new KeyCodeCombination(KeyCode.I);
public static final KeyCodeCombination SCROLL_TO_BOTTOM =
new KeyCodeCombination(KeyCode.N);
public static final KeyCodeCombination SCROLL_UP =
new KeyCodeCombination(KeyCode.J);
public static final KeyCodeCombination SCROLL_DOWN =
new KeyCodeCombination(KeyCode.K);

//ui.issuepanel.PanelControl
public static KeyCodeCombination leftPanel;
public static KeyCodeCombination rightPanel;
public static final KeyCodeCombination LEFT_PANEL =
new KeyCodeCombination(KeyCode.D);
public static final KeyCodeCombination RIGHT_PANEL =
new KeyCodeCombination(KeyCode.F);

// ui.components.NavigableListView
static KeyCodeCombination upIssue;
static KeyCodeCombination downIssue;
public static final KeyCodeCombination UP_ISSUE =
new KeyCodeCombination(KeyCode.T);
public static final KeyCodeCombination DOWN_ISSUE =
new KeyCodeCombination(KeyCode.V);

// non-customizable keyboard shortcuts
// ui.listpanel.ListPanel
public static final KeyCodeCombination JUMP_TO_FIRST_ISSUE =
new KeyCodeCombination(KeyCode.ENTER, KeyCombination.SHORTCUT_DOWN);
public static final KeyCodeCombination JUMP_TO_FILTER_BOX =
Expand Down Expand Up @@ -114,13 +98,11 @@ public final class KeyboardShortcuts {
public static final KeyCodeCombination NEW_COMMENT =
new KeyCodeCombination(KeyCode.R);

//ui.RepositorySelector
public static final KeyCodeCombination REMOVE_FOCUS =
new KeyCodeCombination(KeyCode.ESCAPE);
public static final KeyCodeCombination SHOW_REPO_PICKER =
new KeyCodeCombination(KeyCode.R, KeyCombination.CONTROL_DOWN, KeyCombination.SHIFT_DOWN);

// ui.MenuControl
public static final KeyCodeCombination NEW_ISSUE =
new KeyCodeCombination(KeyCode.I, KeyCombination.SHORTCUT_DOWN, KeyCombination.SHIFT_DOWN);
public static final KeyCodeCombination NEW_LABEL =
Expand Down Expand Up @@ -153,117 +135,4 @@ private static Map<Integer, KeyCodeCombination> populateJumpToNthIssueMap() {
result.put(9, new KeyCodeCombination(KeyCode.DIGIT9, KeyCombination.SHORTCUT_DOWN));
return Collections.unmodifiableMap(result);
}

public static Map<String, String> getDefaultKeyboardShortcuts() {
Map<String, String> defaultKeyboardShortcuts = new HashMap<>();
defaultKeyboardShortcuts.put("MARK_AS_READ", "E");
defaultKeyboardShortcuts.put("MARK_AS_UNREAD", "U");
defaultKeyboardShortcuts.put("CLOSE_ISSUE", "X");
defaultKeyboardShortcuts.put("REOPEN_ISSUE", "O");
defaultKeyboardShortcuts.put("SCROLL_TO_TOP", "I");
defaultKeyboardShortcuts.put("SCROLL_TO_BOTTOM", "N");
defaultKeyboardShortcuts.put("SCROLL_UP", "J");
defaultKeyboardShortcuts.put("SCROLL_DOWN", "K");
defaultKeyboardShortcuts.put("LEFT_PANEL", "D");
defaultKeyboardShortcuts.put("RIGHT_PANEL", "F");
defaultKeyboardShortcuts.put("UP_ISSUE", "T");
defaultKeyboardShortcuts.put("DOWN_ISSUE", "V");
return defaultKeyboardShortcuts;
}

private static void addNonCustomizableShortcutKeys() {
assignedKeys.add(REFRESH);
assignedKeys.add(SHOW_DOCS);
assignedKeys.add(GOTO_MODIFIER);
assignedKeys.add(NEW_COMMENT);
}

private static void getKeyboardShortcutsFromHashMap() {
markAsRead = getKeyCodeCombination("MARK_AS_READ");
markAsUnread = getKeyCodeCombination("MARK_AS_UNREAD");
closeIssue = getKeyCodeCombination("CLOSE_ISSUE");
reopenIssue = getKeyCodeCombination("REOPEN_ISSUE");
scrollToTop = getKeyCodeCombination("SCROLL_TO_TOP");
scrollToBottom = getKeyCodeCombination("SCROLL_TO_BOTTOM");
scrollUp = getKeyCodeCombination("SCROLL_UP");
scrollDown = getKeyCodeCombination("SCROLL_DOWN");
leftPanel = getKeyCodeCombination("LEFT_PANEL");
rightPanel = getKeyCodeCombination("RIGHT_PANEL");
upIssue = getKeyCodeCombination("UP_ISSUE");
downIssue = getKeyCodeCombination("DOWN_ISSUE");
}

public static void loadKeyboardShortcuts(Preferences prefs) {
assignedKeys = new HashSet<>();
if (prefs.getKeyboardShortcuts().size() == 0) {
logger.info("No user specified keyboard shortcuts found, using defaults. ");
prefs.setKeyboardShortcuts(getDefaultKeyboardShortcuts());
}
if (prefs.getKeyboardShortcuts().size() != getDefaultKeyboardShortcuts().size()) {
logger.warn("Invalid number of user specified keyboard shortcuts detected. ");
if (DialogMessage.showYesNoWarningDialog(
"Warning",
"Invalid number of shortcut keys specified",
"Do you want to reset the shortcut keys to their defaults or quit?",
"Reset to default",
"Quit")) {
keyboardShortcuts = getDefaultKeyboardShortcuts();
} else {
Platform.exit();
System.exit(0);
}
} else {
logger.info("Loading user specified keyboard shortcuts. ");
keyboardShortcuts = prefs.getKeyboardShortcuts();
}
addNonCustomizableShortcutKeys();
getKeyboardShortcutsFromHashMap();
prefs.setKeyboardShortcuts(keyboardShortcuts);
}

private static KeyCodeCombination getKeyCodeCombination(String keyboardShortcut) {
KeyCodeCombination keyCodeCombi =
new KeyCodeCombination(KeyCode.getKeyCode(getDefaultKeyboardShortcuts().get(keyboardShortcut)));
if (keyboardShortcuts.containsKey(keyboardShortcut)) {
KeyCode keyCode = KeyCode.getKeyCode(keyboardShortcuts.get(keyboardShortcut).toUpperCase());
if (keyCode != null && !assignedKeys.contains(new KeyCodeCombination(keyCode))) {
keyCodeCombi = new KeyCodeCombination(keyCode);
} else {
logger.warn("Invalid key specified for "
+ keyboardShortcut
+ " or it has already been used for some other shortcut. ");
if (DialogMessage.showYesNoWarningDialog(
"Warning",
"Invalid key specified for " + keyboardShortcut +
" or it has already been used for some other shortcut. ",
"Do you want to use the default key <" +
getDefaultKeyboardShortcuts().get(keyboardShortcut) + "> or quit?",
"Use default key",
"Quit")) {
keyboardShortcuts.put(keyboardShortcut, getDefaultKeyboardShortcuts().get(keyboardShortcut));
} else {
Platform.exit();
System.exit(0);
}
}
} else {
logger.warn("Could not find user defined keyboard shortcut for " + keyboardShortcut);
if (DialogMessage.showYesNoWarningDialog(
"Warning",
"Could not find user defined keyboard shortcut for " + keyboardShortcut,
"Do you want to use the default key <" +
getDefaultKeyboardShortcuts().get(keyboardShortcut) + "> or quit?",
"Use default key",
"Quit")) {
keyboardShortcuts.put(keyboardShortcut, getDefaultKeyboardShortcuts().get(keyboardShortcut));
} else {
Platform.exit();
System.exit(0);
}
}
logger.info("Assigning <" + keyCodeCombi + "> to " + keyboardShortcut);
assignedKeys.add(keyCodeCombi);
return keyCodeCombi;
}

}
4 changes: 2 additions & 2 deletions src/main/java/ui/components/NavigableListView.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,9 @@ private void setupKeyEvents() {
}
}
if (e.getCode() == KeyCode.UP || e.getCode() == KeyCode.DOWN ||
KeyboardShortcuts.upIssue.match(e) || KeyboardShortcuts.downIssue.match(e)) {
KeyboardShortcuts.UP_ISSUE.match(e) || KeyboardShortcuts.DOWN_ISSUE.match(e)) {
e.consume();
handleUpDownKeys(e.getCode() == KeyCode.DOWN || KeyboardShortcuts.downIssue.match(e));
handleUpDownKeys(e.getCode() == KeyCode.DOWN || KeyboardShortcuts.DOWN_ISSUE.match(e));
assert selectedIndex.isPresent() : "handleUpDownKeys doesn't set selectedIndex!";
if (!e.isShiftDown()) {
logger.info("Enter key selection on item index " + selectedIndex.get());
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/ui/issuepanel/PanelControl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -257,8 +257,8 @@ public double getPanelWidth() {

private void setupKeyEvents() {
addEventHandler(KeyEvent.KEY_PRESSED, event -> {
if (KeyboardShortcuts.rightPanel.match(event) || KeyboardShortcuts.leftPanel.match(event)) {
handleKeys(KeyboardShortcuts.rightPanel.match(event));
if (KeyboardShortcuts.RIGHT_PANEL.match(event) || KeyboardShortcuts.LEFT_PANEL.match(event)) {
handleKeys(KeyboardShortcuts.RIGHT_PANEL.match(event));
assert currentlySelectedPanel.isPresent() : "handleKeys doesn't set selectedIndex!";
}
});
Expand Down
22 changes: 11 additions & 11 deletions src/main/java/ui/listpanel/ListPanel.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,18 +250,18 @@ private void setupListView() {

private void setupKeyboardShortcuts() {
addEventHandler(KeyEvent.KEY_PRESSED, event -> {
// Temporary fix for now since markAsRead and Show Related Issue/PR have same keys.
// Will only work if the key for markAsRead is not the default key E.
if (KeyboardShortcuts.markAsRead.match(event) && !SHOW_RELATED_ISSUE_OR_PR.match(event)) {
// Temporary fix for now since MARK_AS_READ and Show Related Issue/PR have same keys.
// Will only work if the key for MARK_AS_READ is not the default key E.
if (KeyboardShortcuts.MARK_AS_READ.match(event) && !SHOW_RELATED_ISSUE_OR_PR.match(event)) {
markAsRead();
}
if (KeyboardShortcuts.markAsUnread.match(event)) {
if (KeyboardShortcuts.MARK_AS_UNREAD.match(event)) {
markAsUnread();
}
if (KeyboardShortcuts.closeIssue.match(event)) {
if (KeyboardShortcuts.CLOSE_ISSUE.match(event)) {
closeIssue();
}
if (KeyboardShortcuts.reopenIssue.match(event)) {
if (KeyboardShortcuts.REOPEN_ISSUE.match(event)) {
reopenIssue();
}
if (SHOW_DOCS.match(event)) {
Expand Down Expand Up @@ -293,14 +293,14 @@ private void setupKeyboardShortcuts() {
ui.getBrowserComponent().showContributors();
event.consume();
}
if (KeyboardShortcuts.scrollToTop.match(event)) {
if (KeyboardShortcuts.SCROLL_TO_TOP.match(event)) {
ui.getBrowserComponent().scrollToTop();
}
if (KeyboardShortcuts.scrollToBottom.match(event)) {
if (KeyboardShortcuts.SCROLL_TO_BOTTOM.match(event)) {
ui.getBrowserComponent().scrollToBottom();
}
if (KeyboardShortcuts.scrollUp.match(event) || KeyboardShortcuts.scrollDown.match(event)) {
ui.getBrowserComponent().scrollPage(KeyboardShortcuts.scrollDown.match(event));
if (KeyboardShortcuts.SCROLL_UP.match(event) || KeyboardShortcuts.SCROLL_DOWN.match(event)) {
ui.getBrowserComponent().scrollPage(KeyboardShortcuts.SCROLL_DOWN.match(event));
}
if (GOTO_MODIFIER.match(event)) {
KeyPress.setLastKeyPressedCodeAndTime(event.getCode());
Expand Down Expand Up @@ -362,7 +362,7 @@ private void setupKeyboardShortcuts() {
if (KeyPress.isValidKeyCombination(GOTO_MODIFIER.getCode(), event.getCode())) {
showRelatedIssueOrPR();
// only for default. can remove if default key for MARK_AS_READ_CHANGES
} else if (KeyboardShortcuts.markAsRead.match(event)) {
} else if (KeyboardShortcuts.MARK_AS_READ.match(event)) {
markAsRead();
}
}
Expand Down
Loading

0 comments on commit 850a1bb

Please sign in to comment.