A11Y: Update bulk selection keyboard shortcuts (#26069)

* A11Y: Update bulk selection keyboard shortcuts

Still a draft, but in current state this:

- adds `shift+b` as a keyboard shortcut to toggle bulk select
- adds `shift+d` as a keyboard shortcut to dismiss selected topic(s) (this
replaces `x r` and `x t` shortcuts)
- adds `x` as a keyboard shortcut to toggle selection (while in bulk select mode)
- fixes a bug with the `shift+a` shortcut, which was not working properly

Note that there is a breaking change here. Previously we had:

- `x r` to dismiss new topics
- `x t` to dismiss unread topics

However, this meant that we couldn't use `x` for selection, because the
itsatrap library does not allow the same character to be used both as a
single character shortcut and as the start of a sequence. The proposed
solution here is more consistent with other apps (Gmail, Github) that use
`x` to toggle selection.

Also, we never show both "Dismiss New" and "Dismiss Unread" in the same
screen, hence it makes sense to consolidate both actions under `shift+d`.

* Address review

app/assets/javascripts/discourse/app/components/modal/keyboard-shortcuts-help.js

@@ -88,11 +88,14 @@ export default class KeyboardShortcutsHelp extends Component {
             keysDelimiter: PLUS,
           }),
           help: buildShortcut("application.help", { keys1: ["?"] }),
