Fix issue where dialog is unable to be closed (#2288)

.changeset/eleven-steaks-press.md

@@ -0,0 +1,7 @@
+---
+'@primer/view-components': patch
+---
+
+Fixes issue where sometimes a dialog cannot be closed if another is open
+
+<!-- Changed components: Primer::Alpha::Dialog -->

app/components/primer/alpha/modal_dialog.ts

@@ -30,20 +30,17 @@ function clickHandler(event: Event) {
       return
     }
   }
-  // Find the top level dialog that is open.
-  const topLevelDialog = overlayStack[overlayStack.length - 1]
-  if (!topLevelDialog) return
 
-  dialogId = button.getAttribute('data-close-dialog-id')
-  if (dialogId === topLevelDialog.id) {
-    overlayStack.pop()
-    topLevelDialog.close()
-  }
+  if (!overlayStack.length) return
 
-  dialogId = button.getAttribute('data-submit-dialog-id')
-  if (dialogId === topLevelDialog.id) {
-    overlayStack.pop()
-    topLevelDialog.close(true)
+  dialogId = button.getAttribute('data-close-dialog-id') || button.getAttribute('data-submit-dialog-id')
+  if (dialogId) {
+    const dialog = document.getElementById(dialogId)
+    if (dialog instanceof ModalDialogElement) {
+      const dialogIndex = overlayStack.findIndex(ele => ele.id === dialogId)
+      overlayStack.splice(dialogIndex, 1)
+      dialog.close(button.hasAttribute('data-submit-dialog-id'))
+    }
   }
 }
 

test/system/alpha/dialog_test.rb

@@ -4,6 +4,18 @@
 
 module Alpha
   class IntegrationDialogTest < System::TestCase
+    def click_on_initial_dialog_close_button
+      find("button[data-close-dialog-id='dialog-one']").trigger("click")
+    end
+
+    def click_on_nested_dialog_close_button
+      find("button[data-close-dialog-id='dialog-two']").click
+    end
+
+    def click_on_nested_dialog_button
+      find("#dialog-show-dialog-two").click
+    end
+
     def test_modal_has_accessible_name
       visit_preview(:default)
 
@@ -20,5 +32,32 @@ def test_focuses_close_button
 
       assert_equal page.evaluate_script("document.activeElement")["aria-label"], "Close"
     end
+
+    def test_closes_top_level_dialog
+      visit_preview(:nested_dialog)
+
+      click_button("Show Dialog")
+      click_on_nested_dialog_button
+
+      assert_equal(find("modal-dialog#dialog-two")["open"], true)
+
+      click_on_nested_dialog_close_button
+
+      assert_selector "modal-dialog#dialog-two", visible: :hidden
+      assert_selector "modal-dialog#dialog-one"
+    end
+
+    def test_closes_dialog_that_is_not_top_level
+      visit_preview(:nested_dialog)
+
+      click_button("Show Dialog")
+      click_on_nested_dialog_button
+
+      assert_equal(find("modal-dialog#dialog-two")["open"], true)
+
+      click_on_initial_dialog_close_button
+
+      assert_selector "modal-dialog#dialog-one", visible: :hidden
+    end
   end
 end