From 97cb50aec5417e3325e58c239c0feca2dca815fe Mon Sep 17 00:00:00 2001
From: Matthew White <matthew-white@users.noreply.github.com>
Date: Fri, 13 Dec 2024 14:51:41 -0500
Subject: [PATCH 1/2] Resize hover cards only based on <dl>, not
 .hover-card-title

---
 src/components/hover-card.vue      | 66 +++++++++++---------
 test/components/hover-card.spec.js | 97 ++++++++++++++++++++++++++++++
 2 files changed, 135 insertions(+), 28 deletions(-)

diff --git a/src/components/hover-card.vue b/src/components/hover-card.vue
index 33cc844dd..6e110329c 100644
--- a/src/components/hover-card.vue
+++ b/src/components/hover-card.vue
@@ -28,7 +28,7 @@ except according to the terms contained in the LICENSE file.
 <script setup>
 import { onMounted, ref } from 'vue';
 
-import { px, truncatesText } from '../util/dom';
+import { px } from '../util/dom';
 
 defineOptions({
   name: 'HoverCard'
@@ -55,34 +55,44 @@ const props = defineProps({
 
 const width = ref('');
 const el = ref(null);
-const resize = () => {
-  if (props.truncateDt) return;
+// Returns the desired width of .hover-card-body based on the width of the <dl>.
+// Returning 0 means that the default width should be used.
+const computeBodyWidth = () => {
+  if (props.truncateDt) return 0;
+  const dd = [...el.value.querySelectorAll('dd')];
+  if (dd.length === 0) return 0;
+
   const dtWidth = el.value.querySelector('dt').getBoundingClientRect().width;
-  const ddWidth = el.value.querySelector('dd').getBoundingClientRect().width;
-  if (dtWidth > ddWidth) {
-    const currentWidth = el.value.getBoundingClientRect().width;
-    const equalWidth = px(currentWidth + dtWidth - ddWidth);
-    const { style } = el.value;
-    style.width = equalWidth;
-    if (![...el.value.querySelectorAll('dd')].some(truncatesText)) {
-      // Don't give the <dd> elements the same width as the <dt> elements unless
-      // they need it. The <dt> elements are already making the hover card wider
-      // than the default. The width of the <dd> elements is allowed to grow up
-      // to the width of the <dt> elements, but it shouldn't grow beyond what it
-      // needs.
-      style.width = 'auto';
-
-      // Bail if setting `width: auto` causes the width of the <dd> elements to
-      // shrink.
-      if (el.value.getBoundingClientRect().width < currentWidth)
-        style.width = '';
-    }
-
-    // Persist the new width in width.value so that it appears in the style
-    // attribute. That's needed for the style to be copied to the popover.
-    width.value = style.width;
-    style.width = '';
-  }
+  const ddWidth = dd[0].getBoundingClientRect().width;
+  // Don't give the <dd> elements the same width as the <dt> elements unless
+  // they need it (unless at least one is truncated). The <dt> elements are
+  // already making the hover card wider than the default. The width of the <dd>
+  // elements is allowed to grow up to the width of the <dt> elements, but it
+  // shouldn't grow beyond what it needs.
+  return dtWidth + Math.min(dtWidth, ddWidth);
+};
+const resize = () => {
+  // Get the width excluding borders.
+  const currentBodyWidth = el.value.querySelector('.hover-card-body').clientWidth;
+
+  // Hide children other than .hover-card-body before setting `width: auto` so
+  // that we can compute the width of the hover card based on the width of
+  // .hover-card-body alone. For example, we don't want .hover-card-title to
+  // influence the width of the hover card.
+  const otherChildren = [...el.value.querySelectorAll(':scope > *:not(.hover-card-body)')];
+  for (const child of otherChildren) child.style.display = 'none';
+  el.value.style.width = 'auto';
+
+  const newBodyWidth = computeBodyWidth();
+
+  el.value.style.width = '';
+  for (const child of otherChildren) child.style.display = '';
+
+  // Persist the new width in width.value so that it appears in the style
+  // attribute. That's needed for the style to be copied to the popover.
+  if (newBodyWidth > currentBodyWidth)
+    width.value = px(el.value.getBoundingClientRect().width +
+      newBodyWidth - currentBodyWidth);
 };
 onMounted(resize);
 </script>
diff --git a/test/components/hover-card.spec.js b/test/components/hover-card.spec.js
index 172db4f41..f2bfe80e5 100644
--- a/test/components/hover-card.spec.js
+++ b/test/components/hover-card.spec.js
@@ -1,4 +1,9 @@
+import { nextTick } from 'vue';
+
 import HoverCard from '../../src/components/hover-card.vue';
+import Popover from '../../src/components/popover.vue';
+
+import { truncatesText } from '../../src/util/dom';
 
 import { mount } from '../util/lifecycle';
 
@@ -10,4 +15,96 @@ describe('HoverCard', () => {
     const icon = component.get('.hover-card-heading span');
     icon.classes('icon-file').should.be.true;
   });
+
+  describe('truncateDt prop', () => {
+    const Parent = {
+      template: `<popover>
+        <hover-card icon="file" :truncate-dt="truncateDt">
+          <template #title>{{ title }}</template>
+          <template #subtitle>foo</template>
+          <template #body>
+            <dl class="dl-horizontal"><dt>{{ dt }}</dt><dd>{{ dd }}</dd></dl>
+          </template>
+        </hover-card>
+      </popover>`,
+      components: { HoverCard, Popover },
+      props: {
+        title: {
+          type: String,
+          default: 'x'
+        },
+        dt: {
+          type: String,
+          required: true
+        },
+        dd: {
+          type: String,
+          default: 'x'
+        },
+        truncateDt: {
+          type: Boolean,
+          // This is the opposite of the default in HoverCard.
+          default: false
+        }
+      }
+    };
+    const setupResize = async (props) => {
+      const component = mount(Parent, { props, attachTo: document.body });
+      // Wait a tick for the resize to happen.
+      await nextTick();
+
+      const { width: hoverCardWidth } = component.get('.hover-card').element
+        .getBoundingClientRect();
+      const { width: dtWidth } = component.get('dt').element
+        .getBoundingClientRect();
+      const { width: ddWidth } = component.get('dd').element
+        .getBoundingClientRect();
+      // 2 for the border.
+      hoverCardWidth.should.equal(dtWidth + ddWidth + 2);
+
+      return { component, dtWidth, ddWidth };
+    };
+
+    const xs = (count) => 'x'.repeat(count);
+
+    it('truncates the <dt> if truncateDt is true', async () => {
+      const { component, dtWidth, ddWidth } = await setupResize({
+        dt: xs(50),
+        truncateDt: true
+      });
+      dtWidth.should.equal(144);
+      // There aren't any tooltips, so we'll assert text truncation this way.
+      truncatesText(component.get('dt').element).should.be.true;
+      ddWidth.should.equal(144);
+    });
+
+    it('allows the <dt> to grow if truncateDt is false', async () => {
+      const { component, dtWidth, ddWidth } = await setupResize({
+        dt: xs(50)
+      });
+      dtWidth.should.be.above(300);
+      truncatesText(component.get('dt').element).should.be.false;
+      ddWidth.should.be.below(50);
+    });
+
+    it('allows the <dd> to grow up to the width of the <dt>', async () => {
+      const { component, dtWidth, ddWidth } = await setupResize({
+        dt: xs(50),
+        dd: xs(100)
+      });
+      dtWidth.should.be.above(300);
+      ddWidth.should.equal(dtWidth);
+      truncatesText(component.get('dd').element).should.be.true;
+    });
+
+    it('does not allow the title to grow arbitrarily wide', async () => {
+      const { component, ddWidth } = await setupResize({
+        dt: xs(50),
+        title: xs(100)
+      });
+      truncatesText(component.get('.hover-card-title').element).should.be.true;
+      // The long title should not cause the <dd> to grow.
+      ddWidth.should.be.below(50);
+    });
+  });
 });

From 680fe73662d7920ea92fc368f33c529cf63ddd95 Mon Sep 17 00:00:00 2001
From: Matthew White <matthew-white@users.noreply.github.com>
Date: Mon, 16 Dec 2024 01:04:39 -0500
Subject: [PATCH 2/2] Make changes after code review

---
 src/components/hover-card.vue | 56 ++++++++++++++++++-----------------
 1 file changed, 29 insertions(+), 27 deletions(-)

diff --git a/src/components/hover-card.vue b/src/components/hover-card.vue
index 6e110329c..076081d2f 100644
--- a/src/components/hover-card.vue
+++ b/src/components/hover-card.vue
@@ -28,7 +28,7 @@ except according to the terms contained in the LICENSE file.
 <script setup>
 import { onMounted, ref } from 'vue';
 
-import { px } from '../util/dom';
+import { px, styleBox } from '../util/dom';
 
 defineOptions({
   name: 'HoverCard'
@@ -57,42 +57,44 @@ const width = ref('');
 const el = ref(null);
 // Returns the desired width of .hover-card-body based on the width of the <dl>.
 // Returning 0 means that the default width should be used.
-const computeBodyWidth = () => {
-  if (props.truncateDt) return 0;
-  const dd = [...el.value.querySelectorAll('dd')];
-  if (dd.length === 0) return 0;
+const computeWidth = (body) => {
+  const dt = body.querySelector('dt');
+  if (dt == null) return 0;
 
-  const dtWidth = el.value.querySelector('dt').getBoundingClientRect().width;
-  const ddWidth = dd[0].getBoundingClientRect().width;
+  const dtWidth = dt.getBoundingClientRect().width;
+  const ddWidth = body.querySelector('dd').getBoundingClientRect().width;
   // Don't give the <dd> elements the same width as the <dt> elements unless
-  // they need it (unless at least one is truncated). The <dt> elements are
-  // already making the hover card wider than the default. The width of the <dd>
-  // elements is allowed to grow up to the width of the <dt> elements, but it
-  // shouldn't grow beyond what it needs.
+  // they need it. The <dt> elements are already making the hover card wider
+  // than the default. The width of the <dd> elements is allowed to grow up to
+  // the width of the <dt> elements, but it shouldn't grow beyond what it needs.
   return dtWidth + Math.min(dtWidth, ddWidth);
 };
 const resize = () => {
-  // Get the width excluding borders.
-  const currentBodyWidth = el.value.querySelector('.hover-card-body').clientWidth;
-
-  // Hide children other than .hover-card-body before setting `width: auto` so
-  // that we can compute the width of the hover card based on the width of
-  // .hover-card-body alone. For example, we don't want .hover-card-title to
-  // influence the width of the hover card.
-  const otherChildren = [...el.value.querySelectorAll(':scope > *:not(.hover-card-body)')];
-  for (const child of otherChildren) child.style.display = 'none';
+  if (props.truncateDt) return;
+
+  const body = el.value.querySelector('.hover-card-body');
+  const box = styleBox(getComputedStyle(body));
+  const currentWidth = body.getBoundingClientRect().width -
+    box.borderLeft - box.borderRight;
+
+  // Hide siblings of .hover-card-body (i.e., .hover-card-heading and
+  // .hover-card-footer) before setting `width: auto`. That way, we can compute
+  // the width of the hover card based on the width of .hover-card-body alone.
+  // For example, we don't want .hover-card-title to influence the width of the
+  // hover card.
+  const siblings = [...el.value.querySelectorAll(':scope > :not(.hover-card-body)')];
+  for (const sibling of siblings) sibling.style.display = 'none';
   el.value.style.width = 'auto';
 
-  const newBodyWidth = computeBodyWidth();
+  const newWidth = computeWidth(body);
 
   el.value.style.width = '';
-  for (const child of otherChildren) child.style.display = '';
+  for (const sibling of siblings) sibling.style.display = '';
 
-  // Persist the new width in width.value so that it appears in the style
-  // attribute. That's needed for the style to be copied to the popover.
-  if (newBodyWidth > currentBodyWidth)
-    width.value = px(el.value.getBoundingClientRect().width +
-      newBodyWidth - currentBodyWidth);
+  if (newWidth > currentWidth)
+    // Persist the new width in width.value so that it is added to the `style`
+    // attribute. That's needed for the style to be copied to the popover.
+    width.value = px(el.value.getBoundingClientRect().width + newWidth - currentWidth);
 };
 onMounted(resize);
 </script>