FIX: Dismiss new keyboard shortcut not working (#13430)

FIX: Dismiss new keyboard shortcut not working (#13430)

The dismiss new keyboard shortcut (x,r) has been broken since 7a79bd7da3e0a59454a3c03f3be31e457d0b9fcc. A fix was done and JS tests were added in 006d52f32b8d61449dcdd42fa09b06b1573091b0 and b01e4738abe206cd1d09db74acacc743c457c374 but the test was not quite correct and so the bottom dismiss new button was not clicked.

This also fixes an issue with our keyboard shortcut click handling. If multiple elements matched the selector they were all clicked. Now we just click the first match.

diff --git a/app/assets/javascripts/discourse/app/lib/keyboard-shortcuts.js b/app/assets/javascripts/discourse/app/lib/keyboard-shortcuts.js
index 3ad6a9c..7c80fb4 100644
--- a/app/assets/javascripts/discourse/app/lib/keyboard-shortcuts.js
+++ b/app/assets/javascripts/discourse/app/lib/keyboard-shortcuts.js
@@ -86,8 +86,8 @@ const DEFAULT_BINDINGS = {
   t: { postAction: "replyAsNewTopic" },
   u: { handler: "goBack", anonymous: true },
   "x r": {
-    click: "#dismiss-new,#dismiss-new-top,#dismiss-posts,#dismiss-posts-top",
-  }, // dismiss new/posts
+    click: "#dismiss-new-bottom,#dismiss-new-top",
+  }, // dismiss new
   "x t": { click: "#dismiss-topics-bottom,#dismiss-topics-top" }, // dismiss topics
 };
 
@@ -549,7 +549,11 @@ export default {
         // If effective, prevent default.
         e.preventDefault();
       }
-      $sel.click();
+
+      // If there is more than one match for the selector, just click
+      // the first one, we don't want to click multiple things from one
+      // shortcut.
+      $sel[0].click();
     });
   },
 
diff --git a/app/assets/javascripts/discourse/tests/acceptance/keyboard-shortcuts-test.js b/app/assets/javascripts/discourse/tests/acceptance/keyboard-shortcuts-test.js
index bb316a1..9ba4298 100644
--- a/app/assets/javascripts/discourse/tests/acceptance/keyboard-shortcuts-test.js
+++ b/app/assets/javascripts/discourse/tests/acceptance/keyboard-shortcuts-test.js
@@ -82,6 +82,7 @@ acceptance("Keyboard Shortcuts - Authenticated Users", function (needs) {
 
   test("dismiss unread from top and bottom button", async function (assert) {
     await visit("/unread");
+    assert.ok(exists("#dismiss-topics-top"));
     await triggerKeyEvent(document, "keypress", "x".charCodeAt(0));
     await triggerKeyEvent(document, "keypress", "t".charCodeAt(0));
     assert.ok(exists("#dismiss-read-confirm"));
@@ -98,7 +99,10 @@ acceptance("Keyboard Shortcuts - Authenticated Users", function (needs) {
     let originalTopics = [...topicList.topic_list.topics];
     topicList.topic_list.topics = [topicList.topic_list.topics[0]];
 
+    // visit root first so topic list starts fresh
+    await visit("/");
     await visit("/unread");
+    assert.notOk(exists("#dismiss-topics-top"));
     await triggerKeyEvent(document, "keypress", "x".charCodeAt(0));
     await triggerKeyEvent(document, "keypress", "t".charCodeAt(0));
     assert.ok(exists("#dismiss-read-confirm"));
@@ -115,6 +119,7 @@ acceptance("Keyboard Shortcuts - Authenticated Users", function (needs) {
 
   test("dismiss new from top and bottom button", async function (assert) {
     await visit("/new");
+    assert.ok(exists("#dismiss-new-top"));
     await triggerKeyEvent(document, "keypress", "x".charCodeAt(0));
     await triggerKeyEvent(document, "keypress", "r".charCodeAt(0));
     assert.equal(resetNewCalled, 1);
@@ -125,7 +130,10 @@ acceptance("Keyboard Shortcuts - Authenticated Users", function (needs) {
     let originalTopics = [...topicList.topic_list.topics];
     topicList.topic_list.topics = [topicList.topic_list.topics[0]];
 
+    // visit root first so topic list starts fresh
+    await visit("/");
     await visit("/new");
+    assert.notOk(exists("#dismiss-new-top"));
     await triggerKeyEvent(document, "keypress", "x".charCodeAt(0));
     await triggerKeyEvent(document, "keypress", "r".charCodeAt(0));
     assert.equal(resetNewCalled, 2);
@@ -133,4 +141,15 @@ acceptance("Keyboard Shortcuts - Authenticated Users", function (needs) {
     // restore the original topic list
     topicList.topic_list.topics = originalTopics;
   });
+
+  test("click event not fired twice when both dismiss buttons are present", async function (assert) {
+    await visit("/new");
+    assert.ok(exists("#dismiss-new-top"));
+    assert.ok(exists("#dismiss-new-bottom"));
+
+    await triggerKeyEvent(document, "keypress", "x".charCodeAt(0));
+    await triggerKeyEvent(document, "keypress", "r".charCodeAt(0));
+
+    assert.equal(resetNewCalled, 1);
+  });
 });

GitHub sha: 7f916ad06d160c0721f3aea80db75eb985582960

This commit appears in #13430 which was approved by tgxworld. It was merged by martin.