FEATURE: Allow TOC for replies (#90)

* FEATURE: Allow TOC for replies

This commit adds an optional setting that allows enabling a TOC for
replies. TOCs for replies are not affected by autoTOC settings like
`auto_TOC_tags` and must be inserted manually.

common/common.scss

@@ -215,7 +215,8 @@ html.rtl SELECTOR {
 }
 
 // Composer preview notice
-.edit-title .d-editor-preview [data-theme-toc] {
+.edit-title .d-editor-preview [data-theme-toc],
+body.toc-for-replies-enabled .d-editor-preview [data-theme-toc] {
   background: var(--tertiary);
   color: var(--secondary);
   position: sticky;

javascripts/discourse/components/toc-contents.gjs

@@ -4,7 +4,6 @@ import { action } from "@ember/object";
 import didInsert from "@ember/render-modifiers/modifiers/did-insert";
 import { service } from "@ember/service";
 import { headerOffset } from "discourse/lib/offset-calculator";
-import { slugify } from "discourse/lib/utilities";
 import { debounce } from "discourse-common/utils/decorators";
 import TocHeading from "../components/toc-heading";
 import TocLargeButtons from "../components/toc-large-buttons";
@@ -16,18 +15,20 @@ const RESIZE_DEBOUNCE = 200;
 
 export default class TocContents extends Component {
   @service tocProcessor;
+  @service appEvents;
 
   @tracked activeHeadingId = null;
   @tracked headingPositions = [];
   @tracked activeAncestorIds = [];
 
-  get flattenedToc() {
+  get mappedToc() {
-    return this.flattenTocStructure(this.args.tocStructure);
+    return this.mappedTocStructure(this.args.tocStructure);
   }
 
   @action
   setup() {
     this.listenForScroll();
+    this.listenForPostChange();
     this.listenForResize();
     this.updateHeadingPositions();
     this.updateActiveHeadingOnScroll(); // manual on setup so active class is added
@@ -37,6 +38,10 @@ export default class TocContents extends Component {
     super.willDestroy(...arguments);
     window.removeEventListener("scroll", this.updateActiveHeadingOnScroll);
     window.removeEventListener("resize", this.calculateHeadingPositions);
+    this.appEvents.off(
+      "topic:current-post-changed",
+      this.calculateHeadingPositions
+    );
   }
 
   @action
@@ -50,6 +55,14 @@ export default class TocContents extends Component {
     window.addEventListener("resize", this.calculateHeadingPositions);
   }
 
+  @action
+  listenForPostChange() {
+    this.appEvents.on(
+      "topic:current-post-changed",
+      this.calculateHeadingPositions
+    );
+  }
+
   @debounce(RESIZE_DEBOUNCE)
   calculateHeadingPositions() {
     this.updateHeadingPositions();
@@ -71,17 +84,27 @@ export default class TocContents extends Component {
       return;
     }
 
-    this.headingPositions = Array.from(headings).map((heading) => {
+    const sameIdCount = new Map();
-      const id = this.getIdFromHeading(heading);
+    const mappedToc = this.mappedToc;
-      return {
+    this.headingPositions = Array.from(headings)
+      .map((heading) => {
+        const id = this.tocProcessor.getIdFromHeading(
+          this.args.postID,
+          heading,
+          sameIdCount
+        );
+        return mappedToc[id]
+          ? {
               id,
               position:
                 heading.getBoundingClientRect().top +
                 window.scrollY -
                 headerOffset() -
                 POSITION_BUFFER,
-      };
+            }
-    });
+          : null;
+      })
+      .compact();
   }
 
   @debounce(SCROLL_DEBOUNCE)
@@ -104,9 +127,8 @@ export default class TocContents extends Component {
       }
     }
 
-    const activeHeading = this.flattenedToc.find(
+    const activeHeading =
-      (h) => h.id === this.headingPositions[activeIndex]?.id
+      this.mappedToc[this.headingPositions[activeIndex]?.id];
-    );
 
     this.activeHeadingId = activeHeading?.id;
     this.activeAncestorIds = [];
@@ -117,20 +139,15 @@ export default class TocContents extends Component {
     }
   }
 
-  getIdFromHeading(heading) {
+  mappedTocStructure(tocStructure, map = null) {
-    // reuse content from autolinked headings
+    map ??= {};
-    const tagName = heading.tagName.toLowerCase();
+    for (const item of tocStructure) {
-    const text = heading.textContent.trim();
+      map[item.id] = item;
-    const anchor = heading.querySelector("a.anchor");
+      if (item.subItems) {
-    return anchor ? anchor.name : `toc-${tagName}-${slugify(text)}`;
+        this.mappedTocStructure(item.subItems, map);
       }
-
+    }
-  flattenTocStructure(tocStructure) {
+    return map;
-    // the post content is flat, but we want to keep the relationships added in tocStructure
-    return tocStructure.flatMap((item) => [
-      item,
-      ...(item.subItems ? this.flattenTocStructure(item.subItems) : []),
-    ]);
   }
 
   <template>

javascripts/discourse/components/toc-heading.gjs

@@ -42,7 +42,9 @@ export default class TocHeading extends Component {
       return;
     }
 
-    const targetElement = document.querySelector(`a[name="${targetId}"]`);
+    const targetElement =
+      document.querySelector(`a[name="${targetId}"]`) ||
+      document.getElementById(targetId);
     if (targetElement) {
       const headerOffsetValue = headerOffset();
       const elementPosition =

javascripts/discourse/components/toc-mini.gjs

@@ -39,13 +39,13 @@ export default class TocMini extends Component {
 
   <template>
     {{#if this.tocProcessor.hasTOC}}
-      <div class="d-toc-mini">
+      <span class="d-toc-mini">
         <DButton
           class="btn-primary"
           @icon="stream"
           @action={{this.toggleTOCOverlay}}
         />
-      </div>
+      </span>
     {{/if}}
   </template>
 }

javascripts/discourse/initializers/disco-toc-composer.js

@@ -30,9 +30,15 @@ export default {
           icon: "align-left",
           label: themePrefix("insert_table_of_contents"),
           condition: (composer) => {
-            return composer.model.topicFirstPost;
+            return (
+              settings.enable_TOC_for_replies || composer.model.topicFirstPost
+            );
           },
         });
+
+        if (settings.enable_TOC_for_replies) {
+          document.body.classList.add("toc-for-replies-enabled");
+        }
       }
     });
   },

javascripts/discourse/services/toc-processor.js

@@ -64,7 +64,7 @@ export default class TocProcessor extends Service {
   }
 
   shouldDisplayToc(post) {
-    return post.post_number === 1;
+    return settings.enable_TOC_for_replies || post.post_number === 1;
   }
 
   containsTocMarkup(content) {
@@ -133,22 +133,42 @@ export default class TocProcessor extends Service {
     );
   }
 
+  /**
+   * @param {number} postId
+   * @param {HTMLHeadingElement} heading
+   * @param {Map<string, number>} sameIdCount
+   */
+  getIdFromHeading(postId, heading, sameIdCount) {
+    const anchor = heading.querySelector("a.anchor");
+    if (anchor) {
+      return anchor.name;
+    }
+    const lowerTagName = heading.tagName.toLowerCase();
+    const text = heading.textContent.trim();
+    let slug = `${slugify(text)}`;
+    if (sameIdCount.has(slug)) {
+      sameIdCount.set(slug, sameIdCount.get(slug) + 1);
+      slug = `${slug}-${sameIdCount.get(slug)}`;
+    } else {
+      sameIdCount.set(slug, 1);
+    }
+    const res = `p-${postId}-toc-${lowerTagName}-${slug}`;
+    heading.id = res;
+    return res;
+  }
+
   generateTocStructure(headings) {
     let root = { subItems: [], level: 0 };
     let ancestors = [root];
 
-    headings.forEach((heading, index) => {
+    const sameIdCount = new Map();
+
+    headings.forEach((heading) => {
       const level = parseInt(heading.tagName[1], 10);
       const text = heading.textContent.trim();
       const lowerTagName = heading.tagName.toLowerCase();
-      const anchor = heading.querySelector("a.anchor");
 
-      let id;
+      const id = this.getIdFromHeading(this.postID, heading, sameIdCount);
-      if (anchor) {
-        id = anchor.name;
-      } else {
-        id = `toc-${lowerTagName}-${slugify(text) || index}`;
-      }
 
       // Remove irrelevant ancestors
       while (ancestors[ancestors.length - 1].level >= level) {
@@ -172,7 +192,7 @@ export default class TocProcessor extends Service {
   }
 
   jumpToEnd(renderTimeline, postID) {
-    const buffer = 150;
+    let buffer = 150;
     const postContainer = document.querySelector(`[data-post-id="${postID}"]`);
 
     if (!renderTimeline) {
@@ -185,6 +205,15 @@ export default class TocProcessor extends Service {
       const topicMapHeight =
         postContainer.querySelector(`.topic-map`)?.offsetHeight || 0;
 
+      if (
+        postContainer.parentElement?.nextElementSibling?.querySelector(
+          "div[data-theme-toc]"
+        )
+      ) {
+        // but if the next post also has a toc, just jump to it
+        buffer = 30 - topicMapHeight;
+      }
+
       const offsetPosition =
         postContainer.getBoundingClientRect().bottom +
         window.scrollY -

locales/en.yml

@@ -12,3 +12,4 @@ en:
       auto_TOC_categories: Automatically enable TOC on topics in these categories
       auto_TOC_tags: Automatically enable TOC on topics with these tags
       TOC_min_heading: Minimum number of headings in a topic for the table of contents to be shown
+      enable_TOC_for_replies: Allows TOC for replies. TOCs for replies are not affected by the <b>auto TOC tags</b> and <b>auto TOC categories</b> settings and must be inserted manually.

settings.yml

@@ -16,6 +16,8 @@ auto_TOC_tags:
   type: list
   list_type: tag
   default: ""
+enable_TOC_for_replies:
+  default: false
 TOC_min_heading:
   default: 3
   min: 1

spec/system/discotoc_author_spec.rb

@@ -7,14 +7,17 @@ RSpec.describe "DiscoTOC", system: true do
   fab!(:user) { Fabricate(:user, trust_level: TrustLevel[1], refresh_auto_groups: true) }
 
   fab!(:topic_1) { Fabricate(:topic) }
-  fab!(:post_1) {
+  fab!(:post_1) do
-    Fabricate(:post, raw: "<div data-theme-toc='true'></div>\n\n# Heading 1\nContent for the first heading\n## Heading 2\nContent for the second heading\n### Heading 3\nContent for the third heading\n# Heading 4\nContent for the fourth heading", topic: topic_1)
+    Fabricate(
-  }
+      :post,
-
+      raw:
-  before do 
+        "<div data-theme-toc='true'></div>\n\n# Heading 1\nContent for the first heading\n## Heading 2\nContent for the second heading\n### Heading 3\nContent for the third heading\n# Heading 4\nContent for the fourth heading",
-    sign_in(user)
+      topic: topic_1,
+    )
   end
 
+  before { sign_in(user) }
+
   it "composer has table of contents button" do
     visit("/c/#{category.id}")
 
@@ -55,4 +58,19 @@ RSpec.describe "DiscoTOC", system: true do
     expect(page).to have_no_css("[data-name='Insert table of contents']")
   end
 
+  context "when enable TOC for replies" do
+    before do
+      theme.update_setting(:enable_TOC_for_replies, true)
+      theme.save!
+    end
+
+    it "table of contents button does appear on replies" do
+      visit("/t/#{topic_1.id}")
+
+      find(".reply").click
+      find(".toolbar-popup-menu-options").click
+
+      expect(page).to have_css("[data-name='Insert table of contents']")
+    end
+  end
 end

spec/system/discotoc_progress_user_spec.rb

@@ -6,20 +6,53 @@ RSpec.describe "DiscoTOC", system: true do
   fab!(:category)
   fab!(:tag)
 
-  fab!(:topic_1) { Fabricate(:topic) }
+  fab!(:topic_1) { Fabricate(:topic, category: category, tags: [tag]) }
   fab!(:topic_2) { Fabricate(:topic, category: category, tags: [tag]) }
 
-  fab!(:post_1) {
+  fab!(:post_1) do
-    Fabricate(:post, raw: "<div data-theme-toc='true'></div>\n\n# Heading 1\nContent for the first heading\n## Heading 2\nContent for the second heading\n### Heading 3\nContent for the third heading\n# Heading 4\nContent for the fourth heading", topic: topic_1)
+    Fabricate(
-  }
+      :post,
+      raw:
+        "<div data-theme-toc='true'></div>\n\n# Heading 1\nContent for the first heading\n## Heading 2\nContent for the second heading\n### Heading 3\nContent for the third heading\n# Heading 4\nContent for the fourth heading",
+      topic: topic_1,
+    )
+  end
 
-  fab!(:post_2) {
+  fab!(:post_2) do
-    Fabricate(:post, raw: "\n# Heading 1\nContent for the first heading\n## Heading 2\nContent for the second heading\n### Heading 3\nContent for the third heading\n# Heading 4\nContent for the fourth heading", topic: topic_2)
+    Fabricate(
-  }
+      :post,
+      raw:
+        "\n# Heading 1\nContent for the first heading\n## Heading 2\nContent for the second heading\n### Heading 3\nContent for the third heading\n# Heading 4\nContent for the fourth heading",
+      topic: topic_2,
+    )
+  end
 
-  fab!(:post_3) {
+  fab!(:post_3) do
-    Fabricate(:post, raw: "intentionally \n long \n content \n so \n there's \n plenty \n to be \n scrolled \n past \n which \n will \n force \n the \n timeline \n to \n hide \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll ", topic: topic_1)
+    Fabricate(
-  }
+      :post,
+      raw:
+        "intentionally \n long \n content \n so \n there's \n plenty \n to be \n scrolled \n past \n which \n will \n force \n the \n timeline \n to \n hide \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll ",
+      topic: topic_1,
+    )
+  end
+
+  fab!(:post_4) do
+    Fabricate(
+      :post,
+      raw:
+        "<div data-theme-toc='true'></div>\n\n# Heading For Reply 1\nContent for the first heading\n## Heading For Reply 2\nContent for the second heading\n### Heading For Reply 3\nContent for the third heading\n# Heading For Reply 4\nContent for the fourth heading",
+      topic: topic_1,
+    )
+  end
+
+  fab!(:post_5) do
+    Fabricate(
+      :post,
+      raw:
+        "intentionally \n long \n content \n so \n there's \n plenty \n to be \n scrolled \n past \n which \n will \n force \n the \n timeline \n to \n hide \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll ",
+      topic: topic_1,
+    )
+  end
 
   it "table of contents button appears in mobile view" do
     visit("/t/#{topic_1.id}/?mobile_view=1")
@@ -50,7 +83,7 @@ RSpec.describe "DiscoTOC", system: true do
       window.scrollTo(0, document.body.scrollHeight);
     JS
 
-    expect(page).to have_css(".d-toc-mini")
+    expect(page).to have_no_css(".d-toc-mini")
   end
 
   it "d-toc-mini does not appear if the first post does not contain the markup" do
@@ -68,6 +101,42 @@ RSpec.describe "DiscoTOC", system: true do
     expect(page).to have_css(".d-toc-mini")
   end
 
+  context "when disable TOC for replies" do
+    before do
+      theme.update_setting(:enable_TOC_for_replies, false)
+      theme.save!
+    end
+
+    it "table of contents button won't appears in mobile view for replies" do
+      visit("/t/-/#{topic_1.id}/3/?mobile_view=1")
+
+      expect(page).to have_no_css(".d-toc-mini")
+    end
+  end
+
+  context "when enable TOC for replies" do
+    before do
+      theme.update_setting(:enable_TOC_for_replies, true)
+      theme.save!
+    end
+
+    it "table of contents button appears in mobile view for replies" do
+      visit("/t/-/#{topic_1.id}/3/?mobile_view=1")
+
+      expect(page).to have_css(".d-toc-mini")
+    end
+
+    it "d-toc-mini will not appear without markup for replies regardless of auto_TOC_categories and auto_TOC_tags" do
+      theme.update_setting(:auto_TOC_categories, "#{category.id}")
+      theme.update_setting(:auto_TOC_tags, "#{tag.name}")
+      theme.save!
+
+      visit("/t/-/#{topic_1.id}/2/?mobile_view=1")
+
+      expect(page).to have_no_css(".d-toc-mini")
+    end
+  end
+
   it "d-toc-mini will not appear automatically if auto_TOC_categories is set to a different category" do
     theme.update_setting(:auto_TOC_categories, "99")
     theme.save!

spec/system/discotoc_timeline_user_spec.rb

@@ -6,20 +6,53 @@ RSpec.describe "DiscoTOC", system: true do
   fab!(:category)
   fab!(:tag)
 
-  fab!(:topic_1) { Fabricate(:topic) }
+  fab!(:topic_1) { Fabricate(:topic, category: category, tags: [tag]) }
   fab!(:topic_2) { Fabricate(:topic, category: category, tags: [tag]) }
 
-  fab!(:post_1) {
+  fab!(:post_1) do
-    Fabricate(:post, raw: "<div data-theme-toc='true'></div>\n\n# Heading 1\nContent for the first heading\n## Heading 2\nContent for the second heading\n### Heading 3\nContent for the third heading\n# Heading 4\nContent for the fourth heading", topic: topic_1)
+    Fabricate(
-  }
+      :post,
+      raw:
+        "<div data-theme-toc='true'></div>\n\n# Heading 1\nContent for the first heading\n## Heading 2\nContent for the second heading\n### Heading 3\nContent for the third heading\n# Heading 4\nContent for the fourth heading",
+      topic: topic_1,
+    )
+  end
 
-  fab!(:post_2) {
+  fab!(:post_2) do
-    Fabricate(:post, raw: "\n# Heading 1\nContent for the first heading\n## Heading 2\nContent for the second heading\n### Heading 3\nContent for the third heading\n# Heading 4\nContent for the fourth heading", topic: topic_2)
+    Fabricate(
-  }
+      :post,
+      raw:
+        "\n# Heading 1\nContent for the first heading\n## Heading 2\nContent for the second heading\n### Heading 3\nContent for the third heading\n# Heading 4\nContent for the fourth heading",
+      topic: topic_2,
+    )
+  end
 
-  fab!(:post_3) {
+  fab!(:post_3) do
-    Fabricate(:post, raw: "intentionally \n long \n content \n so \n there's \n plenty \n to be \n scrolled \n past \n which \n will \n force \n the \n timeline \n to \n hide \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll ", topic: topic_1)
+    Fabricate(
-  }
+      :post,
+      raw:
+        "intentionally \n long \n content \n so \n there's \n plenty \n to be \n scrolled \n past \n which \n will \n force \n the \n timeline \n to \n hide \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll ",
+      topic: topic_1,
+    )
+  end
+
+  fab!(:post_4) do
+    Fabricate(
+      :post,
+      raw:
+        "<div data-theme-toc='true'></div>\n\n# Heading For Reply 1\nContent for the first heading\n## Heading For Reply 2\nContent for the second heading\n### Heading For Reply 3\nContent for the third heading\n# Heading For Reply 4\nContent for the fourth heading",
+      topic: topic_1,
+    )
+  end
+
+  fab!(:post_5) do
+    Fabricate(
+      :post,
+      raw:
+        "intentionally \n long \n content \n so \n there's \n plenty \n to be \n scrolled \n past \n which \n will \n force \n the \n timeline \n to \n hide \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll \n scroll ",
+      topic: topic_1,
+    )
+  end
 
   it "table of contents appears when the relevant markup is added to first post in topic" do
     visit("/t/#{topic_1.id}")
@@ -105,4 +138,27 @@ RSpec.describe "DiscoTOC", system: true do
 
     expect(page).to have_no_css(".d-toc-item.d-toc-h1")
   end
+
+  context "when enable TOC for replies" do
+    before do
+      theme.update_setting(:enable_TOC_for_replies, true)
+      theme.save!
+    end
+
+    it "timeline does not appear for replies when the table of contents is shown" do
+      visit("/t/-/#{topic_1.id}/3")
+
+      expect(page).to have_no_css(".topic-timeline")
+    end
+
+    it "d-toc-mini will not appear without markup for replies regardless of auto_TOC_categories and auto_TOC_tags" do
+      theme.update_setting(:auto_TOC_categories, "#{category.id}")
+      theme.update_setting(:auto_TOC_tags, "#{tag.name}")
+      theme.save!
+
+      visit("/t/-/#{topic_1.id}/2")
+
+      expect(page).to have_no_css(".d-toc-item.d-toc-h1")
+    end
+  end
 end