diff --git a/app/assets/javascripts/discourse/app/lib/keyboard-shortcuts.js b/app/assets/javascripts/discourse/app/lib/keyboard-shortcuts.js index 0b2c07f955d..694e867551d 100644 --- a/app/assets/javascripts/discourse/app/lib/keyboard-shortcuts.js +++ b/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, - { focusTabLoc: true } + article.querySelector("#post_1") ? 0 : articleTopPosition ); } @@ -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() { diff --git a/app/assets/javascripts/discourse/app/lib/utilities.js b/app/assets/javascripts/discourse/app/lib/utilities.js index 9d6e9d7d5a9..1b5903f045b 100644 --- a/app/assets/javascripts/discourse/app/lib/utilities.js +++ b/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"); + const element = container.querySelector(".topic-body, .small-action-desc"); 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); diff --git a/app/assets/javascripts/discourse/app/widgets/post-small-action.js b/app/assets/javascripts/discourse/app/widgets/post-small-action.js index c9ec30955e2..875eb114a51 100644 --- a/app/assets/javascripts/discourse/app/widgets/post-small-action.js +++ b/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), diff --git a/app/assets/javascripts/discourse/app/widgets/post.js b/app/assets/javascripts/discourse/app/widgets/post.js index fa5e0e95b60..eaf2d94ec0f 100644 --- a/app/assets/javascripts/discourse/app/widgets/post.js +++ b/app/assets/javascripts/discourse/app/widgets/post.js @@ -795,11 +795,7 @@ createWidget("post-article", { }, html(attrs, state) { - const rows = [ - h("span.tabLoc", { - attributes: { "aria-hidden": true, tabindex: -1 }, - }), - ]; + const rows = []; if (state.repliesAbove.length) { const replies = state.repliesAbove.map((p) => { return this.attach("embedded-post", p, { diff --git a/app/assets/javascripts/discourse/tests/acceptance/topic-test.js b/app/assets/javascripts/discourse/tests/acceptance/topic-test.js index 2e58940e0e9..7d8c92e1684 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/topic-test.js +++ b/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) { - await visit("/t/internationalization-localization/280"); - await selectText("#post_5 blockquote"); - await triggerKeyEvent(document, "keypress", "J"); - await triggerKeyEvent(document, "keypress", "T"); + // Using J/K on Firefox clean the text selection, so this won't work there + chromeTest( + "Quoting a quote with replyAsNewTopic keeps the original poster name", + async function (assert) { + await visit("/t/internationalization-localization/280"); + await selectText("#post_5 blockquote"); + await triggerKeyEvent(document, "keypress", "J"); + await triggerKeyEvent(document, "keypress", "T"); - assert.ok( - query(".d-editor-input").value.includes( - 'quote="codinghorror said, post:3, topic:280"' - ) - ); - }); + assert.ok( + query(".d-editor-input").value.includes( + '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"); diff --git a/app/assets/stylesheets/common/base/topic-post.scss b/app/assets/stylesheets/common/base/topic-post.scss index 3ee5ca6638d..3bc1abf630b 100644 --- a/app/assets/stylesheets/common/base/topic-post.scss +++ b/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); } diff --git a/app/assets/stylesheets/common/components/keyboard_shortcuts.scss b/app/assets/stylesheets/common/components/keyboard_shortcuts.scss index 73397b26d56..6a068fb9eae 100644 --- a/app/assets/stylesheets/common/components/keyboard_shortcuts.scss +++ b/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 { - outline: none; +.topic-list tr.selected, +.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 { diff --git a/spec/system/topic_list_focus_spec.rb b/spec/system/topic_list_focus_spec.rb index 75b93c75e35..5d9473f4799 100644 --- a/spec/system/topic_list_focus_spec.rb +++ b/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