A11Y: improve setting focus to a post (#24786)

See https://github.com/discourse/discourse/pull/23367 for implementation details.

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

@@ -10,9 +10,7 @@ import DiscourseURL from "discourse/lib/url";
 import Composer from "discourse/models/composer";
 import { capabilities } from "discourse/services/capabilities";
 import { INPUT_DELAY } from "discourse-common/config/environment";
-import discourseDebounce from "discourse-common/lib/debounce";
 import discourseLater from "discourse-common/lib/later";
-import { bind } from "discourse-common/utils/decorators";
 import domUtils from "discourse-common/utils/dom-utils";
 
 let extraKeyboardShortcutsHelp = {};
@@ -750,8 +748,11 @@ export default {
 
     for (const a of articles) {
       a.classList.remove("selected");
+      a.removeAttribute("tabindex");
     }
     article.classList.add("selected");
+    article.setAttribute("tabindex", "0");
+    article.focus();
 
     this.appEvents.trigger("keyboard:move-selection", {
       articles,
@@ -768,8 +769,7 @@ export default {
       );
     } else if (article.classList.contains("topic-post")) {
       return this._scrollTo(
-        article.querySelector("#post_1") ? 0 : articleTopPosition,
+        article.querySelector("#post_1") ? 0 : articleTopPosition
-        { focusTabLoc: true }
       );
     }
 
@@ -786,25 +786,11 @@ export default {
     this._scrollTo(articleTopPosition - window.innerHeight * scrollRatio);
   },
 
-  _scrollTo(scrollTop, opts = {}) {
+  _scrollTo(scrollTop) {
     window.scrollTo({
       top: scrollTop,
       behavior: "smooth",
     });
-
-    if (opts.focusTabLoc) {
-      window.addEventListener("scroll", this._onScrollEnds, { passive: true });
-    }
-  },
-
-  @bind
-  _onScrollEnds() {
-    window.removeEventListener("scroll", this._onScrollEnds, { passive: true });
-    discourseDebounce(this, this._onScrollEndsCallback, animationDuration);
-  },
-
-  _onScrollEndsCallback() {
-    document.querySelector(".topic-post.selected span.tabLoc")?.focus();
   },
 
   categoriesTopicsList() {

app/assets/javascripts/discourse/app/lib/utilities.js

@@ -78,17 +78,21 @@ export function highlightPost(postNumber) {
     return;
   }
 
-  container.querySelector(".tabLoc")?.focus();
+  const element = container.querySelector(".topic-body, .small-action-desc");
-
-  const element = container.querySelector(".topic-body");
   if (!element || element.classList.contains("highlighted")) {
     return;
   }
 
   element.classList.add("highlighted");
 
+  if (postNumber > 1) {
+    element.setAttribute("tabindex", "0");
+    element.focus();
+  }
+
   const removeHighlighted = function () {
     element.classList.remove("highlighted");
+    element.removeAttribute("tabindex");
     element.removeEventListener("animationend", removeHighlighted);
   };
   element.addEventListener("animationend", removeHighlighted);

app/assets/javascripts/discourse/app/widgets/post-small-action.js

@@ -190,9 +190,6 @@ export default createWidget("post-small-action", {
     }
 
     return [
-      h("span.tabLoc", {
-        attributes: { "aria-hidden": true, tabindex: -1 },
-      }),
       h("div.topic-avatar", iconNode(icons[attrs.actionCode] || "exclamation")),
       h("div.small-action-desc", [
         h("div.small-action-contents", contents),

app/assets/javascripts/discourse/app/widgets/post.js

@@ -795,11 +795,7 @@ createWidget("post-article", {
   },
 
   html(attrs, state) {
-    const rows = [
+    const rows = [];
-      h("span.tabLoc", {
-        attributes: { "aria-hidden": true, tabindex: -1 },
-      }),
-    ];
     if (state.repliesAbove.length) {
       const replies = state.repliesAbove.map((p) => {
         return this.attach("embedded-post", p, {

app/assets/javascripts/discourse/tests/acceptance/topic-test.js

@@ -11,6 +11,7 @@ import CategoryFixtures from "discourse/tests/fixtures/category-fixtures";
 import topicFixtures from "discourse/tests/fixtures/topic";
 import {
   acceptance,
+  chromeTest,
   count,
   exists,
   publishToMessageBus,
@@ -425,18 +426,22 @@ acceptance("Topic featured links", function (needs) {
     );
   });
 
-  test("Quoting a quote with replyAsNewTopic keeps the original poster name", async function (assert) {
+  // Using J/K on Firefox clean the text selection, so this won't work there
-    await visit("/t/internationalization-localization/280");
+  chromeTest(
-    await selectText("#post_5 blockquote");
+    "Quoting a quote with replyAsNewTopic keeps the original poster name",
-    await triggerKeyEvent(document, "keypress", "J");
+    async function (assert) {
-    await triggerKeyEvent(document, "keypress", "T");
+      await visit("/t/internationalization-localization/280");
+      await selectText("#post_5 blockquote");
+      await triggerKeyEvent(document, "keypress", "J");
+      await triggerKeyEvent(document, "keypress", "T");
 
-    assert.ok(
+      assert.ok(
-      query(".d-editor-input").value.includes(
+        query(".d-editor-input").value.includes(
-        'quote="codinghorror said, post:3, topic:280"'
+          'quote="codinghorror said, post:3, topic:280"'
-      )
+        )
-    );
+      );
-  });
+    }
+  );
 
   test("Quoting by selecting text can mark the quote as full", async function (assert) {
     await visit("/t/internationalization-localization/280");

app/assets/stylesheets/common/base/topic-post.scss

@@ -67,6 +67,7 @@
     vertical-align: middle;
     a {
       color: var(--primary-high-or-secondary-low);
+      outline-offset: -1px;
     }
   }
   .fa {
@@ -941,11 +942,24 @@ aside.quote {
     border-top: 1px solid var(--primary-low);
     padding-top: 0.5em;
   }
+
   @media (prefers-reduced-motion: no-preference) {
     &.highlighted {
       animation: background-fade-highlight 2.5s ease-out;
     }
   }
+}
+
+.topic-body:not(.deleted),
+.small-action-desc {
+  @media (prefers-reduced-motion: no-preference) {
+    &.highlighted {
+      animation: background-fade-highlight 2.5s ease-out;
+    }
+  }
+  &.highlighted:focus-visible {
+    outline: none;
+  }
   .deleted & {
     // Disable so the deleted background is visible immediately
     &.highlighted {
@@ -1140,6 +1154,10 @@ blockquote > *:last-child {
   display: flex;
   align-items: center;
 
+  &:focus-visible {
+    outline: none;
+  }
+
   &.deleted {
     background-color: var(--danger-low-mid);
   }

app/assets/stylesheets/common/components/keyboard_shortcuts.scss

@@ -7,7 +7,7 @@
 .topic-list-item.selected td:first-child,
 .latest-topic-list-item.selected,
 .search-results .fps-result.selected {
-  box-shadow: inset 3px 0 0 var(--danger); // needs to be inset for Edge
+  box-shadow: inset 3px 0 0 var(--danger);
 }
 
 .featured-topic.selected,
@@ -15,8 +15,15 @@
   box-shadow: -3px 0 0 var(--danger);
 }
 
-.tabLoc:focus {
+.topic-list tr.selected,
-  outline: none;
+.topic-list-item.selected,
+.featured-topic.selected,
+.topic-post.selected,
+.latest-topic-list-item.selected,
+.search-results .fps-result.selected {
+  &:focus-visible {
+    outline: none;
+  }
 }
 
 .latest .featured-topic {

spec/system/topic_list_focus_spec.rb

@@ -23,6 +23,10 @@ describe "Topic list focus", type: :system do
     )&.to_i
   end
 
+  def focussed_post_id
+    page.evaluate_script("document.activeElement.closest('.onscreen-post')?.dataset.postId")&.to_i
+  end
+
   it "refocusses last clicked topic when going back to topic list" do
     visit("/latest")
     expect(page).to have_css("body.navigation-topics")
@@ -103,4 +107,17 @@ describe "Topic list focus", type: :system do
     expect(page).to have_css("body.navigation-topics")
     expect(focussed_topic_id).to eq(oldest_topic.id)
   end
+
+  it "sets focus to the last post when navigating to a topic" do
+    extra_posts = Fabricate.times(5, :post, topic: topics[2])
+
+    visit("/latest")
+
+    discovery.topic_list.visit_topic_last_reply_via_keyboard(topics[2])
+    # send Tab key twice, the first event serves to focus the window
+    find("body").native.send_keys :tab
+    find("body").native.send_keys :tab
+
+    expect(focussed_post_id).to eq(topics[2].posts.last.id)
+  end
 end