From 7b8ed815cdf8b57f7691b3706711579c0ee0762b Mon Sep 17 00:00:00 2001
From: Andy Smith <andy@37signals.com>
Date: Wed, 9 Apr 2025 14:09:08 -0500
Subject: [PATCH] Address PR feedback

---
 app/assets/stylesheets/buttons.css              |  9 ---------
 app/assets/stylesheets/cards.css                |  3 ++-
 app/assets/stylesheets/expander.css             | 11 ++++++-----
 .../controllers/details_controller.js           | 13 +++++++++++++
 .../controllers/expand_btn_controller.js        | 17 -----------------
 app/views/bubbles/_pop_toggle.html.erb          |  4 ++--
 6 files changed, 23 insertions(+), 34 deletions(-)
 create mode 100644 app/javascript/controllers/details_controller.js
 delete mode 100644 app/javascript/controllers/expand_btn_controller.js

diff --git a/app/assets/stylesheets/buttons.css b/app/assets/stylesheets/buttons.css
index b31080b88..0ca763506 100644
--- a/app/assets/stylesheets/buttons.css
+++ b/app/assets/stylesheets/buttons.css
@@ -194,12 +194,3 @@
   }
 }
 
-.btn--dialog {
-  .icon--caret-down {
-    font-size: 1ch;
-  }
-
-  + .popup {
-    margin-top: var(--btn-size);
-  }
-}
diff --git a/app/assets/stylesheets/cards.css b/app/assets/stylesheets/cards.css
index 4d66afe4b..fceb080a8 100644
--- a/app/assets/stylesheets/cards.css
+++ b/app/assets/stylesheets/cards.css
@@ -116,8 +116,9 @@
     margin-block: calc(var(--block-space) * -1) var(--block-space);
     padding: var(--block-space) var(--inline-space-double);
     position: relative;
+    z-index: 0;
 
-    &:has(dialog[open]) {
+    &:has([open]) {
       z-index: 1;
     }
 
diff --git a/app/assets/stylesheets/expander.css b/app/assets/stylesheets/expander.css
index fc1b5ff2b..e7be6c769 100644
--- a/app/assets/stylesheets/expander.css
+++ b/app/assets/stylesheets/expander.css
@@ -46,6 +46,10 @@
   flex-direction: column;
   gap: 0;
   padding-block-start: var(--expander-height);
+
+  &:hover {
+    box-shadow: none;
+  }
 }
 
 /* For browsers that support animation */
@@ -56,6 +60,7 @@
     block-size: var(--expander-height);
     border-radius: var(--expander-radius);
     inset: 0 auto auto 0;
+    inline-size: var(--expander-width);
     overflow: clip;
     position: absolute;
     transition: content-visibility var(--expander-duration) allow-discrete, block-size var(--expander-duration), inline-size var(--expander-duration);
@@ -64,12 +69,9 @@
   .expander[open]::details-content {
     block-size: auto;
     inline-size: auto;
-
-    box-shadow: 0 0 0 var(--hover-size) var(--hover-color);
   }
 
   .expander__content {
-    inline-size: var(--expander-width);
     min-inline-size: var(--expander-width);
   }
 }
@@ -84,7 +86,6 @@
 
     [open] & {
       block-size: auto;
-      box-shadow: 0 0 0 var(--hover-size) var(--hover-color);
       inline-size: auto;
     }
   }
@@ -99,7 +100,7 @@
   --hover-size: 0;
   --outline-color: transparent;
 
-  inline-size: calc(100% + var(--inline-space) * 2);
+  inline-size: calc(100% + var(--inline-space-double));
   margin-inline-start: calc(-1 * var(--inline-space));
   justify-content: flex-start;
   white-space: nowrap;
diff --git a/app/javascript/controllers/details_controller.js b/app/javascript/controllers/details_controller.js
new file mode 100644
index 000000000..7259fd665
--- /dev/null
+++ b/app/javascript/controllers/details_controller.js
@@ -0,0 +1,13 @@
+import { Controller } from "@hotwired/stimulus"
+
+export default class extends Controller {
+  static targets = [ "details" ]
+
+  close() {
+    this.detailsTarget.removeAttribute("open")
+  }
+
+  closeOnClickOutside({ target }) {
+    if (!this.element.contains(target)) this.close()
+  }
+}
diff --git a/app/javascript/controllers/expand_btn_controller.js b/app/javascript/controllers/expand_btn_controller.js
deleted file mode 100644
index f62605470..000000000
--- a/app/javascript/controllers/expand_btn_controller.js
+++ /dev/null
@@ -1,17 +0,0 @@
-import { Controller } from "@hotwired/stimulus"
-
-export default class extends Controller {
-  static targets = [ "dialog", "content" ]
-
-  connect() {
-    const width = this.dialogTarget.offsetWidth;
-  }
-
-  toggle() {
-    const isOpen = this.dialogTarget.open;
-
-    if (!isOpen) {
-      console.log("isOpen");
-    }
-  }
-}
diff --git a/app/views/bubbles/_pop_toggle.html.erb b/app/views/bubbles/_pop_toggle.html.erb
index 438f4f821..1268061b5 100644
--- a/app/views/bubbles/_pop_toggle.html.erb
+++ b/app/views/bubbles/_pop_toggle.html.erb
@@ -5,8 +5,8 @@
       <span>Un-do</span>
     <% end %>
   <% else %>
-    <div class="flex-inline center position-relative" data-controller="dialog" data-action="keydown.esc->dialog#close click@document->dialog#closeOnClickOutside">
-      <details class="expander">
+    <div class="flex-inline center position-relative">
+      <details class="expander" data-controller="details" data-details-target="details" data-action="keydown.esc->details#close click@document->details#closeOnClickOutside">
         <summary class="expander__button btn borderless">
           Close as
           <%= icon_tag "caret-down" %>