From 0816ad4fe26d6a1b8918d768e2a8f6e9a3bb888a Mon Sep 17 00:00:00 2001 From: Giancarlo Salamanca Date: Wed, 11 Nov 2026 11:32:52 -0500 Subject: [PATCH] HOOD-6816 Keep linked document and earlier-text discussions open --- .../coplan/text_selection_controller.js | 5 +- spec/system/linked_comment_navigation_spec.rb | 56 +++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 spec/system/linked_comment_navigation_spec.rb diff --git a/engine/app/javascript/controllers/coplan/text_selection_controller.js b/engine/app/javascript/controllers/coplan/text_selection_controller.js index 3f54483a..d195d094 100644 --- a/engine/app/javascript/controllers/coplan/text_selection_controller.js +++ b/engine/app/javascript/controllers/coplan/text_selection_controller.js @@ -551,7 +551,10 @@ export default class extends Controller { // Margin cards survive re-highlighting; keep the window beside its card. if (this._activeMark?.isConnected && this._activeMark.closest(".comment-margin")) return - const replacementMark = this.contentTarget.querySelector(`.anchor-highlight[data-thread-id="${threadId}"]`) + // Document and earlier-text discussions have list entries instead of + // inline marks. Rendering updates must keep those windows attached too. + const replacementMark = this.contentTarget.querySelector(`.anchor-highlight[data-thread-id="${threadId}"]`) || + this.element.querySelector(`#plan-general-comments [data-thread-id="${threadId}"], #plan-detached-comments [data-thread-id="${threadId}"]`) if (replacementMark && this._findOpenPopover() === this._activePopover) { this._showThreadPopoverFor(replacementMark) return diff --git a/spec/system/linked_comment_navigation_spec.rb b/spec/system/linked_comment_navigation_spec.rb new file mode 100644 index 00000000..5a903efb --- /dev/null +++ b/spec/system/linked_comment_navigation_spec.rb @@ -0,0 +1,56 @@ +require "rails_helper" + +RSpec.describe "Linked comment navigation", type: :system do + let(:author) { create(:coplan_user, email: "linked-comment-author@example.com") } + let(:plan) { create(:plan, created_by_user: author) } + + %w[light dark].each do |theme| + [ "inline", "document", "earlier text", "resolved earlier text" ].each do |kind| + it "keeps the linked #{kind} discussion open after rendering in #{theme} mode" do + author.update!(theme_preference: theme) + version = plan.current_plan_version + set_version_fixture(version, content_markdown: "# Review\n\nThe retry budget needs review.") + thread = plan.comment_threads.create!(plan_version: version, created_by_user: author, + anchor_text: kind == "document" ? nil : "retry budget", anchor_occurrence: 1) + thread.comments.create!(author_type: "human", author_id: author.id, body_markdown: "Can we extend the retry window?") + thread.update!(out_of_date: true, out_of_date_since_version: version) if kind.include?("earlier text") + thread.resolve!(author) if kind.start_with?("resolved") + + visit sign_in_path + fill_in "Email address", with: author.email + click_button "Sign In" + expect(page).to have_current_path(library_page_path(author)) + + visit plan_page_path(plan, thread: thread.id) + popover = "#comment_thread_#{thread.id}_popover:popover-open" + expect(page).to have_css(popover, text: "Can we extend the retry window?") + + # Async renderers announce this after rewriting the document DOM. + # The linked discussion must remain readable through every refresh. + 2.times do + page.execute_script(<<~JS) + document.querySelector("#plan-content-body").dispatchEvent( + new CustomEvent("coplan:highlight-settled", { bubbles: true }) + ) + JS + expect(page).to have_css(popover, text: "Can we extend the retry window?") + end + + if thread.open? + within(popover) { find("textarea").fill_in(with: "I can still reply.") } + expect(page).to have_field(with: "I can still reply.") + end + page.save_screenshot(Rails.root.join("tmp/linked-comments/#{kind.tr(' ', '-')}-#{theme}.png")) + + page.driver.browser.action.send_keys(:escape).perform + expect(page).to have_no_css(popover) + page.execute_script(<<~JS) + document.querySelector("#plan-content-body").dispatchEvent( + new CustomEvent("coplan:highlight-settled", { bubbles: true }) + ) + JS + expect(page).to have_no_css(popover) + end + end + end +end