-          dismiss_new: buildShortcut("application.dismiss_new", {
-            keys1: ["x", "r"],
+          bulk_select: buildShortcut("application.toggle_bulk_select", {
+            keys1: [SHIFT, "b"],
           }),
-          dismiss_topics: buildShortcut("application.dismiss_topics", {
-            keys1: ["x", "t"],
+          dismiss: buildShortcut("application.dismiss", {
+            keys1: [SHIFT, "d"],
+          }),
+          x: buildShortcut("application.x", {
+            keys1: ["x"],
           }),
           log_out: buildShortcut("application.log_out", {
             keys1: [SHIFT, "z"],

app/assets/javascripts/discourse/app/lib/keyboard-shortcuts.js

@@ -116,12 +116,14 @@ const DEFAULT_BINDINGS = {
   "shift+f11": { handler: "fullscreenComposer", global: true },
   "shift+u": { handler: "deferTopic" },
   "shift+a": { handler: "toggleAdminActions" },
+  "shift+b": { handler: "toggleBulkSelect" },
   t: { postAction: "replyAsNewTopic" },
   u: { handler: "goBack", anonymous: true },
-  "x r": {
-    click: "#dismiss-new-bottom,#dismiss-new-top",
-  }, // dismiss new
-  "x t": { click: "#dismiss-topics-bottom,#dismiss-topics-top" }, // dismiss topics
+  x: { handler: "bulkSelectItem" },
+  "shift+d": {
+    click:
+      "#dismiss-new-bottom, #dismiss-new-top, #dismiss-topics-bottom, #dismiss-topics-top",
+  }, // dismiss new or unread
 };
 
 const animationDuration = 100;
@@ -412,6 +414,13 @@ export default {
     this._moveSelection({ direction: -1, scrollWithinPosts: true });
   },
 
+  bulkSelectItem() {
+    const elem = document.querySelector(
+      ".selected input.bulk-select, .selected .select-post"
+    );
+    elem?.click();
+  },
+
   goBack() {
     history.back();
   },
@@ -895,7 +904,17 @@ export default {
   },
 
   toggleAdminActions() {
-    this.appEvents.trigger("topic:toggle-actions");
+    document.querySelector(".toggle-admin-menu")?.click();
+  },
+
+  toggleBulkSelect() {
+    const bulkSelect = document.querySelector("button.bulk-select");
+
+    if (bulkSelect) {
+      bulkSelect.click();
+    } else {
+      getOwner(this).lookup("controller:topic").send("toggleMultiSelect");
+    }
   },
 
   toggleArchivePM() {

app/assets/javascripts/discourse/tests/acceptance/keyboard-shortcuts-test.js

@@ -153,8 +153,7 @@ acceptance("Keyboard Shortcuts - Authenticated Users", function (needs) {
       exists("#dismiss-topics-top"),
       "dismiss unread top button is present"
     );
-    await triggerKeyEvent(document, "keypress", "X");
-    await triggerKeyEvent(document, "keypress", "T");
+    await triggerKeyEvent(document, "keydown", "D", { shiftKey: true });
     assert.ok(
       exists("#dismiss-read-confirm"),
       "confirmation modal to dismiss unread is present"
@@ -183,8 +182,8 @@ acceptance("Keyboard Shortcuts - Authenticated Users", function (needs) {
       exists("#dismiss-topics-bottom"),
       "dismiss unread bottom button is hidden"
     );
-    await triggerKeyEvent(document, "keypress", "X");
-    await triggerKeyEvent(document, "keypress", "T");
+
+    await triggerKeyEvent(document, "keydown", "D", { shiftKey: true });
     assert.ok(
       exists("#dismiss-read-confirm"),
       "confirmation modal to dismiss unread is present"
@@ -212,8 +211,8 @@ acceptance("Keyboard Shortcuts - Authenticated Users", function (needs) {
     document.getElementById("ember-testing-container").scrollTop = 0;
     await visit("/new");
     assert.ok(exists("#dismiss-new-top"), "dismiss new top button is present");
-    await triggerKeyEvent(document, "keypress", "X");
-    await triggerKeyEvent(document, "keypress", "R");
+
+    await triggerKeyEvent(document, "keydown", "D", { shiftKey: true });
     assert.strictEqual(resetNewCalled, 1);
 
     // we get rid of all but one topic so the bottom dismiss button doesn't
@@ -229,8 +228,8 @@ acceptance("Keyboard Shortcuts - Authenticated Users", function (needs) {
       exists("#dismiss-new-bottom"),
       "dismiss new bottom button has been hidden"
     );
-    await triggerKeyEvent(document, "keypress", "X");
-    await triggerKeyEvent(document, "keypress", "R");
+
+    await triggerKeyEvent(document, "keydown", "D", { shiftKey: true });
     assert.strictEqual(resetNewCalled, 2);
 
     // restore the original topic list
@@ -252,8 +251,7 @@ acceptance("Keyboard Shortcuts - Authenticated Users", function (needs) {
       "dismiss new bottom button is present"
     );
 
-    await triggerKeyEvent(document, "keypress", "X");
-    await triggerKeyEvent(document, "keypress", "R");
+    await triggerKeyEvent(document, "keydown", "D", { shiftKey: true });
 
     assert.strictEqual(resetNewCalled, 1);
   });

config/locales/client.en.yml

@@ -4254,8 +4254,9 @@ en:
         search: "%{shortcut} Search"
         filter_sidebar: "%{shortcut} Filter sidebar"
         help: "%{shortcut} Open keyboard help"
-        dismiss_new: "%{shortcut} Dismiss New"
-        dismiss_topics: "%{shortcut} Dismiss Topics"
+        toggle_bulk_select: "%{shortcut} Toggle bulk select"
+        dismiss: "%{shortcut} Dismiss selected topic(s)"
+        x: "%{shortcut} Toggle selection (in bulk select mode)"
         log_out: "%{shortcut} Log Out"
       composing:
         title: "Composing"

spec/system/page_objects/components/topic_list.rb

@@ -46,6 +46,14 @@ module PageObjects
         page.has_no_css?("#{topic_list_item_unread_badge(topic)}")
       end
 
+      def has_checkbox_selected_on_row?(n)
+        page.has_css?("#{TOPIC_LIST_ITEM_SELECTOR}:nth-child(#{n}) input.bulk-select:checked")
+      end
+
+      def has_no_checkbox_selected_on_row?(n)
+        page.has_no_css?("#{TOPIC_LIST_ITEM_SELECTOR}:nth-child(#{n}) input.bulk-select:checked")
+      end
+
       def click_topic_checkbox(topic)
         find("#{topic_list_item_class(topic)} input#bulk-select-#{topic.id}").click
       end

spec/system/page_objects/components/topic_list_header.rb

@@ -50,6 +50,10 @@ module PageObjects
         find("#topic-bulk-action-options__silent").click
       end
 
+      def click_dismiss_read_confirm
+        find("#dismiss-read-confirm").click
+      end
+
       private
 
       def bulk_select_dropdown_item(name)

spec/system/topic_bulk_select_spec.rb

@@ -87,5 +87,37 @@ describe "Topic bulk select", type: :system do
       visit("/latest")
       expect(topic_list).to have_no_unread_badge(topics.first)
     end
+
+    it "works with keyboard shortcuts" do
+      sign_in(admin)
+      visit("/latest")
+
+      send_keys([:shift, "b"])
+      send_keys("j")
+      send_keys("x") # toggle select
+      expect(topic_list).to have_checkbox_selected_on_row(1)
+
+      send_keys("x") # toggle deselect
+      expect(topic_list).to have_no_checkbox_selected_on_row(1)
+
+      # watch topic and add a reply so we have something in /unread
+      topic = topics.first
+      visit("/t/#{topic.slug}/#{topic.id}")
+      topic_page.watch_topic
+      expect(topic_page).to have_read_post(1)
+      Fabricate(:post, topic: topic)
+
+      visit("/unread")
+      expect(topic_list).to have_topics
+
+      send_keys([:shift, "b"])
+      send_keys("j")
+      send_keys("x")
+      send_keys([:shift, "d"])
+
+      topic_list_header.click_dismiss_read_confirm
+
+      expect(topic_list).to have_no_topics
+    end
   end
 end

spec/system/topic_page_spec.rb

@@ -2,6 +2,7 @@
 
 describe "Topic page", type: :system do
   fab!(:topic)
+  fab!(:admin)
 
   before { Fabricate(:post, topic: topic, cooked: <<~HTML) }
     <h2 dir="ltr" id="toc-h2-testing" data-d-toc="toc-h2-testing" class="d-toc-post-heading">
@@ -58,7 +59,7 @@ describe "Topic page", type: :system do
       PostDestroyer.new(Discourse.system_user, post2).destroy
       PostDestroyer.new(Discourse.system_user, post3).destroy
 
-      sign_in Fabricate(:admin)
+      sign_in admin
     end
 
     it "displays the gap to admins, and allows them to expand it" do
@@ -69,4 +70,20 @@ describe "Topic page", type: :system do
       expect(page).to have_css(".topic-post", count: 4)
     end
   end
+
+  it "supports shift+a kbd shortcut to toggle admin menu" do
+    sign_in admin
+
+    visit("/t/#{topic.slug}/#{topic.id}")
+
+    expect(".topic-admin-menu-button-container").to be_present
+
+    send_keys([:shift, "a"])
+
+    expect(page).to have_css(".topic-admin-popup-menu")
+
+    send_keys([:shift, "a"])
+
+    expect(page).to have_no_css(".topic-admin-popup-menu")
+  end
 end