Use a mouseup mousedown event to guard accidental clicks dismissing dialog
ADD
.changeset/witty-mails-attend.md
+---
+'@primer/view-components': patch
+---
+
+Fix bug where clicking inside dialog, but finishing click outside, dismisses dialog

MODIFY
app/components/primer/alpha/modal_dialog.ts
app/components/primer/alpha/modal_dialog.ts
@@ -12,10 +12,11 @@ const overlayStack: ModalDialogElement[] = []
function clickHandler(event: Event) {
const target = event.target as HTMLElement
const button = target?.closest('button')
+ if (!button) return
// If the user is clicking a valid dialog trigger
let dialogId = button?.getAttribute('data-show-dialog-id')
- if (button && dialogId) {
+ if (dialogId) {
event.stopPropagation()
const dialog = document.getElementById(dialogId)
if (dialog instanceof ModalDialogElement) {
@@ -24,24 +25,10 @@ function clickHandler(event: Event) {
return
}
}
-
// Find the top level dialog that is open.
const topLevelDialog = overlayStack[overlayStack.length - 1]
if (!topLevelDialog) return
- // Check if the click happened outside the boundary of the top level dialog
- const clickOutsideDialog = !target.closest(`#${topLevelDialog.getAttribute('id')}`)
-
- // Only close dialog if it's a click outside the dialog and the dialog has a
- // button?
- if (!button) {
- if (clickOutsideDialog) {
- overlayStack.pop()
- topLevelDialog.close()
- }
- return
- }
-
dialogId = button.getAttribute('data-close-dialog-id')
if (dialogId === topLevelDialog.id) {
overlayStack.pop()
@@ -55,6 +42,32 @@ function clickHandler(event: Event) {
}
}
+function mousedownHandler(event: Event) {
+ const target = event.target as HTMLElement
+ if (target?.closest('button')) return
+
+ // Find the top level dialog that is open.
+ const topLevelDialog = overlayStack[overlayStack.length - 1]
+ if (!topLevelDialog) return
+
+ // Check if the mousedown happened outside the boundary of the top level dialog
+ const mouseDownOutsideDialog = !target.closest(`#${topLevelDialog.getAttribute('id')}`)
+
+ // Only close dialog if it's a click outside the dialog and the dialog has a button?
+ if (mouseDownOutsideDialog) {
+ target.ownerDocument.addEventListener(
+ 'mouseup',
+ (upEvent: Event) => {
+ if (upEvent.target === target) {
+ overlayStack.pop()
+ topLevelDialog.close()
+ }
+ },
+ {once: true}
+ )
+ }
+}
+
export class ModalDialogElement extends HTMLElement {
//TODO: Do we remove the abortController from focusTrap?
#focusAbortController = new AbortController()
@@ -109,6 +122,7 @@ export class ModalDialogElement extends HTMLElement {
if (!this.hasAttribute('role')) this.setAttribute('role', 'dialog')
document.addEventListener('click', clickHandler)
+ document.addEventListener('mousedown', mousedownHandler)
this.addEventListener('keydown', e => this.#keydown(e))
}

MODIFY
previews/primer/alpha/dialog_preview.rb
previews/primer/alpha/dialog_preview.rb
@@ -112,6 +112,21 @@ module Primer
show_divider: show_divider
})
end
+
+ # @label Dialog with text input
+ #
+ # @param title [String] text
+ # @param subtitle [String] text
+ # @param button_text [String] text
+ # @param show_divider [Boolean] toggle
+ def with_text_input(title: "Test Dialog", subtitle: nil, button_text: "Show Dialog", show_divider: true)
+ render_with_template(locals: {
+ title: title,
+ subtitle: subtitle,
+ button_text: button_text,
+ show_divider: show_divider
+ })
+ end
end
end
end

ADD
previews/primer/alpha/dialog_preview/with_text_input.html.erb
+<%= render(Primer::Alpha::Dialog.new(id: "dialog-one", title: title, subtitle: subtitle, visually_hide_title: false)) do |d| %>
+ <% d.show_button { button_text } %>
+ <% d.body do %>
+ <p>Dialog One!</p>
+
+ <form>
+ <input type="text" value="Some text goes in here">
+ </form>
+ <% end %>
+<% end %>

Use a mouseup mousedown event to guard accidental clicks dismissing dialog (#1754)