Address review comments.

drag_and_drop_v2/public/css/drag_and_drop.css

@@ -116,7 +116,8 @@
 
 /* Focused option */
 .xblock--drag-and-drop .drag-container .item-bank .option:focus,
-.xblock--drag-and-drop .drag-container .item-bank .option:hover {
+.xblock--drag-and-drop .drag-container .item-bank .option:hover,
+.xblock--drag-and-drop .drag-container .item-bank .option[aria-grabbed='true'] {
     outline-width: 2px;
     outline-style: solid;
     outline-offset: -4px;

drag_and_drop_v2/public/js/drag_and_drop.js

@@ -13,7 +13,6 @@ function DragAndDropBlock(runtime, element, configuration) {
     var __vdom = virtualDom.h();  // blank virtual DOM
 
     // Keyboard accessibility
-    var CTRL = 17;
     var ESC = 27;
     var RET = 13;
     var SPC = 32;
@@ -21,7 +20,6 @@ function DragAndDropBlock(runtime, element, configuration) {
     var M = 77;
     var QUESTION_MARK = 63;
 
-    var ctrlDown = false;
     var placementMode = false;
     var $selectedItem;
     var $focusedElement;
@@ -213,16 +211,17 @@ function DragAndDropBlock(runtime, element, configuration) {
         });
     };
 
-    var isCycleKey = function(key) {
-        return !ctrlDown && key === TAB;
+    var isCycleKey = function(evt) {
+        return !evt.ctrlKey && !evt.metaKey && evt.which === TAB;
     };
 
-    var isCancelKey = function(key) {
-        return !ctrlDown && key === ESC;
+    var isCancelKey = function(evt) {
+        return !evt.ctrlKey && !evt.metaKey  && evt.which === ESC;
     };
 
-    var isActionKey = function(key) {
-        if (ctrlDown) {
+    var isActionKey = function(evt) {
+        var key = evt.which;
+        if (evt.ctrlKey || evt.metaKey) {
             return key === M;
         }
         return key === RET || key === SPC;
@@ -286,28 +285,20 @@ function DragAndDropBlock(runtime, element, configuration) {
             var $zone = $(this);
             $zone.on('keydown', function(evt) {
                 if (placementMode) {
-                    var key = evt.which;
-                    if (key === CTRL) {
-                        ctrlDown = true;
-                        return;
-                    }
-                    if (isCycleKey(key)) {
+                    if (isCycleKey(evt)) {
                         focusNextZone(evt, $zone);
-                    } else if (isCancelKey(key)) {
+                    } else if (isCancelKey(evt)) {
                         evt.preventDefault();
                         placementMode = false;
-                    } else if (isActionKey(key)) {
+                        releaseItem($selectedItem);
+                    } else if (isActionKey(evt)) {
                         evt.preventDefault();
                         placementMode = false;
                         placeItem($zone);
+                        releaseItem($selectedItem);
                     }
                 }
             });
-            $zone.on('keyup', function(evt) {
-                if (evt.which === CTRL) {
-                    ctrlDown = false;
-                }
-            });
         });
 
         // Make zone accept items that are dropped using the mouse
@@ -328,23 +319,14 @@ function DragAndDropBlock(runtime, element, configuration) {
 
             // Allow item to be "picked up" using the keyboard
             $item.on('keydown', function(evt) {
-                var key = evt.which;
-                if (key === CTRL) {
-                    ctrlDown = true;
-                    return;
-                }
-                if (isActionKey(key)) {
+                if (isActionKey(evt)) {
                     evt.preventDefault();
                     placementMode = true;
+                    grabItem($item);
                     $selectedItem = $item;
                     $root.find('.target .zone').first().focus();
                 }
             });
-            $item.on('keyup', function(evt) {
-                if (evt.which === CTRL) {
-                    ctrlDown = false;
-                }
-            });
 
             // Make item draggable using the mouse
             try {
@@ -355,18 +337,15 @@ function DragAndDropBlock(runtime, element, configuration) {
                     revert: 'invalid',
                     revertDuration: 150,
                     start: function(evt, ui) {
-                        var item_id = $(this).data('value');
-                        setGrabbedState(item_id, true);
-                        updateDOM();
+                        var $item = $(this);
+                        grabItem($item);
                         publishEvent({
                             event_type: 'xblock.drag-and-drop-v2.item.picked-up',
-                            item_id: item_id
+                            item_id: $item.data('value'),
                         });
                     },
                     stop: function(evt, ui) {
-                        var item_id = $(this).data('value');
-                        setGrabbedState(item_id, false);
-                        updateDOM();
+                        releaseItem($(this));
                     }
                 });
             } catch (e) {
@@ -376,6 +355,18 @@ function DragAndDropBlock(runtime, element, configuration) {
         });
     };
 
+    var grabItem = function($item) {
+        var item_id = $item.data('value');
+        setGrabbedState(item_id, true);
+        updateDOM();
+    };
+
+    var releaseItem = function($item) {
+        var item_id = $item.data('value');
+        setGrabbedState(item_id, false);
+        updateDOM();
+    };
+
     var setGrabbedState = function(item_id, grabbed) {
         for (var i = 0; i < configuration.items.length; i++) {
             if (configuration.items[i].id === item_id) {
@@ -524,10 +515,11 @@ function DragAndDropBlock(runtime, element, configuration) {
             if (item.grabbed !== undefined) {
                 grabbed = item.grabbed;
             }
+            var placed = item_user_state && ('input' in item_user_state || item_user_state.correct_input);
             var itemProperties = {
                 value: item.id,
                 drag_disabled: Boolean(item_user_state || state.finished),
-                class_name: item_user_state && ('input' in item_user_state || item_user_state.correct_input) ? 'fade': undefined,
+                class_name: placed || state.finished ? 'fade' : undefined,
                 xhr_active: (item_user_state && item_user_state.submitting_location),
                 input: input,
                 displayName: item.displayName,

drag_and_drop_v2/public/js/view.js

@@ -176,8 +176,8 @@
                             h('p', gettext('You can complete this exercise using only your keyboard.')),
                             h('ul', [
                                 h('li', gettext('Use "Tab" and "Shift-Tab" to navigate between items and zones.')),
-                                h('li', gettext('Press "Enter" or "Space" on an item to select it for dropping, then navigate to the zone you want to drop it on.')),
-                                h('li', gettext('Press "Enter" or "Space" to drop the item on the current zone.')),
+                                h('li', gettext('Press "Enter", "Space", "Ctrl-m", or "⌘-m" on an item to select it for dropping, then navigate to the zone you want to drop it on.')),
+                                h('li', gettext('Press "Enter", "Space", "Ctrl-m", or "⌘-m" to drop the item on the current zone.')),
                                 h('li', gettext('Press "Escape" if you want to cancel the drop operation (e.g. because you would like to select a different item).')),
                                 h('li', gettext('Press "?" at any time to bring up this dialog.')),
                             ])

tests/integration/test_base.py

@@ -72,6 +72,9 @@ class BaseIntegrationTest(SeleniumBaseTest):
     def _get_keyboard_help_dialog(self):
         return self._page.find_element_by_css_selector(".keyboard-help .keyboard-help-dialog")
 
+    def _get_reset_button(self):
+        return self._page.find_element_by_css_selector('.reset-button')
+
     def _get_feedback(self):
         return self._page.find_element_by_css_selector(".feedback")
 

tests/integration/test_interaction.py

@@ -31,12 +31,19 @@ class ItemDefinition(object):
 
 class InteractionTestBase(object):
     @classmethod
-    def _get_correct_item_for_zone(cls, items_map):
+    def _get_items_with_zone(cls, items_map):
         return {
             item_key: definition for item_key, definition in items_map.items()
             if definition.zone_id is not None
         }
 
+    @classmethod
+    def _get_items_without_zone(cls, items_map):
+        return {
+            item_key: definition for item_key, definition in items_map.items()
+            if definition.zone_id is None
+        }
+
     def setUp(self):
         super(InteractionTestBase, self).setUp()
 
@@ -64,17 +71,19 @@ class InteractionTestBase(object):
         element = self._get_item_by_value(item_value)
         return element.find_element_by_class_name('numerical-input')
 
-    def _send_input(self, item_value, value):
-        element = self._get_item_by_value(item_value)
-        self.wait_until_visible(element)
-        element.find_element_by_class_name('input').send_keys(value)
-        element.find_element_by_class_name('submit-input').click()
+    def _get_zone_position(self, zone_id):
+        return self.browser.execute_script(
+            'return $("div[data-zone=\'{zone_id}\']").prevAll(".zone").length'.format(zone_id=zone_id)
+        )
 
-    def get_feedback_popup(self):
-        return self._page.find_element_by_css_selector(".popup-content")
+    def _focus_item(self, item_position):
+        self.browser.execute_script("$('.option:nth-child({n})').focus()".format(n=item_position+1))
 
-    def get_reset_button(self):
-        return self._page.find_element_by_css_selector('.reset-button')
+    def place_item(self, item_value, zone_id, action_key=None):
+        if action_key is None:
+            self.drag_item_to_zone(item_value, zone_id)
+        else:
+            self.move_item_to_zone(item_value, zone_id, action_key)
 
     def drag_item_to_zone(self, item_value, zone_id):
         element = self._get_item_by_value(item_value)
@@ -86,27 +95,29 @@ class InteractionTestBase(object):
         # Get item position
         item_position = item_value
         # Get zone position
-        zone_position = self.get_zone_position(zone_id)
+        zone_position = self._get_zone_position(zone_id)
 
-        self.focus_item(0)
+        self._focus_item(0)
         focused_item = self._get_item_by_value(0)
         for i in range(item_position):
             focused_item.send_keys(Keys.TAB)
             focused_item = self._get_item_by_value(i+1)
         focused_item.send_keys(action_key)  # Focus is on first *zone* now
+        self.assert_grabbed_item(focused_item)
         focused_zone = self._get_zone_by_id(ZONES_MAP[0])
         for i in range(zone_position):
             focused_zone.send_keys(Keys.TAB)
             focused_zone = self._get_zone_by_id(ZONES_MAP[i+1])
         focused_zone.send_keys(action_key)
 
-    def get_zone_position(self, zone_id):
-        return self.browser.execute_script(
-            'return $("div[data-zone=\'{zone_id}\']").prevAll(".zone").length'.format(zone_id=zone_id)
-        )
+    def send_input(self, item_value, value):
+        element = self._get_item_by_value(item_value)
+        self.wait_until_visible(element)
+        element.find_element_by_class_name('input').send_keys(value)
+        element.find_element_by_class_name('submit-input').click()
 
-    def focus_item(self, item_position):
-        self.browser.execute_script("$('.option:nth-child({n})').focus()".format(n=item_position+1))
+    def assert_grabbed_item(self, item):
+        self.assertEqual(item.get_attribute('aria-grabbed'), 'true')
 
     def assert_placed_item(self, item_value, zone_id):
         item = self._get_placed_item_by_value(item_value)
@@ -125,6 +136,7 @@ class InteractionTestBase(object):
         item = self._get_item_by_value(item_value)
         item_content = item.find_element_by_css_selector('.item-content')
 
+        self.assertEqual(item.get_attribute('class'), 'option ui-draggable')
         self.assertEqual(item.get_attribute('tabindex'), '0')
         self.assertEqual(item.get_attribute('draggable'), 'true')
         self.assertEqual(item.get_attribute('aria-grabbed'), 'false')
@@ -138,43 +150,40 @@ class InteractionTestBase(object):
         else:
             self.fail('Reverted item should not have .sr description.')
 
-    def parameterized_item_positive_feedback_on_good_move(
-        self, items_map, scroll_down=100, use_keyboard=False, action_key=Keys.RETURN
-    ):
+    def assert_decoy_items(self, items_map):
+        decoy_items = self._get_items_without_zone(items_map)
+        for item_key in decoy_items:
+            item = self._get_item_by_value(item_key)
+
+            self.assertEqual(item.get_attribute('class'), 'option fade')
+            self.assertEqual(item.get_attribute('aria-grabbed'), 'false')
+            self.assertEqual(item.get_attribute('data-drag-disabled'), 'true')
+
+    def parameterized_item_positive_feedback_on_good_move(self, items_map, scroll_down=100, action_key=None):
         # Scroll drop zones into view to make sure Selenium can successfully drop items
         self.scroll_down(pixels=scroll_down)
 
-        for definition in self._get_correct_item_for_zone(items_map).values():
+        for definition in self._get_items_with_zone(items_map).values():
             if not definition.input:
-                if use_keyboard:
-                    self.move_item_to_zone(definition.item_id, definition.zone_id, action_key)
-                else:
-                    self.drag_item_to_zone(definition.item_id, definition.zone_id)
-                feedback_popup = self.get_feedback_popup()
-                self.wait_until_html_in(definition.feedback_positive, feedback_popup)
+                self.place_item(definition.item_id, definition.zone_id, action_key)
+                feedback_popup_content = self._get_popup_content()
+                self.wait_until_html_in(definition.feedback_positive, feedback_popup_content)
                 self.assert_placed_item(definition.item_id, definition.zone_id)
 
-    def parameterized_item_positive_feedback_on_good_input(
-        self, items_map, scroll_down=100, use_keyboard=False, action_key=Keys.RETURN
-    ):
+    def parameterized_item_positive_feedback_on_good_input(self, items_map, scroll_down=100, action_key=None):
         self.scroll_down(pixels=scroll_down)
-        feedback_popup = self.get_feedback_popup()
-        for definition in self._get_correct_item_for_zone(items_map).values():
+        feedback_popup_content = self._get_popup_content()
+        for definition in self._get_items_with_zone(items_map).values():
             if definition.input:
-                if use_keyboard:
-                    self.move_item_to_zone(definition.item_id, definition.zone_id, action_key)
-                else:
-                    self.drag_item_to_zone(definition.item_id, definition.zone_id)
-                self._send_input(definition.item_id, definition.input)
+                self.place_item(definition.item_id, definition.zone_id, action_key)
+                self.send_input(definition.item_id, definition.input)
                 input_div = self._get_input_div_by_value(definition.item_id)
                 self.wait_until_has_class('correct', input_div)
-                self.wait_until_html_in(definition.feedback_positive, feedback_popup)
+                self.wait_until_html_in(definition.feedback_positive, feedback_popup_content)
                 self.assert_placed_item(definition.item_id, definition.zone_id)
 
-    def parameterized_item_negative_feedback_on_bad_move(
-        self, items_map, all_zones, scroll_down=100, use_keyboard=False, action_key=Keys.RETURN
-    ):
-        feedback_popup = self.get_feedback_popup()
+    def parameterized_item_negative_feedback_on_bad_move(self, items_map, all_zones, scroll_down=100, action_key=None):
+        feedback_popup_content = self._get_popup_content()
 
         # Scroll drop zones into view to make sure Selenium can successfully drop items
         self.scroll_down(pixels=scroll_down)
@@ -183,40 +192,30 @@ class InteractionTestBase(object):
             for zone in all_zones:
                 if zone == definition.zone_id:
                     continue
-                if use_keyboard:
-                    self.move_item_to_zone(definition.item_id, zone, action_key)
-                else:
-                    self.drag_item_to_zone(definition.item_id, zone)
-                self.wait_until_html_in(definition.feedback_negative, feedback_popup)
+                self.place_item(definition.item_id, zone, action_key)
+                self.wait_until_html_in(definition.feedback_negative, feedback_popup_content)
                 self.assert_reverted_item(definition.item_id)
 
-    def parameterized_item_negative_feedback_on_bad_input(
-        self, items_map, scroll_down=100, use_keyboard=False, action_key=Keys.RETURN
-    ):
-        feedback_popup = self.get_feedback_popup()
+    def parameterized_item_negative_feedback_on_bad_input(self, items_map, scroll_down=100, action_key=None):
+        feedback_popup_content = self._get_popup_content()
 
         # Scroll drop zones into view to make sure Selenium can successfully drop items
         self.scroll_down(pixels=scroll_down)
 
-        for definition in self._get_correct_item_for_zone(items_map).values():
+        for definition in self._get_items_with_zone(items_map).values():
             if definition.input:
-                if use_keyboard:
-                    self.move_item_to_zone(definition.item_id, definition.zone_id, action_key)
-                else:
-                    self.drag_item_to_zone(definition.item_id, definition.zone_id)
-                self._send_input(definition.item_id, '1999999')
+                self.place_item(definition.item_id, definition.zone_id, action_key)
+                self.send_input(definition.item_id, '1999999')
                 input_div = self._get_input_div_by_value(definition.item_id)
                 self.wait_until_has_class('incorrect', input_div)
-                self.wait_until_html_in(definition.feedback_negative, feedback_popup)
+                self.wait_until_html_in(definition.feedback_negative, feedback_popup_content)
                 self.assert_placed_item(definition.item_id, definition.zone_id)
 
-    def parameterized_final_feedback_and_reset(
-        self, items_map, feedback, scroll_down=100, use_keyboard=False, action_key=Keys.RETURN
-    ):
+    def parameterized_final_feedback_and_reset(self, items_map, feedback, scroll_down=100, action_key=None):
         feedback_message = self._get_feedback_message()
         self.assertEqual(self.get_element_html(feedback_message), feedback['intro'])  # precondition check
 
-        items = self._get_correct_item_for_zone(items_map)
+        items = self._get_items_with_zone(items_map)
 
         def get_locations():
             return {item_id: self._get_item_by_value(item_id).location for item_id in items.keys()}
@@ -227,23 +226,23 @@ class InteractionTestBase(object):
         self.scroll_down(pixels=scroll_down)
 
         for item_key, definition in items.items():
-            if use_keyboard:
-                self.move_item_to_zone(definition.item_id, definition.zone_id, action_key)
-            else:
-                self.drag_item_to_zone(definition.item_id, definition.zone_id)
+            self.place_item(definition.item_id, definition.zone_id, action_key)
             if definition.input:
-                self._send_input(item_key, definition.input)
+                self.send_input(item_key, definition.input)
                 input_div = self._get_input_div_by_value(item_key)
                 self.wait_until_has_class('correct', input_div)
             self.assert_placed_item(definition.item_id, definition.zone_id)
 
         self.wait_until_html_in(feedback['final'], self._get_feedback_message())
 
+        # Check decoy items
+        self.assert_decoy_items(items_map)
+
         # Scroll "Reset exercise" button into view to make sure Selenium can successfully click it
         self.scroll_down(pixels=scroll_down+150)
 
-        reset = self.get_reset_button()
-        if use_keyboard:
+        reset = self._get_reset_button()
+        if action_key is not None:  # Using keyboard to interact with block:
             reset.send_keys(Keys.RETURN)
         else:
             reset.click()
@@ -281,6 +280,12 @@ class InteractionTestBase(object):
         self.assertFalse(dialog_modal_overlay.is_displayed())
         self.assertFalse(dialog_modal.is_displayed())
 
+        if use_keyboard:  # Try again with "?" key
+            self._page.send_keys("?")
+
+            self.assertTrue(dialog_modal_overlay.is_displayed())
+            self.assertTrue(dialog_modal.is_displayed())
+
 
 class BasicInteractionTest(InteractionTestBase):
     """
@@ -333,38 +338,28 @@ class BasicInteractionTest(InteractionTestBase):
 
 @ddt
 class KeyboardInteractionTest(BasicInteractionTest, BaseIntegrationTest):
-    @data(Keys.RETURN, Keys.SPACE)
-    def test_item_positive_feedback_on_good_move(self, action_key):
-        self.parameterized_item_positive_feedback_on_good_move(
-            self.items_map, use_keyboard=True, action_key=action_key
-        )
+    @data(Keys.RETURN, Keys.SPACE, Keys.CONTROL+'m', Keys.COMMAND+'m')
+    def test_item_positive_feedback_on_good_move_with_keyboard(self, action_key):
+        self.parameterized_item_positive_feedback_on_good_move(self.items_map, action_key=action_key)
 
-    @data(Keys.RETURN, Keys.SPACE)
-    def test_item_positive_feedback_on_good_input(self, action_key):
-        self.parameterized_item_positive_feedback_on_good_input(
-            self.items_map, use_keyboard=True, action_key=action_key
-        )
+    @data(Keys.RETURN, Keys.SPACE, Keys.CONTROL+'m', Keys.COMMAND+'m')
+    def test_item_positive_feedback_on_good_input_with_keyboard(self, action_key):
+        self.parameterized_item_positive_feedback_on_good_input(self.items_map, action_key=action_key)
 
-    @data(Keys.RETURN, Keys.SPACE)
-    def test_item_negative_feedback_on_bad_move(self, action_key):
-        self.parameterized_item_negative_feedback_on_bad_move(
-            self.items_map, self.all_zones, use_keyboard=True, action_key=action_key
-        )
+    @data(Keys.RETURN, Keys.SPACE, Keys.CONTROL+'m', Keys.COMMAND+'m')
+    def test_item_negative_feedback_on_bad_move_with_keyboard(self, action_key):
+        self.parameterized_item_negative_feedback_on_bad_move(self.items_map, self.all_zones, action_key=action_key)
 
-    @data(Keys.RETURN, Keys.SPACE)
-    def test_item_negative_feedback_on_bad_input(self, action_key):
-        self.parameterized_item_negative_feedback_on_bad_input(
-            self.items_map, use_keyboard=True, action_key=action_key
-        )
+    @data(Keys.RETURN, Keys.SPACE, Keys.CONTROL+'m', Keys.COMMAND+'m')
+    def test_item_negative_feedback_on_bad_input_with_keyboard(self, action_key):
+        self.parameterized_item_negative_feedback_on_bad_input(self.items_map, action_key=action_key)
 
-    @data(Keys.RETURN, Keys.SPACE)
-    def test_final_feedback_and_reset(self, action_key):
-        self.parameterized_final_feedback_and_reset(
-            self.items_map, self.feedback, use_keyboard=True, action_key=action_key
-        )
+    @data(Keys.RETURN, Keys.SPACE, Keys.CONTROL+'m', Keys.COMMAND+'m')
+    def test_final_feedback_and_reset_with_keyboard(self, action_key):
+        self.parameterized_final_feedback_and_reset(self.items_map, self.feedback, action_key=action_key)
 
-    def test_keyboard_help(self, use_keyboard=True):
-        self.interact_with_keyboard_help()
+    def test_keyboard_help(self):
+        self.interact_with_keyboard_help(use_keyboard=True)
 
 
 class CustomDataInteractionTest(BasicInteractionTest, BaseIntegrationTest):