allow dialogs within deferred menus (#2212)

.changeset/smart-scissors-type.md

@@ -0,0 +1,7 @@
+---
+'@primer/view-components': patch
+---
+
+Fix dialog invocation within deferred ActionMenus
+
+<!-- Changed components: Primer::Alpha::ActionMenu -->

app/components/primer/alpha/action_menu/action_menu_element.ts

@@ -13,7 +13,8 @@ const menuItemSelectors = ['[role="menuitem"]', '[role="menuitemcheckbox"]', '[r
 
 @controller
 export class ActionMenuElement extends HTMLElement {
-  @target includeFragment: IncludeFragmentElement
+  @target
+  includeFragment: IncludeFragmentElement
 
   #abortController: AbortController
   #originalLabel = ''
@@ -57,7 +58,9 @@ export class ActionMenuElement extends HTMLElement {
     const id = this.querySelector('[role=menu]')?.id
     if (!id) return null
     for (const el of this.querySelectorAll(`[aria-controls]`)) {
-      if (el.getAttribute('aria-controls') === id) return el as HTMLButtonElement
+      if (el.getAttribute('aria-controls') === id) {
+        return el as HTMLButtonElement
+      }
     }
     return null
   }
@@ -94,7 +97,9 @@ export class ActionMenuElement extends HTMLElement {
     this.#updateInput()
 
     if (this.includeFragment) {
-      this.includeFragment.addEventListener('include-fragment-replaced', this, {signal})
+      this.includeFragment.addEventListener('include-fragment-replaced', this, {
+        signal
+      })
     }
   }
 
@@ -103,7 +108,8 @@ export class ActionMenuElement extends HTMLElement {
   }
 
   handleEvent(event: Event) {
-    if (event.target === this.invokerElement && this.#isActivationKeydown(event)) {
+    const activation = this.#isActivationKeydown(event)
+    if (event.target === this.invokerElement && activation) {
       if (this.#firstItem) {
         event.preventDefault()
         this.popoverElement?.showPopover()
@@ -112,11 +118,39 @@ export class ActionMenuElement extends HTMLElement {
       }
     }
 
+    // Ignore events within dialogs within menus
+    if ((event.target as Element)?.closest('dialog') || (event.target as Element)?.closest('modal-dialog')) {
+      return
+    }
+
+    // If a dialog has been rendered within the menu, we do not want to hide
+    // the entire menu, as that will also hide the Dialog. Instead we want to
+    // show the Dialog while hiding just the visible part of the menu.
+    if ((activation || event.type === 'click') && (event.target as HTMLElement)?.closest('[data-show-dialog-id]')) {
+      const dialogInvoker = (event.target as HTMLElement)!.closest('[data-show-dialog-id]')
+      const dialog = this.ownerDocument.getElementById(dialogInvoker?.getAttribute('data-show-dialog-id') || '')
+      if (dialogInvoker && dialog && this.contains(dialogInvoker) && this.contains(dialog)) {
+        this.querySelector<HTMLElement>('.ActionListWrap')!.style.display = 'none'
+        const dialog_controller = new AbortController()
+        const {signal} = dialog_controller
+        const handleDialogClose = () => {
+          dialog_controller.abort()
+          this.querySelector<HTMLElement>('.ActionListWrap')!.style.display = ''
+          if (this.popoverElement?.matches(':popover-open')) {
+            this.popoverElement?.hidePopover()
+          }
+        }
+        dialog.addEventListener('close', handleDialogClose, {signal})
+        dialog.addEventListener('cancel', handleDialogClose, {signal})
+        return
+      }
+    }
+
     if (!this.popoverElement?.matches(':popover-open')) return
 
     if (event.type === 'include-fragment-replaced') {
       if (this.#firstItem) this.#firstItem.focus()
-    } else if (this.#isActivationKeydown(event) || (event instanceof MouseEvent && event.type === 'click')) {
+    } else if (activation || (event instanceof MouseEvent && event.type === 'click')) {
       // Hide popover after current event loop to prevent changes in focus from
       // altering the target of the event. Not doing this specifically affects
       // <a> tags. It causes the event to be sent to the currently focused element

demo/app/views/action_menu/deferred.html.erb

@@ -2,4 +2,22 @@
   <% list.with_item(label: "Copy link", value: "", autofocus: true) %>
   <% list.with_item(label: "Quote reply", value: "") %>
   <% list.with_item(label: "Reference in new issue", value: "") %>
+  <% list.with_item(
+    label: "Show dialog",
+    tag: :button,
+    content_arguments: { "data-show-dialog-id": "my-dialog" },
+    value: "",
+    scheme: :danger
+  ) %>
+<% end %>
+
+
+<%= render(Primer::Alpha::Dialog.new(id: "my-dialog", title: "Confirm deletion")) do |d| %>
+  <%= render(Primer::Alpha::Dialog::Body.new()) do %>
+    Are you sure you want to delete this?
+  <% end %>
+  <%= render(Primer::Alpha::Dialog::Footer.new()) do %>
+    <%= render(Primer::Beta::Button.new(data: { "close-dialog-id": "my-dialog" })) { "Cancel" } %>
+    <%= render(Primer::Beta::Button.new(scheme: :danger)) { "Delete" } %>
+  <% end %>
 <% end %>

test/system/alpha/action_menu_test.rb

@@ -265,6 +265,16 @@ def test_deferred_loading_on_keydown
       assert_equal page.evaluate_script("document.activeElement").text, "Copy link"
     end
 
+    def test_deferred_dialog_opens
+      visit_preview(:with_deferred_content)
+
+      find("action-menu button[aria-controls]").click
+
+      find("action-menu ul li:nth-child(4)").click
+
+      assert_selector "modal-dialog[open]"
+    end
+
     def test_opening_second_menu_closes_first_menu
       visit_preview(:two_menus)