ensure direct clicks to the dialog can only close the dialog (#2553)

Co-authored-by: Cameron Dutro <[email protected]>

.changeset/strange-windows-occur.md

@@ -0,0 +1,5 @@
+---
+"@primer/view-components": patch
+---
+
+Ensure only direct clicks to the dialog can close it

app/components/primer/dialog_helper.ts

@@ -7,15 +7,13 @@ function dialogInvokerButtonHandler(event: Event) {
   // If the user is clicking a valid dialog trigger
   let dialogId = button?.getAttribute('data-show-dialog-id')
   if (dialogId) {
-    event.stopPropagation()
     const dialog = document.getElementById(dialogId)
     if (dialog instanceof HTMLDialogElement) {
       dialog.showModal()
       // A buttons default behaviour in some browsers it to send a pointer event
       // If the behaviour is allowed through the dialog will be shown but then
       // quickly hidden- as if it were never shown. This prevents that.
       event.preventDefault()
-      event.stopPropagation()
     }
   }
 
@@ -72,11 +70,8 @@ export class DialogHelperElement extends HTMLElement {
   handleEvent(event: MouseEvent) {
     const target = event.target as HTMLElement
     const dialog = this.dialog
-    if (!dialog?.open) return
-
-    // if the target is inside the dialog, but is not the dialog itself, leave
-    // the dialog open
-    if (target?.closest('dialog') === dialog && target !== dialog) return
+    // The click target _must_ be the dialog element itself, and not elements underneath or inside.
+    if (target !== dialog || !dialog?.open) return
 
     const rect = dialog.getBoundingClientRect()
     const clickWasInsideDialog =

test/system/alpha/dialog_test.rb

@@ -94,6 +94,21 @@ def test_outside_menu_click_does_not_close_dialog
       assert_selector "dialog[open]"
     end
 
+    def test_click_events_can_be_added_to_invoker_buttons
+      # use this preview because it assigns a static ID to the invoker button
+      visit_preview(:with_header)
+
+      page.evaluate_script(<<~JS)
+        document.querySelector('#dialog-show-my-dialog').addEventListener('click', () => {
+          window.dialogInvokerClicked = true
+        })
+      JS
+
+      click_button("Show Dialog")
+
+      assert page.evaluate_script("window.dialogInvokerClicked"), "click event was not fired"
+    end
+
     def test_dialog_inside_overlay_opens_when_clicked
       visit_preview(:dialog_inside_overlay)