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.

:warning: 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 <[email protected]>

Author: jjaffeux

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.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,
-      });
-    }
+    await this.currentMenu.close();
+
+    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();
+      },
+    });
 
-    this.showMainButtons = false;
-    this.menuState = this.MENU_STATES.options;
-  }
+    await this.currentMenu.destroy();
 
-  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,
-      },
-      post: this.args.outletArgs.post,
-      selectedText: this.selectedText,
-    };
+    this.highlightSelectedText();
   }
 
   <template>
-    {{#if this.showMainButtons}}
-      {{yield}}
-    {{/if}}
-
-    {{#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}}
+    {{yield}}
+
+    <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

@@ -73,6 +73,12 @@ def inject_into(plugin)
               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