Skip to content

Commit

Permalink
Remove ability to invoke "Keyboard Help" by pressing "?".
Browse files Browse the repository at this point in the history
This fixes two issues related to keyboard help for DnDv2:

- If multiple DnDv2 blocks are on the same page, pressing global
  shortcut ("?") brings up multiple dialogs
- It's almost impossible to enter "?" into any of the text boxes in the
  block settings in Studio, since it brings up the keyboard help dialog
  instead

Not using "?" as a global shortcut for "Keyboard Help" also means that
DnDv2 will not conflict with other XBlocks and/or runtimes that might be
using the same shortcut to, e.g., bring up help dialogs.
  • Loading branch information
itsjeyd committed Jan 18, 2016
1 parent 47d67d9 commit 6c7cd63
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 66 deletions.
27 changes: 0 additions & 27 deletions drag_and_drop_v2/public/js/drag_and_drop.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ function DragAndDropBlock(runtime, element, configuration) {
var SPC = 32;
var TAB = 9;
var M = 77;
var QUESTION_MARK = 63;

var placementMode = false;
var $selectedItem;
Expand Down Expand Up @@ -47,12 +46,6 @@ function DragAndDropBlock(runtime, element, configuration) {
initDroppable();

$(document).on('keydown mousedown touchstart', closePopup);
if (isFirstExercise()) { // Set up handler for "?" key only once,
// even if unit contains multiple DnDv2 exercises
$(document).on('keypress', function(evt) {
runOnKey(evt, QUESTION_MARK, showKeyboardHelp);
});
}
$element.on('click', '.keyboard-help-button', showKeyboardHelp);
$element.on('keydown', '.keyboard-help-button', function(evt) {
runOnKey(evt, RET, showKeyboardHelp);
Expand All @@ -70,26 +63,6 @@ function DragAndDropBlock(runtime, element, configuration) {
});
};

var isFirstExercise = function() {
var klass = $element.attr('class');
var $block;
var siblingSelector;

if (klass.startsWith('xblock ')) { // We are in the LMS
$block = $element.parent();
siblingSelector = '.vert[data-id*="drag-and-drop-v2"]';
} else if (klass.startsWith('xblock-v1 ')) { // We are in the workbench
$block = $element;
siblingSelector = '.xblock-v1[data-block-type="drag-and-drop-v2"]';
}

var $previousBlocks = $block.prevAll(siblingSelector);
if ($previousBlocks.length === 0) {
return true;
}
return false;
};

var runOnKey = function(evt, key, handler) {
if (evt.which === key) {
handler(evt);
Expand Down
43 changes: 4 additions & 39 deletions tests/integration/test_interaction.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,12 @@ def _get_input_div_by_value(self, item_value):
element = self._get_item_by_value(item_value)
return element.find_element_by_class_name('numerical-input')

def _get_dialog_components(self, dialog):
def _get_dialog_components(self, dialog): # pylint: disable=no-self-use
dialog_modal_overlay = dialog.find_element_by_css_selector('.modal-window-overlay')
dialog_modal = dialog.find_element_by_css_selector('.modal-window')
return dialog_modal_overlay, dialog_modal

def _get_dialog_dismiss_button(self, dialog_modal):
def _get_dialog_dismiss_button(self, dialog_modal): # pylint: disable=no-self-use
return dialog_modal.find_element_by_css_selector('.modal-dismiss-button')

def _get_zone_position(self, zone_id):
Expand Down Expand Up @@ -273,7 +273,7 @@ def get_locations():
self.assertDictEqual(locations_after_reset[item_key], initial_locations[item_key])
self.assert_reverted_item(item_key)

def interact_with_keyboard_help(self, scroll_down=250, use_keyboard=False, multiple_blocks=False):
def interact_with_keyboard_help(self, scroll_down=250, use_keyboard=False):
keyboard_help_button = self._get_keyboard_help_button()
keyboard_help_dialog = self._get_keyboard_help_dialog()
dialog_modal_overlay, dialog_modal = self._get_dialog_components(keyboard_help_dialog)
Expand All @@ -298,18 +298,6 @@ def interact_with_keyboard_help(self, scroll_down=250, use_keyboard=False, multi
self.assertFalse(dialog_modal_overlay.is_displayed())
self.assertFalse(dialog_modal.is_displayed())

if use_keyboard and not multiple_blocks: # Try again with "?" key
# (behavior for multiple blocks has dedicated test below)
self._page.send_keys("?")

self.assertTrue(dialog_modal_overlay.is_displayed())
self.assertTrue(dialog_modal.is_displayed())

self._page.send_keys(Keys.ESCAPE)

self.assertFalse(dialog_modal_overlay.is_displayed())
self.assertFalse(dialog_modal.is_displayed())


class BasicInteractionTest(InteractionTestBase):
"""
Expand Down Expand Up @@ -505,27 +493,4 @@ def test_keyboard_help(self):
self._switch_to_block(1)
# Test mouse and keyboard interaction
self.interact_with_keyboard_help(scroll_down=900)
self.interact_with_keyboard_help(scroll_down=0, use_keyboard=True, multiple_blocks=True)

# When pressing "?" on a page with multiple DnDv2 exercises,
# only a single "Keyboard Help" dialog should be shown:
first_keyboard_help_dialog, second_keyboard_help_dialog = self._get_keyboard_help_dialogs()
first_dialog_modal_overlay, first_dialog_modal = self._get_dialog_components(first_keyboard_help_dialog)
first_dialog_dismiss_button = self._get_dialog_dismiss_button(first_dialog_modal)
second_dialog_modal_overlay, second_dialog_modal = self._get_dialog_components(second_keyboard_help_dialog)

self._switch_to_block(0)
self._page.send_keys("?")
self.assertTrue(first_dialog_modal_overlay.is_displayed())
self.assertTrue(first_dialog_modal.is_displayed())
self.assertFalse(second_dialog_modal_overlay.is_displayed())
self.assertFalse(second_dialog_modal.is_displayed())
first_dialog_dismiss_button.click()

self._switch_to_block(1)
self._page.send_keys("?")
self.assertTrue(first_dialog_modal_overlay.is_displayed())
self.assertTrue(first_dialog_modal.is_displayed())
self.assertFalse(second_dialog_modal_overlay.is_displayed())
self.assertFalse(second_dialog_modal.is_displayed())
first_dialog_dismiss_button.click()
self.interact_with_keyboard_help(scroll_down=0, use_keyboard=True)

0 comments on commit 6c7cd63

Please sign in to comment.