Skip to content

Commit

Permalink
Fix dialog click outside early return (#2549)
Browse files Browse the repository at this point in the history
Co-authored-by: Cameron Dutro <[email protected]>
  • Loading branch information
keithamus and camertron authored Jan 31, 2024
1 parent c3260e1 commit 1259249
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 3 deletions.
5 changes: 5 additions & 0 deletions .changeset/small-flowers-refuse.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/view-components": patch
---

Ensure dialogs do not close when a child menu item (or similar) is clicked
8 changes: 6 additions & 2 deletions app/components/primer/dialog_helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ function dialogInvokerButtonHandler(event: 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()
}
}

Expand All @@ -35,7 +36,7 @@ export class DialogHelperElement extends HTMLElement {
#abortController: AbortController | null = null
connectedCallback() {
const {signal} = (this.#abortController = new AbortController())
document.addEventListener('click', dialogInvokerButtonHandler)
document.addEventListener('click', dialogInvokerButtonHandler, true)
document.addEventListener('click', this, {signal})
this.ownerDocument.body.style.setProperty(
'--dialog-scrollgutter',
Expand All @@ -58,7 +59,10 @@ export class DialogHelperElement extends HTMLElement {
const target = event.target as HTMLElement
const dialog = this.dialog
if (!dialog?.open) return
if (target?.closest('dialog') !== dialog) 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

const rect = dialog.getBoundingClientRect()
const clickWasInsideDialog =
Expand Down
3 changes: 2 additions & 1 deletion test/css/component_specific_selectors_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ class ComponentSpecificSelectorsTest < Minitest::Test
".Banner .Banner-close"
],
Primer::Alpha::Dialog => [
".Overlay"
".Overlay",
".has-modal"
],
Primer::Alpha::Layout => [
".Layout-divider",
Expand Down
19 changes: 19 additions & 0 deletions test/system/alpha/dialog_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,5 +74,24 @@ def test_with_scrollable_body
visit_preview(:long_text)
click_button("Show Dialog")
end

def test_outside_click_closes_dialog
visit_preview(:default)

click_button("Show Dialog")
page.driver.browser.mouse.click(x: 0, y: 0)

refute_selector "dialog[open]"
end

def test_outside_menu_click_does_not_close_dialog
visit_preview(:with_auto_complete)

click_button("Show Dialog")
fill_in "input", with: "a"

find(".ActionListItem", text: "Avocado").click
assert_selector "dialog[open]"
end
end
end

0 comments on commit 1259249

Please sign in to comment.