Refactor Dialog to use <dialog> internally (#2496)

Co-authored-by: Jon Rohan <[email protected]>
primer · Jan 17, 2024 · 983e3a5 · 983e3a5
1 parent dfd306c
commit 983e3a5
Show file tree

Hide file tree

Showing 13 changed files with 182 additions and 177 deletions.
diff --git a/.changeset/wise-coats-buy.md b/.changeset/wise-coats-buy.md
@@ -0,0 +1,5 @@
+---
+'@primer/view-components': minor
+---
+
+Primer::Alpha::Dialog uses <dialog> internally
diff --git a/app/components/primer/alpha/action_menu/action_menu_element.ts b/app/components/primer/alpha/action_menu/action_menu_element.ts
@@ -279,7 +279,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})
   }

diff --git a/app/components/primer/alpha/dialog.html.erb b/app/components/primer/alpha/dialog.html.erb
@@ -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? %>
@@ -11,4 +11,4 @@
       <%= footer %>
     <% end %>
   <% end %>
-</div>
+</dialog-helper>
diff --git a/app/components/primer/alpha/dialog.pcss b/app/components/primer/alpha/dialog.pcss
@@ -1,5 +1,9 @@
 /* Overlay */
 
+dialog.Overlay:not([open]) {
+  display: none;
+}
+
 .Overlay--hidden {
   display: none !important;
 }
@@ -13,12 +17,18 @@
 
 .Overlay {
   display: flex;
+  inset: 0;
+  position: static;
+  margin: auto;
+  padding: 0;
   width: min(var(--overlay-width), 100vw - 2rem);
   min-width: 192px;
   max-height: min(calc(100vh - 2rem), var(--overlay-height));
   white-space: normal;
   flex-direction: column;
   background-color: var(--overlay-bgColor);
+  color: var(--fgColor-default);
+  border: 0;
   border-radius: var(--borderRadius-large);
   box-shadow: var(--shadow-floating-small);
   opacity: 1;
@@ -74,6 +84,23 @@
     height: auto;
   }
 
+  &.Overlay--placement-left, &.Overlay--placement-right {
+    height: 100%;
+    max-height: unset;
+  }
+
+  &.Overlay--placement-left {
+    inset: 0 auto 0 0;
+    border-top-left-radius: 0;
+    border-bottom-left-radius: 0;
+  }
+
+  &.Overlay--placement-right {
+    inset: 0 0 0 auto;
+    border-top-right-radius: 0;
+    border-bottom-right-radius: 0;
+  }
+
   /* start deprecate in favor of Alpha::Dialog */
   &.Overlay--height-xsmall {
     height: min(192px, 100vh - 2rem);
@@ -295,121 +322,50 @@
   }
 }
 
-@define-mixin Overlay-backdrop {
-  position: fixed;
-  top: 0;
-  right: 0;
-  bottom: 0;
-  left: 0;
-  z-index: 999;
-  display: flex;
-  background-color: var(--overlay-backdrop-bgColor, var(--color-neutral-muted));
-}
-
-@define-mixin Overlay-backdrop--transparent {
-  position: absolute;
-  z-index: 999;
-  background-color: transparent;
-}
-
 /* variants must be mixins so we can extend within a media query (@extend is not supported inside media queries) */
 
 /* border-radius repeats within placement options to ensure the original radius is reset when two classes co-exist */
 
