A11Y: Fix selecting topic when navigation via keyboard (#22996)

This fixes:
- a regression from 30c152c, where navigating to a topic's last reply
  via keyboard would lose track of the topic when returning to the topic
  list
- an issue where if a topic's last post is a small post, navigating to it
   via keyboard would not focus the post

Co-authored-by: David Taylor <david@taylorhq.com>
This commit is contained in:
Penar Musaraj 2023-08-07 17:05:16 -04:00 committed by GitHub
parent 976219ed5c
commit 161d3d190a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 85 additions and 6 deletions

View File

@ -4,6 +4,7 @@ import DiscourseURL from "discourse/lib/url";
import I18n from "I18n"; import I18n from "I18n";
import discourseComputed, { bind } from "discourse-common/utils/decorators"; import discourseComputed, { bind } from "discourse-common/utils/decorators";
import { scheduleOnce } from "@ember/runloop"; import { scheduleOnce } from "@ember/runloop";
import { inject as service } from "@ember/service";
function entranceDate(dt, showTime) { function entranceDate(dt, showTime) {
const today = new Date(); const today = new Date();
@ -29,6 +30,8 @@ function entranceDate(dt, showTime) {
} }
export default Component.extend(CleansUp, { export default Component.extend(CleansUp, {
router: service(),
session: service(),
elementId: "topic-entrance", elementId: "topic-entrance",
classNameBindings: ["visible::hidden"], classNameBindings: ["visible::hidden"],
topic: null, topic: null,
@ -161,6 +164,11 @@ export default Component.extend(CleansUp, {
}, },
_jumpTo(destination) { _jumpTo(destination) {
this.session.set("lastTopicIdViewed", {
topicId: this.topic.id,
historyUuid: this.router.location.getState?.().uuid,
});
this.cleanUp(); this.cleanUp();
DiscourseURL.routeTo(destination); DiscourseURL.routeTo(destination);
}, },

View File

@ -37,9 +37,9 @@ export function showEntrance(e) {
export function navigateToTopic(topic, href) { export function navigateToTopic(topic, href) {
const owner = getOwner(this); const owner = getOwner(this);
const siteSettings = owner.lookup("service:site-settings");
const router = owner.lookup("service:router"); const router = owner.lookup("service:router");
const session = owner.lookup("service:session"); const session = owner.lookup("service:session");
const siteSettings = owner.lookup("service:site-settings");
const appEvents = owner.lookup("service:appEvents"); const appEvents = owner.lookup("service:appEvents");
if (siteSettings.page_loading_indicator !== "slider") { if (siteSettings.page_loading_indicator !== "slider") {
@ -284,7 +284,10 @@ export default Component.extend({
} }
} }
if (classList.contains("raw-topic-link")) { if (
classList.contains("raw-topic-link") ||
classList.contains("post-activity")
) {
if (wantsNewWindow(e)) { if (wantsNewWindow(e)) {
return true; return true;
} }
@ -319,6 +322,13 @@ export default Component.extend({
unhandledRowClick() {}, unhandledRowClick() {},
keyDown(e) {
if (e.key === "Enter" && e.target.classList.contains("post-activity")) {
e.preventDefault();
return this.navigateToTopic(this.topic, e.target.getAttribute("href"));
}
},
navigateToTopic, navigateToTopic,
highlight(opts = { isLastViewedTopic: false }) { highlight(opts = { isLastViewedTopic: false }) {

View File

@ -76,6 +76,9 @@ export function highlightPost(postNumber) {
if (!container) { if (!container) {
return; return;
} }
container.querySelector(".tabLoc")?.focus();
const element = container.querySelector(".topic-body"); const element = container.querySelector(".topic-body");
if (!element || element.classList.contains("highlighted")) { if (!element || element.classList.contains("highlighted")) {
return; return;
@ -88,7 +91,6 @@ export function highlightPost(postNumber) {
element.removeEventListener("animationend", removeHighlighted); element.removeEventListener("animationend", removeHighlighted);
}; };
element.addEventListener("animationend", removeHighlighted); element.addEventListener("animationend", removeHighlighted);
container.querySelector(".tabLoc").focus();
} }
export function emailValid(email) { export function emailValid(email) {

View File

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

View File

@ -328,6 +328,9 @@
.num.activity { .num.activity {
a { a {
padding: 15px 5px; padding: 15px 5px;
span.relative-date {
pointer-events: none;
}
} }
} }
} }

View File

@ -38,6 +38,15 @@ module PageObjects
find("#{topic_list_item_class(topic)} a.raw-topic-link").click find("#{topic_list_item_class(topic)} a.raw-topic-link").click
end end
def visit_topic_last_reply_via_keyboard(topic)
find("#{topic_list_item_class(topic)} a.post-activity").native.send_keys(:return)
end
def visit_topic_first_reply_via_keyboard(topic)
find("#{topic_list_item_class(topic)} button.posts-map").native.send_keys(:return)
find("#topic-entrance button.jump-top").native.send_keys(:return)
end
private private
def topic_list_item_class(topic) def topic_list_item_class(topic)

View File

@ -1,9 +1,18 @@
# frozen_string_literal: true # frozen_string_literal: true
describe "Topic list focus", type: :system do describe "Topic list focus", type: :system do
let!(:topics) { Fabricate.times(10, :post).map(&:topic) } fab!(:topics) { Fabricate.times(10, :post).map(&:topic) }
before { Fabricate(:admin) } before_all do
sidebar_url = Fabricate(:sidebar_url, name: "my topic link", value: "/t/#{topics[4].id}")
Fabricate(
:sidebar_section_link,
sidebar_section:
SidebarSection.find_by(section_type: SidebarSection.section_types[:community]),
linkable: sidebar_url,
)
end
let(:discovery) { PageObjects::Pages::Discovery.new } let(:discovery) { PageObjects::Pages::Discovery.new }
let(:topic) { PageObjects::Pages::Topic.new } let(:topic) { PageObjects::Pages::Topic.new }
@ -15,7 +24,7 @@ describe "Topic list focus", type: :system do
end end
it "refocusses last clicked topic when going back to topic list" do it "refocusses last clicked topic when going back to topic list" do
visit("/") visit("/latest")
expect(page).to have_css("body.navigation-topics") expect(page).to have_css("body.navigation-topics")
expect(discovery.topic_list).to have_topics expect(discovery.topic_list).to have_topics
@ -37,4 +46,39 @@ describe "Topic list focus", type: :system do
expect(page).to have_css("body.navigation-topics") expect(page).to have_css("body.navigation-topics")
expect(focussed_topic_id).to eq(nil) expect(focussed_topic_id).to eq(nil)
end end
it "refocusses properly when navigating via the 'last activity' link" do
visit("/latest")
# Visit topic via activity column and keyboard
discovery.topic_list.visit_topic_last_reply_via_keyboard(topics[2])
expect(topic).to have_topic_title(topics[2].title)
# Going back to the topic-list should re-focus
page.go_back
expect(page).to have_css("body.navigation-topics")
expect(focussed_topic_id).to eq(topics[2].id)
# Visit topic via keyboard using posts map (OP button)
discovery.topic_list.visit_topic_first_reply_via_keyboard(topics[4])
expect(topic).to have_topic_title(topics[4].title)
# Going back to the topic-list should re-focus
page.go_back
expect(page).to have_css("body.navigation-topics")
expect(focussed_topic_id).to eq(topics[4].id)
end
it "does not refocus topic when visiting via something other than topic list" do
visit("/latest")
# Clicking sidebar link should visit topic
find(".sidebar-section-link[data-link-name='my topic link']").click
expect(topic).to have_topic_title(topics[4].title)
# Going back to the topic-list should not re-focus
page.go_back
expect(page).to have_css("body.navigation-topics")
expect(focussed_topic_id).to eq(nil)
end
end end