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() {
-    return this.flattenTocStructure(this.args.tocStructure);
+  get mappedToc() {
+    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 id = this.getIdFromHeading(heading);
-      return {
-        id,
-        position:
-          heading.getBoundingClientRect().top +
-          window.scrollY -
-          headerOffset() -
-          POSITION_BUFFER,
-      };
-    });
+    const sameIdCount = new Map();
+    const mappedToc = this.mappedToc;
+    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(
-      (h) => h.id === this.headingPositions[activeIndex]?.id
-    );
+    const activeHeading =
+      this.mappedToc[this.headingPositions[activeIndex]?.id];
 
     this.activeHeadingId = activeHeading?.id;
     this.activeAncestorIds = [];
@@ -117,20 +139,15 @@ export default class TocContents extends Component {
     }
   }
 
-  getIdFromHeading(heading) {
-    // reuse content from autolinked headings
-    const tagName = heading.tagName.toLowerCase();
-    const text = heading.textContent.trim();
-    const anchor = heading.querySelector("a.anchor");
-    return anchor ? anchor.name : `toc-${tagName}-${slugify(text)}`;
-  }
-
-  flattenTocStructure(tocStructure) {
-    // 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) : []),
-    ]);
+  mappedTocStructure(tocStructure, map = null) {
+    map ??= {};
+    for (const item of tocStructure) {
+      map[item.id] = item;
+      if (item.subItems) {
+        this.mappedTocStructure(item.subItems, map);
+      }
+    }
+    return map;
   }
 
   <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;
-      if (anchor) {
-        id = anchor.name;
-      } else {
-        id = `toc-${lowerTagName}-${slugify(text) || index}`;
-      }
+      const id = this.getIdFromHeading(this.postID, heading, sameIdCount);
 
       // 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 @@
   fab!(:user) { Fabricate(:user, trust_level: TrustLevel[1], refresh_auto_groups: true) }
 
   fab!(:topic_1) { Fabricate(:topic) }
-  fab!(:post_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)
-  }
-
-  before do 
-    sign_in(user)
+  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,
+    )
   end
 
+  before { sign_in(user) }
+
   it "composer has table of contents button" do
     visit("/c/#{category.id}")
 
@@ -35,7 +38,7 @@
   end
 
   it "table of contents button is hidden by trust level setting" do
-    theme.update_setting(:minimum_trust_level_to_create_TOC, "2" )
+    theme.update_setting(:minimum_trust_level_to_create_TOC, "2")
     theme.save!
 
     visit("/c/#{category.id}")
@@ -54,5 +57,20 @@
 
     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