diff --git a/.changeset/eleven-steaks-press.md b/.changeset/eleven-steaks-press.md new file mode 100644 index 0000000000..b11f88cc19 --- /dev/null +++ b/.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 + + diff --git a/app/components/primer/alpha/modal_dialog.ts b/app/components/primer/alpha/modal_dialog.ts index aad3b0f510..62165c3810 100644 --- a/app/components/primer/alpha/modal_dialog.ts +++ b/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')) + } } } diff --git a/test/system/alpha/dialog_test.rb b/test/system/alpha/dialog_test.rb index 7238074605..b16b65e12f 100644 --- a/test/system/alpha/dialog_test.rb +++ b/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