DEV: makes ai menu helper a standalone menu (#1434)

The current menu was rendering inside the post text toolbar (on desktop). This is not ideal as the post text toolbar rendering is conditioned on the presence of text selection, when you click a button on the toolbar, by design of the web browsers you will lose your text selection, making all of this super tricky.

This commit makes desktop and mobile behave in the same way by rendering their own menu and capturing the quote state when we render the post text selection toolbar, this allows us to reason a much simpler way about the AI helper.

This commit also removes what appears to be an unused file and corrects which was seemingly copy/paste mistakes.

⚠️ Technical note, this commit is correcting the message bus subscription which amongst other things allows to write specs which are not flaky. However due to the current implementation we have a channel per post, which means we need to serialize on last message bus id per post. 

We have two possible solutions here:
- subscribe at the topic level
- refactor the code to be able to use `MessageBus.last_ids` to be able to grab multiple posts at once instead of having to call `MessageBus.last_id` and done one Redis call per post

---------

Co-authored-by: Keegan George <kgeorge13@gmail.com>

assets/javascripts/discourse/components/ai-post-helper-menu.gjs

@@ -14,7 +14,6 @@ import concatClass from "discourse/helpers/concat-class";
 import { ajax } from "discourse/lib/ajax";
 import { popupAjaxError } from "discourse/lib/ajax-error";
 import { bind } from "discourse/lib/decorators";
-import { withPluginApi } from "discourse/lib/plugin-api";
 import { sanitize } from "discourse/lib/text";
 import { clipboardCopy } from "discourse/lib/utilities";
 import { i18n } from "discourse-i18n";
@@ -44,6 +43,9 @@ export default class AiPostHelperMenu extends Component {
   @tracked lastSelectedOption = null;
   @tracked isSavingFootnote = false;
   @tracked supportsAddFootnote = this.args.data.supportsFastEdit;
+  @tracked
+  channel =
+    `/discourse-ai/ai-helper/stream_suggestion/${this.args.data.quoteState.postId}`;
 
   @tracked
   smoothStreamer = new SmoothStreamer(
@@ -78,23 +80,6 @@ export default class AiPostHelperMenu extends Component {
 
   @tracked _activeAiRequest = null;
 
-  constructor() {
-    super(...arguments);
-
-    withPluginApi((api) => {
-      api.registerValueTransformer(
-        "post-text-selection-prevent-close",
-        ({ value }) => {
-          if (this.menuState === this.MENU_STATES.result) {
-            return true;
-          }
-
-          return value;
-        }
-      );
-    });
-  }
-
   get footnoteDisabled() {
     return this.streaming || !this.supportsAddFootnote;
   }
@@ -167,16 +152,17 @@ export default class AiPostHelperMenu extends Component {
 
   @bind
   subscribe() {
-    const channel = `/discourse-ai/ai-helper/stream_suggestion/${this.args.data.quoteState.postId}`;
-    this.messageBus.subscribe(channel, this._updateResult);
+    this.messageBus.subscribe(
+      this.channel,
+      (data) => this._updateResult(data),
+      this.args.data.post
+        .discourse_ai_helper_stream_suggestion_last_message_bus_id
+    );
   }
 
   @bind
   unsubscribe() {
-    this.messageBus.unsubscribe(
-      "/discourse-ai/ai-helper/stream_suggestion/*",
-      this._updateResult
-    );
+    this.messageBus.unsubscribe(this.channel, this._updateResult);
   }
 
   @bind
@@ -239,7 +225,7 @@ export default class AiPostHelperMenu extends Component {
       data: {
         location: "post",
         mode: option.name,
-        text: this.args.data.selectedText,
+        text: this.args.data.quoteState.buffer,
         post_id: this.args.data.quoteState.postId,
         custom_prompt: this.customPromptValue,
         client_id: this.messageBus.clientId,
@@ -292,12 +278,10 @@ export default class AiPostHelperMenu extends Component {
 
   @action
   closeMenu() {
-    if (this.site.mobileView) {
-      return this.args.close();
-    }
-
-    const menu = this.menu.getByIdentifier("post-text-selection-toolbar");
-    return menu?.close();
+    // reset state and close
+    this.suggestion = "";
+    this.customPromptValue = "";
+    return this.args.close();
   }
 
   @action
@@ -317,9 +301,9 @@ export default class AiPostHelperMenu extends Component {
         const credits = i18n(
           "discourse_ai.ai_helper.post_options_menu.footnote_credits"
         );
-        const withFootnote = `${this.args.data.selectedText} ^[${sanitizedSuggestion} (${credits})]`;
+        const withFootnote = `${this.args.data.quoteState.buffer} ^[${sanitizedSuggestion} (${credits})]`;
         const newRaw = result.raw.replace(
-          this.args.data.selectedText,
+          this.args.data.quoteState.buffer,
           withFootnote
         );
 
@@ -338,7 +322,7 @@ export default class AiPostHelperMenu extends Component {
       (and this.site.mobileView (eq this.menuState this.MENU_STATES.options))
     }}
       <div class="ai-post-helper-menu__selected-text">
-        {{@data.selectedText}}
+        {{@data.quoteState.buffer}}
       </div>
     {{/if}}
 

assets/javascripts/discourse/connectors/post-text-buttons/ai-post-helper-trigger.gjs

@@ -3,8 +3,7 @@ import { tracked } from "@glimmer/tracking";
 import { action } from "@ember/object";
 import { service } from "@ember/service";
 import DButton from "discourse/components/d-button";
-import virtualElementFromTextRange from "discourse/lib/virtual-element-from-text-range";
-import eq from "truth-helpers/helpers/eq";
+import { selectedRange } from "discourse/lib/utilities";
 import AiPostHelperMenu from "../../components/ai-post-helper-menu";
 import { showPostAIHelper } from "../../lib/show-ai-helper";
 
@@ -13,27 +12,27 @@ export default class AiPostHelperTrigger extends Component {
     return showPostAIHelper(outletArgs, helper);
   }
 
-  @service site;
   @service menu;
 
-  @tracked menuState = this.MENU_STATES.triggers;
-  @tracked showMainButtons = true;
-  @tracked showAiButtons = true;
   @tracked postHighlighted = false;
   @tracked currentMenu = this.menu.getByIdentifier(
     "post-text-selection-toolbar"
   );
 
-  MENU_STATES = {
-    triggers: "TRIGGERS",
-    options: "OPTIONS",
+  // capture the state at the moment the toolbar is rendered
+  // so we ensure change of state (selection change for example)
+  // is not impacting the menu data
+  menuData = {
+    ...this.args.outletArgs.data,
+    quoteState: {
+      buffer: this.args.outletArgs.data.quoteState.buffer,
+      opts: this.args.outletArgs.data.quoteState.opts,
+      postId: this.args.outletArgs.data.quoteState.postId,
+    },
+    post: this.args.outletArgs.post,
+    selectedRange: selectedRange(),
   };
 
-  willDestroy() {
-    super.willDestroy(...arguments);
-    this.removeHighlightedText();
-  }
-
   highlightSelectedText() {
     const postId = this.args.outletArgs.data.quoteState.postId;
     const postElement = document.querySelector(
@@ -44,14 +43,7 @@ export default class AiPostHelperTrigger extends Component {
       return;
     }
 
-    this.selectedText = this.args.outletArgs.data.quoteState.buffer;
-
-    const selection = window.getSelection();
-    if (!selection.rangeCount) {
-      return;
-    }
-
-    const range = selection.getRangeAt(0);
+    const range = this.menuData.selectedRange;
 
     // Split start/end text nodes at their range boundary
     if (
@@ -97,11 +89,10 @@ export default class AiPostHelperTrigger extends Component {
       // Replace textNode with highlighted clone
       const clone = textNode.cloneNode(true);
       highlight.appendChild(clone);
-
       textNode.parentNode.replaceChild(highlight, textNode);
     }
 
-    selection.removeAllRanges();
+    window.getSelection().removeAllRanges();
     this.postHighlighted = true;
   }
 
@@ -110,16 +101,7 @@ export default class AiPostHelperTrigger extends Component {
       return;
     }
 
-    const postId = this.args.outletArgs.data.quoteState.postId;
-    const postElement = document.querySelector(
-      `article[data-post-id='${postId}'] .cooked`
-    );
-
-    if (!postElement) {
-      return;
-    }
-
-    const highlightedSpans = postElement.querySelectorAll(
+    const highlightedSpans = document.querySelectorAll(
       "span.ai-helper-highlighted-selection"
     );
 
@@ -133,65 +115,40 @@ export default class AiPostHelperTrigger extends Component {
 
   @action
   async showAiPostHelperMenu() {
-    this.highlightSelectedText();
-    if (this.site.mobileView) {
-      this.currentMenu.close();
+    await this.currentMenu.close();
 
-      await this.menu.show(virtualElementFromTextRange(), {
-        identifier: "ai-post-helper-menu",
-        component: AiPostHelperMenu,
-        inline: true,
-        interactive: true,
-        placement: this.shouldRenderUnder ? "bottom-start" : "top-start",
-        fallbackPlacements: this.shouldRenderUnder
-          ? ["bottom-end", "top-start"]
-          : ["bottom-start"],
-        trapTab: false,
-        closeOnScroll: false,
-        modalForMobile: true,
-        data: this.menuData,
-      });
-    }
-
-    this.showMainButtons = false;
-    this.menuState = this.MENU_STATES.options;
-  }
-
-  get menuData() {
-    // Streamline of data model to be passed to the component when
-    // instantiated as a DMenu or a simple component in the template
-    return {
-      ...this.args.outletArgs.data,
-      quoteState: {
-        buffer: this.args.outletArgs.data.quoteState.buffer,
-        opts: this.args.outletArgs.data.quoteState.opts,
-        postId: this.args.outletArgs.data.quoteState.postId,
+    await this.menu.show(this.currentMenu.trigger, {
+      identifier: "ai-post-helper-menu",
+      component: AiPostHelperMenu,
+      interactive: true,
+      trapTab: false,
+      closeOnScroll: false,
+      modalForMobile: true,
+      data: this.menuData,
+      placement: "top-start",
+      fallbackPlacements: ["bottom-start"],
+      updateOnScroll: false,
+      onClose: () => {
+        this.removeHighlightedText();
       },
-      post: this.args.outletArgs.post,
-      selectedText: this.selectedText,
-    };
+    });
+
+    await this.currentMenu.destroy();
+
+    this.highlightSelectedText();
   }
 
   <template>
-    {{#if this.showMainButtons}}
-      {{yield}}
-    {{/if}}
+    {{yield}}
 
-    {{#if this.showAiButtons}}
-      <div class="ai-post-helper">
-        {{#if (eq this.menuState this.MENU_STATES.triggers)}}
-          <DButton
-            @icon="discourse-sparkles"
-            @title="discourse_ai.ai_helper.post_options_menu.title"
-            @label="discourse_ai.ai_helper.post_options_menu.trigger"
-            @action={{this.showAiPostHelperMenu}}
-            class="btn-flat ai-post-helper__trigger"
-          />
-
-        {{else if (eq this.menuState this.MENU_STATES.options)}}
-          <AiPostHelperMenu @data={{this.menuData}} />
-        {{/if}}
-      </div>
-    {{/if}}
+    <div class="ai-post-helper">
+      <DButton
+        @icon="discourse-sparkles"
+        @title="discourse_ai.ai_helper.post_options_menu.title"
+        @label="discourse_ai.ai_helper.post_options_menu.trigger"
+        @action={{this.showAiPostHelperMenu}}
+        class="btn-flat ai-post-helper__trigger"
+      />
+    </div>
   </template>
 }

assets/javascripts/discourse/lib/virtual-element-from-caret-coords.js

@@ -1,45 +0,0 @@
-class VirtualElementFromCaretCoords {
-  constructor(caretCoords, offset = [0, 0]) {
-    this.caretCoords = caretCoords;
-    this.offset = offset;
-    this.updateRect();
-  }
-
-  updateRect() {
-    const [xOffset, yOffset] = this.offset;
-    this.rect = {
-      top: this.caretCoords.y + yOffset,
-      right: this.caretCoords.x,
-      bottom: this.caretCoords.y + yOffset,
-      left: this.caretCoords.x + xOffset,
-      width: 0,
-      height: 0,
-      x: this.caretCoords.x,
-      y: this.caretCoords.y,
-      toJSON() {
-        return this;
-      },
-    };
-    return this.rect;
-  }
-
-  getBoundingClientRect() {
-    return this.rect;
-  }
-
-  getClientRects() {
-    return [this.rect];
-  }
-
-  get clientWidth() {
-    return this.rect.width;
-  }
-
-  get clientHeight() {
-    return this.rect.height;
-  }
-}
-
-export default function virtualElementFromCaretCoords(caretCoords, offset) {
-  return new VirtualElementFromCaretCoords(caretCoords, offset);
-}

lib/ai_helper/entry_point.rb

@@ -73,6 +73,12 @@ module DiscourseAi
               scope.user.in_any_groups?(SiteSetting.ai_auto_image_caption_allowed_groups_map)
           end,
         ) { object.auto_image_caption }
+
+        plugin.add_to_serializer(
+          :post,
+          :discourse_ai_helper_stream_suggestion_last_message_bus_id,
+          include_condition: -> { SiteSetting.ai_helper_enabled && scope.authenticated? },
+        ) { MessageBus.last_id("/discourse-ai/ai-helper/stream_suggestion/#{object.id}") }
       end
     end
   end

spec/system/ai_helper/ai_post_helper_spec.rb

@@ -30,6 +30,7 @@ RSpec.describe "AI Post helper", type: :system, js: true do
     Group.find_by(id: Group::AUTO_GROUPS[:admins]).add(user)
     assign_fake_provider_to(:ai_helper_model)
     SiteSetting.ai_helper_enabled = true
+    Jobs.run_immediately!
     sign_in(user)
   end
 
@@ -87,13 +88,52 @@ RSpec.describe "AI Post helper", type: :system, js: true do
       end
 
       it "pre-fills fast edit with proofread text" do
-        skip("Test is flaky in CI, possibly some timing issue?") if ENV["CI"]
         select_post_text(post_3)
         post_ai_helper.click_ai_button
         DiscourseAi::Completions::Llm.with_prepared_responses([proofread_response]) do
           post_ai_helper.select_helper_model(mode)
-          wait_for { fast_editor.has_content?(proofread_response) }
-          expect(fast_editor).to have_content(proofread_response)
+          expect(page.find("#fast-edit-input")).to have_content(proofread_response)
+        end
+      end
+    end
+
+    context "when using explain mode" do
+      let(:mode) { DiscourseAi::AiHelper::Assistant::EXPLAIN }
+      let(:explain_response) { "This is about pie." }
+
+      it "shows the explanation in the AI helper" do
+        select_post_text(post)
+        post_ai_helper.click_ai_button
+
+        DiscourseAi::Completions::Llm.with_prepared_responses([explain_response]) do
+          post_ai_helper.select_helper_model(mode)
+          expect(post_ai_helper).to have_suggestion_value(explain_response)
+        end
+      end
+
+      context "with footnotes enabled" do
+        before do
+          SiteSetting.enable_markdown_footnotes = true
+          SiteSetting.display_footnotes_inline = true
+        end
+
+        it "allows adding the explanation as a footnote to the post" do
+          select_post_text(post)
+          post_ai_helper.click_ai_button
+
+          DiscourseAi::Completions::Llm.with_prepared_responses([explain_response]) do
+            post_ai_helper.select_helper_model(mode)
+
+            expect(post_ai_helper).to have_suggestion_value(explain_response)
+
+            post_ai_helper.click_add_footnote
+
+            expect(post_ai_helper).to have_no_post_ai_helper
+
+            expect(post.reload.raw).to include(
+              "^[#{explain_response} (#{I18n.t("js.discourse_ai.ai_helper.post_options_menu.footnote_credits")})]",
+            )
+          end
         end
       end
     end
@@ -128,13 +168,11 @@ RSpec.describe "AI Post helper", type: :system, js: true do
     end
 
     it "pre-fills fast edit with proofread text" do
-      skip("Test is flaky in CI, possibly some timing issue?") if ENV["CI"]
       select_post_text(post_3)
       find(".quote-edit-label").click
       DiscourseAi::Completions::Llm.with_prepared_responses([proofread_response]) do
         find(".btn-ai-suggest-edit", visible: :all).click
-        wait_for { fast_editor.has_content?(proofread_response) }
-        expect(fast_editor).to have_content(proofread_response)
+        expect(page.find("#fast-edit-input")).to have_content(proofread_response)
       end
     end
   end

spec/system/page_objects/components/ai_post_helper_menu.rb

@@ -9,7 +9,6 @@ module PageObjects
       SHARE_SELECTOR = ".quote-sharing"
 
       AI_HELPER_SELECTOR = ".ai-post-helper"
-      AI_HELPER_MOBILE_SELECTOR = ".ai-post-helper-menu-content"
       TRIGGER_SELECTOR = "#{AI_HELPER_SELECTOR}__trigger"
       OPTIONS_SELECTOR = ".ai-helper-options"
       LOADING_SELECTOR = ".ai-helper-context-menu__loading"
@@ -28,6 +27,10 @@ module PageObjects
         find("#{OPTIONS_SELECTOR} .btn[data-name=\"#{mode}\"]").click
       end
 
+      def has_suggestion_value?(value)
+        page.has_css?("#{SUGGESTION_SELECTOR}__text", text: value)
+      end
+
       def suggestion_value
         find("#{SUGGESTION_SELECTOR}__text").text
       end
@@ -41,11 +44,11 @@ module PageObjects
       end
 
       def has_mobile_post_ai_helper?
-        page.has_css?(AI_HELPER_MOBILE_SELECTOR)
+        page.has_css?(".fk-d-menu-modal #{AI_HELPER_SELECTOR}")
       end
 
       def has_no_mobile_post_ai_helper?
-        page.has_no_css?(AI_HELPER_MOBILE_SELECTOR)
+        page.has_no_css?(".fk-d-menu-modal #{AI_HELPER_SELECTOR}")
       end
 
       def has_post_ai_helper?
@@ -81,6 +84,10 @@ module PageObjects
         page.has_no_css?(QUOTE_SELECTOR) || page.has_no_css?(EDIT_SELECTOR) ||
           page.has_no_css?(SHARE_SELECTOR)
       end
+
+      def has_suggestions?
+        page.has_css?(SUGGESTION_SELECTOR)
+      end
     end
   end
 end