diff --git a/app/assets/javascripts/discourse/app/components/topic-list-item.js b/app/assets/javascripts/discourse/app/components/topic-list-item.js index f18e912aa53..029771c44cf 100644 --- a/app/assets/javascripts/discourse/app/components/topic-list-item.js +++ b/app/assets/javascripts/discourse/app/components/topic-list-item.js @@ -13,6 +13,7 @@ import { schedule } from "@ember/runloop"; import { topicTitleDecorators } from "discourse/components/topic-title"; import { wantsNewWindow } from "discourse/lib/intercept-click"; import { htmlSafe } from "@ember/template"; +import { inject as service } from "@ember/service"; export function showEntrance(e) { let target = $(e.target); @@ -39,11 +40,18 @@ export function navigateToTopic(topic, href) { // so skip setting it early. this.appEvents.trigger("header:update-topic", topic); } + + this.session.set("lastTopicIdViewed", { + topicId: topic.id, + historyUuid: this.router.location.getState?.().uuid, + }); + DiscourseURL.routeTo(href || topic.get("url")); return false; } export default Component.extend({ + router: service(), tagName: "tr", classNameBindings: [":topic-list-item", "unboundClassNames", "topic.visited"], attributeBindings: ["data-topic-id", "role", "ariaLevel:aria-level"], @@ -328,7 +336,14 @@ export default Component.extend({ _highlightIfNeeded: on("didInsertElement", function () { // highlight the last topic viewed - if (this.session.get("lastTopicIdViewed") === this.get("topic.id")) { + const lastViewedTopicInfo = this.session.get("lastTopicIdViewed"); + + const isLastViewedTopic = + lastViewedTopicInfo?.topicId === this.topic.id && + lastViewedTopicInfo?.historyUuid === + this.router.location.getState?.().uuid; + + if (isLastViewedTopic) { this.session.set("lastTopicIdViewed", null); this.highlight({ isLastViewedTopic: true }); } else if (this.get("topic.highlight")) { diff --git a/app/assets/javascripts/discourse/app/routes/topic.js b/app/assets/javascripts/discourse/app/routes/topic.js index 520c3f3896f..815745ce3ff 100644 --- a/app/assets/javascripts/discourse/app/routes/topic.js +++ b/app/assets/javascripts/discourse/app/routes/topic.js @@ -323,9 +323,6 @@ const TopicRoute = DiscourseRoute.extend({ activate() { this._super(...arguments); this.set("isTransitioning", false); - - const topic = this.modelFor("topic"); - this.session.set("lastTopicIdViewed", parseInt(topic.get("id"), 10)); }, deactivate() { diff --git a/app/assets/javascripts/discourse/tests/acceptance/last-visited-topic-focus-test.js b/app/assets/javascripts/discourse/tests/acceptance/last-visited-topic-focus-test.js deleted file mode 100644 index b2456c72da1..00000000000 --- a/app/assets/javascripts/discourse/tests/acceptance/last-visited-topic-focus-test.js +++ /dev/null @@ -1,26 +0,0 @@ -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")); - }); -}); diff --git a/spec/system/page_objects/components/topic_list.rb b/spec/system/page_objects/components/topic_list.rb index ab7d0a6a960..4b03dcb3c93 100644 --- a/spec/system/page_objects/components/topic_list.rb +++ b/spec/system/page_objects/components/topic_list.rb @@ -34,6 +34,10 @@ module PageObjects find("#{TOPIC_LIST_BODY_SELECTOR} a", text: title).click end + def visit_topic(topic) + find("#{topic_list_item_class(topic)} a.raw-topic-link").click + end + private def topic_list_item_class(topic) diff --git a/spec/system/topic_list_focus_spec.rb b/spec/system/topic_list_focus_spec.rb new file mode 100644 index 00000000000..228fcd4e303 --- /dev/null +++ b/spec/system/topic_list_focus_spec.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +describe "Topic list focus", type: :system do + let!(:topics) { Fabricate.times(10, :post).map(&:topic) } + + before { Fabricate(:admin) } + + let(:discovery) { PageObjects::Pages::Discovery.new } + let(:topic) { PageObjects::Pages::Topic.new } + + def focussed_topic_id + page.evaluate_script( + "document.activeElement.closest('.topic-list-item')?.dataset.topicId", + )&.to_i + end + + it "refocusses last clicked topic when going back to topic list" do + visit("/") + expect(page).to have_css("body.navigation-topics") + expect(discovery.topic_list).to have_topics + + # Click a topic + discovery.topic_list.visit_topic(topics[5]) + expect(topic).to have_topic_title(topics[5].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[5].id) + + # Click topic again + discovery.topic_list.visit_topic(topics[5]) + expect(topic).to have_topic_title(topics[5].title) + + # Visiting a topic list another way should not focus + find(".sidebar-section-link[data-link-name='everything']").click + expect(page).to have_css("body.navigation-topics") + expect(focussed_topic_id).to eq(nil) + end +end