Skip to content

Commit

Permalink
refactor Dialog to use <dialog> internally (#2364)
Browse files Browse the repository at this point in the history
  • Loading branch information
keithamus authored Dec 8, 2023
1 parent 65f418f commit 6c45a4b
Show file tree
Hide file tree
Showing 18 changed files with 151 additions and 245 deletions.
5 changes: 5 additions & 0 deletions .changeset/wise-coats-buy.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/view-components': minor
---

Primer::Alpha::Dialog uses <dialog> internally
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,15 @@ export class ActionMenuElement extends HTMLElement {
if (this.#isOpen()) {
this.#hide()
}
const activeElement = this.ownerDocument.activeElement
const lostFocus = this.ownerDocument.activeElement === this.ownerDocument.body
const focusInClosedMenu = this.contains(activeElement)
if (lostFocus || focusInClosedMenu) {
setTimeout(() => this.invokerElement?.focus(), 0)
}
}
// a modal <dialog> element will close all popovers
setTimeout(() => this.#show(), 0)
dialog.addEventListener('close', handleDialogClose, {signal})
dialog.addEventListener('cancel', handleDialogClose, {signal})
}
Expand Down
4 changes: 2 additions & 2 deletions app/components/primer/alpha/dialog.html.erb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<%= show_button %>
<div class="Overlay--hidden <%= @backdrop_classes %>" data-modal-dialog-overlay>
<dialog-helper>
<%= render Primer::BaseComponent.new(**@system_arguments) do %>
<%= header %>
<% if content.present? %>
Expand All @@ -9,4 +9,4 @@
<%= footer %>
<% end %>
<% end %>
</div>
</dialog-helper>
10 changes: 10 additions & 0 deletions app/components/primer/alpha/dialog.pcss
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
/* Overlay */

dialog.Overlay:not([open]) {
display: none;
}

/* dialog defualts to CanvasText not inherit */
dialog.Overlay {
color: var(--fgColor-default);
}


.Overlay--hidden {
display: none !important;
}
Expand Down
3 changes: 1 addition & 2 deletions app/components/primer/alpha/dialog.rb
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,7 @@ def initialize(
@position_narrow = position_narrow
@visually_hide_title = visually_hide_title

@system_arguments[:tag] = "modal-dialog"
@system_arguments[:role] = "dialog"
@system_arguments[:tag] = "dialog"
@system_arguments[:id] = @id
@system_arguments[:aria] = { modal: true }
@system_arguments[:aria] = merge_aria(
Expand Down
199 changes: 0 additions & 199 deletions app/components/primer/alpha/modal_dialog.ts

This file was deleted.

9 changes: 8 additions & 1 deletion app/components/primer/alpha/overlay.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@ anchored-position[popover] {
overflow: visible;
}

.Overlay {
display: flex;
border-width: 0;
padding: 0;
}

anchored-position:not(.Overlay) {
background: none;
}
Expand All @@ -15,6 +21,7 @@ anchored-position:not(.Overlay) {
display: none
}

anchored-position.not-anchored::backdrop {
anchored-position.not-anchored::backdrop, dialog::backdrop {
background-color: var(--overlay-backdrop-bgColor, var(--color-neutral-muted));
}

77 changes: 77 additions & 0 deletions app/components/primer/dialog_helper.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
function dialogInvokerButtonHandler(event: Event) {
const target = event.target as HTMLElement
const button = target?.closest('button')

if (!button || button.hasAttribute('disabled') || button.getAttribute('aria-disabled') === 'true') return

// 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()
}
}

dialogId = button.getAttribute('data-close-dialog-id') || button.getAttribute('data-submit-dialog-id')
if (dialogId) {
const dialog = document.getElementById(dialogId)
if (dialog instanceof HTMLDialogElement && dialog.open) {
dialog.close()
}
}
}

export class DialogHelperElement extends HTMLElement {
get dialog() {
return this.querySelector('dialog')
}

#abortController: AbortController | null = null
connectedCallback() {
const {signal} = (this.#abortController = new AbortController())
document.addEventListener('click', dialogInvokerButtonHandler)
document.addEventListener('click', this, {signal})
}

disconnectedCallback() {
this.#abortController?.abort()
}

handleEvent(event: MouseEvent) {
const target = event.target as HTMLElement
const dialog = this.dialog
if (!dialog?.open) return
if (target?.closest('dialog') !== dialog) return

const rect = dialog.getBoundingClientRect()
const clickWasInsideDialog =
rect.top <= event.clientY &&
event.clientY <= rect.top + rect.height &&
rect.left <= event.clientX &&
event.clientX <= rect.left + rect.width

if (!clickWasInsideDialog) {
dialog.close()
}
}
}

declare global {
interface Window {
DialogHelperElement: typeof DialogHelperElement
}
interface HTMLElementTagNameMap {
'dialog-helper': DialogHelperElement
}
}

if (!window.customElements.get('dialog-helper')) {
window.DialogHelperElement = DialogHelperElement
window.customElements.define('dialog-helper', DialogHelperElement)
}
2 changes: 1 addition & 1 deletion app/components/primer/primer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ import '@github/include-fragment-element'
import './alpha/action_bar_element'
import './alpha/dropdown'
import './anchored_position'
import './dialog_helper'
import './focus_group'
import './alpha/image_crop'
import './alpha/modal_dialog'
import './beta/nav_list'
import './beta/nav_list_group_element'
import './alpha/segmented_control'
Expand Down
Loading

0 comments on commit 6c45a4b

Please sign in to comment.