-/* center */
-@define-mixin Overlay-backdrop--center {
-  @mixin Overlay-backdrop;
-
-  align-items: center;
-  justify-content: center;
-}
-
-/* anchor */
-@define-mixin Overlay-backdrop--anchor {
-  @mixin Overlay-backdrop--transparent;
+/* full width */
+.Overlay--full {
+  width: 100%;
+  max-width: 100vw;
+  height: 100%;
+  max-height: 100vh;
+  border-radius: unset !important;
+  flex-grow: 1;
 }
 
-/* anchor side(s) */
-@define-mixin Overlay-backdrop--side $responsiveVariant {
-  @mixin Overlay-backdrop;
-
-  /* default left */
-  align-items: center;
-  justify-content: left;
-
-  &.Overlay-backdrop--placement-left$(responsiveVariant) {
-    align-items: center;
-    justify-content: left;
-
-    & > .Overlay$(responsiveVariant) {
-      height: 100vh;
-      max-height: unset;
-      border-radius: var(--borderRadius-large);
-      border-top-left-radius: 0;
-      border-bottom-left-radius: 0;
+/* responsive variants */
 
-      @media screen and (prefers-reduced-motion: no-preference) {
-        animation: 250ms cubic-bezier(0.33, 1, 0.68, 1) 0s 1 normal none running Overlay--motion-slideInRight;
-      }
-    }
+/* --viewportRange-narrowLandscape */
+@media (max-width: 767px) {
+  .Overlay--placement-left-whenNarrow, .Overlay--placement-right-whenNarrow {
+    height: 100%;
+    max-height: 100vh;
   }
 
-  &.Overlay-backdrop--placement-right$(responsiveVariant) {
-    align-items: center;
-    justify-content: right;
-
-    & > .Overlay$(responsiveVariant) {
-      height: 100vh;
-      max-height: unset;
-      border-radius: var(--borderRadius-large);
-      border-top-right-radius: 0;
-      border-bottom-right-radius: 0;
-
-      @media screen and (prefers-reduced-motion: no-preference) {
-        animation: 250ms cubic-bezier(0.33, 1, 0.68, 1) 0s 1 normal none running Overlay--motion-slideInLeft;
-      }
-    }
+  .Overlay--placement-left-whenNarrow {
+    inset: 0 auto 0 0;
+    border-top-left-radius: 0;
+    border-bottom-left-radius: 0;
   }
 
-  &.Overlay-backdrop--placement-bottom$(responsiveVariant) {
-    align-items: end;
-    justify-content: center;
-
-    & > .Overlay$(responsiveVariant) {
-      width: 100vw;
-      height: auto;
-      max-height: calc(100vh - 2rem);
-      border-radius: var(--borderRadius-large);
-      border-bottom-right-radius: 0;
-      border-bottom-left-radius: 0;
-
-      @media screen and (prefers-reduced-motion: no-preference) {
-        animation: 250ms cubic-bezier(0.33, 1, 0.68, 1) 0s 1 normal none running Overlay--motion-slideUp;
-      }
-    }
+  .Overlay--placement-right-whenNarrow {
+    inset: 0 0 0 auto;
+    border-top-right-radius: 0;
+    border-bottom-right-radius: 0;
   }
 
-  &.Overlay-backdrop--placement-top$(responsiveVariant) {
-    align-items: start;
-    justify-content: center;
-
-    & > .Overlay$(responsiveVariant) {
-      border-radius: var(--borderRadius-large);
-      border-top-left-radius: 0;
-      border-top-right-radius: 0;
-
-      @media screen and (prefers-reduced-motion: no-preference) {
-        animation: 250ms cubic-bezier(0.33, 1, 0.68, 1) 0s 1 normal none running Overlay--motion-slideDown;
-      }
-    }
+  .Overlay--placement-bottom-whenNarrow {
+    width: 100%;
+    max-width: 100vw;
+    inset: auto 0 0;
+    border-bottom-left-radius: 0;
+    border-bottom-right-radius: 0;
   }
-}
 
-/* full width */
-@define-mixin Overlay-backdrop--full {
-  @mixin Overlay-backdrop;
-
-  & .Overlay {
+  .Overlay--full-whenNarrow {
     width: 100%;
     max-width: 100vw;
     height: 100%;
@@ -419,44 +375,6 @@
   }
 }
 
-/* Overlay variant classnames */
-.Overlay-backdrop--center {
-  @mixin Overlay-backdrop--center;
-}
-
-.Overlay-backdrop--anchor {
-  @mixin Overlay-backdrop--anchor;
-}
-
-.Overlay-backdrop--side {
-  @mixin Overlay-backdrop--side;
-}
-
-.Overlay-backdrop--full {
-  @mixin Overlay-backdrop--full;
-}
-
-/* responsive variants */
-
-/* --viewportRange-narrowLandscape */
-@media (max-width: 767px) {
-  .Overlay-backdrop--center-whenNarrow {
-    @mixin Overlay-backdrop--center;
-  }
-
-  .Overlay-backdrop--anchor-whenNarrow {
-    @mixin Overlay-backdrop--anchor;
-  }
-
-  .Overlay-backdrop--side-whenNarrow {
-    @mixin Overlay-backdrop--side -whenNarrow;
-  }
-
-  .Overlay-backdrop--full-whenNarrow {
-    @mixin Overlay-backdrop--full;
-  }
-}
-
 @keyframes Overlay--motion-slideDown {
   from {
     transform: translateY(-100%);

diff --git a/app/components/primer/alpha/dialog.rb b/app/components/primer/alpha/dialog.rb
@@ -34,19 +34,19 @@ class Dialog < Primer::Component
 
       DEFAULT_POSITION = :center
       POSITION_MAPPINGS = {
-        DEFAULT_POSITION => "Overlay-backdrop--center",
-        :left => "Overlay-backdrop--side Overlay-backdrop--placement-left",
-        :right => "Overlay-backdrop--side Overlay-backdrop--placement-right"
+        DEFAULT_POSITION => "",
+        :left => "Overlay--placement-left",
+        :right => "Overlay--placement-right"
       }.freeze
       POSITION_OPTIONS = POSITION_MAPPINGS.keys
 
       DEFAULT_POSITION_NARROW = :inherit
       POSITION_NARROW_MAPPINGS = {
         DEFAULT_POSITION_NARROW => "",
-        :bottom => "Overlay-backdrop--side-whenNarrow Overlay-backdrop--placement-bottom-whenNarrow",
-        :fullscreen => "Overlay-backdrop--full-whenNarrow",
-        :left => "Overlay-backdrop--side-whenNarrow Overlay-backdrop--placement-left-whenNarrow",
-        :right => "Overlay-backdrop--side-whenNarrow Overlay-backdrop--placement-right-whenNarrow"
+        :bottom => "Overlay--placement-bottom-whenNarrow",
+        :fullscreen => "Overlay--full-whenNarrow",
+        :left => "Overlay--placement-left-whenNarrow",
+        :right => "Overlay--placement-right-whenNarrow"
       }.freeze
       POSITION_NARROW_OPTIONS = POSITION_NARROW_MAPPINGS.keys
 
@@ -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(
@@ -143,11 +142,9 @@ def initialize(
           "Overlay-whenNarrow",
           SIZE_MAPPINGS[fetch_or_fallback(SIZE_OPTIONS, @size, DEFAULT_SIZE)],
           "Overlay--motion-scaleFade",
-          system_arguments[:classes]
-        )
-        @backdrop_classes = class_names(
           POSITION_MAPPINGS[fetch_or_fallback(POSITION_OPTIONS, @position, DEFAULT_POSITION)],
-          POSITION_NARROW_MAPPINGS[fetch_or_fallback(POSITION_NARROW_MAPPINGS, @position_narrow, DEFAULT_POSITION_NARROW)]
+          POSITION_NARROW_MAPPINGS[fetch_or_fallback(POSITION_NARROW_MAPPINGS, @position_narrow, DEFAULT_POSITION_NARROW)],
+          system_arguments[:classes]
         )
       end
 

diff --git a/app/components/primer/alpha/overlay.pcss b/app/components/primer/alpha/overlay.pcss
@@ -15,7 +15,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));
 }