A11Y: Focus last viewed topic in topic lists (take 3) (#16257)

Another attempt at fixing https://meta.discourse.org/t/discourse-with-a-screen-reader/178105/88?u=osama. Previous PR (reverted): #16240.

The problems with the previous PR were:

1. As you scrolled down a topics list, the first topic of every new batch of topics would receive focus and the indicator would show up.
2. Similar to 1, clicking the `See X new or updated topics` notice would also focus a random topic from the new topics that were just loaded.
3. Topics in the suggested topics list received focus too
4. Our custom focus indicator appeared on mobile, but it shouldn't.

This commit should have none of these problems.
This commit is contained in:
Osama Sayegh 2022-03-23 13:03:56 +03:00 committed by GitHub
parent 8040b95e8c
commit eb237e634a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 88 additions and 9 deletions

View File

@ -1,4 +1,7 @@
import discourseComputed, { observes } from "discourse-common/utils/decorators";
import discourseComputed, {
bind,
observes,
} from "discourse-common/utils/decorators";
import Component from "@ember/component";
import DiscourseURL from "discourse/lib/url";
import I18n from "I18n";
@ -58,6 +61,11 @@ export default Component.extend({
if (this.selected && this.selected.includes(this.topic)) {
this.element.querySelector("input.bulk-select").checked = true;
}
const title = this.element.querySelector(".main-link .title");
if (title) {
title.addEventListener("focus", this._onTitleFocus);
title.addEventListener("blur", this._onTitleBlur);
}
});
}
},
@ -98,6 +106,11 @@ export default Component.extend({
if (this.includeUnreadIndicator) {
this.messageBus.unsubscribe(this.unreadIndicatorChannel);
}
const title = this.element?.querySelector(".main-link .title");
if (title) {
title.removeEventListener("focus", this._onTitleFocus);
title.removeEventListener("blur", this._onTitleBlur);
}
},
@discourseComputed("topic.id")
@ -259,12 +272,21 @@ export default Component.extend({
return;
}
const $topic = $(this.element);
$topic
.addClass("highlighted")
.attr("data-islastviewedtopic", opts.isLastViewedTopic);
$topic.on("animationend", () => $topic.removeClass("highlighted"));
this.element.classList.add("highlighted");
this.element.setAttribute(
"data-islastviewedtopic",
opts.isLastViewedTopic
);
this.element.addEventListener("animationend", () => {
this.element.classList.remove("highlighted");
});
if (
this.focusLastVisitedTopic &&
opts.isLastViewedTopic &&
!this.site.mobileView
) {
this.element.querySelector(".main-link .title").focus();
}
});
},
@ -279,4 +301,20 @@ export default Component.extend({
this.highlight();
}
}),
@bind
_onTitleFocus() {
if (this.element && !this.isDestroying && !this.isDestroyed) {
const mainLink = this.element.querySelector(".main-link");
mainLink.classList.add("focused");
}
},
@bind
_onTitleBlur() {
if (this.element && !this.isDestroying && !this.isDestroyed) {
const mainLink = this.element.querySelector(".main-link");
mainLink.classList.remove("focused");
}
},
});

View File

@ -42,7 +42,8 @@
lastVisitedTopic=lastVisitedTopic
selected=selected
lastChecked=lastChecked
tagsForUser=tagsForUser}}
tagsForUser=tagsForUser
focusLastVisitedTopic=focusLastVisitedTopic}}
{{raw "list/visited-line" lastVisitedTopic=lastVisitedTopic topic=topic}}
{{/each}}
</tbody>

View File

@ -62,7 +62,9 @@ model=model showResetNew=showResetNew showDismissRead=showDismissRead resetNew=(
topics=model.topics
discoveryList=true
scrollOnLoad=true
onScroll=discoveryTopicList.saveScrollPosition}}
onScroll=discoveryTopicList.saveScrollPosition
focusLastVisitedTopic=true
}}
{{/if}}
{{plugin-outlet name="after-topic-list" tagName="span" connectorTagName="div" args=(hash category=category)}}

View File

@ -88,6 +88,7 @@
changeSort=(action "changeSort")
onScroll=discoveryTopicList.saveScrollPosition
scrollOnLoad=true
focusLastVisitedTopic=true
}}
{{/if}}
{{/discovery-topics-list}}

View File

@ -0,0 +1,26 @@
import { acceptance, query } from "discourse/tests/helpers/qunit-helpers";
import { test } from "qunit";
import { visit } from "@ember/test-helpers";
import { cloneJSON } from "discourse-common/lib/object";
import topicFixtures from "discourse/tests/fixtures/topic";
acceptance("Last Visited Topic Focus", function (needs) {
needs.pretender((server, helper) => {
const fixture = cloneJSON(topicFixtures["/t/54077.json"]);
fixture.id = 11996;
fixture.slug =
"its-really-hard-to-navigate-the-create-topic-reply-pane-with-the-keyboard";
server.get("/t/11996.json", () => helper.response(fixture));
});
test("last visited topic receives focus when you return back to the topic list", async function (assert) {
await visit("/");
await visit(
"/t/its-really-hard-to-navigate-the-create-topic-reply-pane-with-the-keyboard/11996"
);
await visit("/");
const visitedTopicTitle = query(
'.topic-list-body tr[data-topic-id="11996"] .main-link'
);
assert.ok(visitedTopicTitle.classList.contains("focused"));
});
});

View File

@ -234,6 +234,17 @@
.raw-topic-link > * {
pointer-events: none;
}
&.focused {
box-shadow: inset 3px 0 0 var(--tertiary);
}
/* we have a custom focus indicator so we can remove the native one */
.title:focus {
outline: none;
}
.title:focus-visible {
outline: none;
}
}
.unread-indicator {