From 2a2676c452c320f0b68ec1d8d1717e93322c09e6 Mon Sep 17 00:00:00 2001
From: Andrew Steele <andrew.steele@adhocteam.us>
Date: Thu, 3 Oct 2024 12:14:22 -0400
Subject: [PATCH] va-on-this-page: Rewrite to use ul instead of dl for list of
 links (#1346)

* va-on-this-page: Rewrite to use ul instead of dl for list of links

* Improvements to on-this-page unit tests

* Updates to link alignment when wrapped

* Updated tests

* Adjusting item width

* Attempting a manual trigger of Chromatic

* Reverting chromatic's onlyChanged

* Reverting max-width to 285px
---
 .../test/va-on-this-page.e2e.ts               | 51 ++++++++++---------
 .../va-on-this-page/va-on-this-page.css       | 30 ++++++-----
 .../va-on-this-page/va-on-this-page.tsx       | 16 +++---
 3 files changed, 53 insertions(+), 44 deletions(-)

diff --git a/packages/web-components/src/components/va-on-this-page/test/va-on-this-page.e2e.ts b/packages/web-components/src/components/va-on-this-page/test/va-on-this-page.e2e.ts
index cc316ead7..3c13599c0 100644
--- a/packages/web-components/src/components/va-on-this-page/test/va-on-this-page.e2e.ts
+++ b/packages/web-components/src/components/va-on-this-page/test/va-on-this-page.e2e.ts
@@ -12,10 +12,9 @@ describe('va-on-this-page', () => {
       <va-on-this-page class="hydrated">
         <mock:shadow-root>
           <nav aria-labelledby="on-this-page">
-            <dl>
-              <dt id="on-this-page">on-this-page</dt>
-              <dd role="definition"></dd>
-            </dl>
+            <ul>
+              <li id="on-this-page">on-this-page</li>
+            </ul>
           </nav>
         </mock:shadow-root>
       </va-on-this-page>
@@ -59,19 +58,21 @@ describe('va-on-this-page', () => {
       <va-on-this-page class="hydrated">
         <mock:shadow-root>
           <nav aria-labelledby="on-this-page">
-            <dl>
-              <dt id="on-this-page">on-this-page</dt>
-              <dd role="definition">
+            <ul>
+              <li id="on-this-page">on-this-page</li>
+              <li>
                 <a href="#an-id">
                   <va-icon class="hydrated"></va-icon>
-                  Hello
+                  <span>Hello</span>
                 </a>
+              </li>
+              <li>
                 <a href="#its-me">
                   <va-icon class="hydrated"></va-icon>
-                  It's me
+                  <span>It's me</span>
                 </a>
-              </dd>
-            </dl>
+              </li>
+            </ul>
           </nav>
         </mock:shadow-root>
       </va-on-this-page>
@@ -98,15 +99,15 @@ describe('va-on-this-page', () => {
     <va-on-this-page class="hydrated">
     <mock:shadow-root>
       <nav aria-labelledby="on-this-page">
-        <dl>
-          <dt id="on-this-page">on-this-page</dt>
-          <dd role="definition">
+        <ul>
+          <li id="on-this-page">on-this-page</li>
+          <li>
             <a href="#foo">
               <va-icon class="hydrated"></va-icon>
-              Foo
+              <span>Foo</span>
             </a>
-          </dd>
-        </dl>
+          </li>
+        </ul>
       </nav>
     </mock:shadow-root>
   </va-on-this-page>
@@ -135,19 +136,21 @@ describe('va-on-this-page', () => {
       <va-on-this-page class="hydrated">
         <mock:shadow-root>
           <nav aria-labelledby="on-this-page">
-            <dl>
-              <dt id="on-this-page">on-this-page</dt>
-              <dd role="definition">
+            <ul>
+              <li id="on-this-page">on-this-page</li>
+              <li>
                 <a href="#an-id">
                   <va-icon class="hydrated"></va-icon>
-                  Hello
+                  <span>Hello</span>
                 </a>
+              </li>
+              <li>
                 <a href="#its-me">
                   <va-icon class="hydrated"></va-icon>
-                  It's me
+                  <span>It's me</span>
                 </a>
-              </dd>
-            </dl>
+              </li>
+            </ul>
           </nav>
         </mock:shadow-root>
       </va-on-this-page>
diff --git a/packages/web-components/src/components/va-on-this-page/va-on-this-page.css b/packages/web-components/src/components/va-on-this-page/va-on-this-page.css
index 476dd70df..3a391163e 100644
--- a/packages/web-components/src/components/va-on-this-page/va-on-this-page.css
+++ b/packages/web-components/src/components/va-on-this-page/va-on-this-page.css
@@ -7,6 +7,7 @@
   border-radius: 4px;
   padding: 1em;
 }
+
 @media (min-width: 768px) {
   :host {
     border: none;
@@ -14,17 +15,13 @@
   }
 }
 
-dt {
+#on-this-page {
   font-family: var(--font-serif);
   font-size: 1.25rem;
   margin: 0.4em 0em;
 }
 
-dd {
-  margin: 0.4em 0em;
-}
-
-dl {
+ul {
   margin-top: 0;
   margin-bottom: 0;
   margin-left: 0;
@@ -32,18 +29,27 @@ dl {
   padding-left: 0;
 }
 
-dd a {
-  display: flex;
-  align-items: baseline;
+li {
   padding: 0.5em;
-  line-height: 1.5em;
   max-width: 285px;
 }
 
-a:active {
-  background-color: var(--vads-color-base-lighter);
+li a {
+  line-height: 1.5em;
+  display: flex;
+}
+
+li:hover {
+  background-color: rgba(0, 0, 0, 0.05);
+}
+
+/* Needed to override the background color from the links mixin */
+li a:hover {
+  background-color: transparent;
 }
 
 va-icon {
   margin-right: 4px;
+  padding-top: 4px;
+  line-height: 1;
 }
\ No newline at end of file
diff --git a/packages/web-components/src/components/va-on-this-page/va-on-this-page.tsx b/packages/web-components/src/components/va-on-this-page/va-on-this-page.tsx
index 6be75fc84..62c7558ba 100644
--- a/packages/web-components/src/components/va-on-this-page/va-on-this-page.tsx
+++ b/packages/web-components/src/components/va-on-this-page/va-on-this-page.tsx
@@ -90,17 +90,17 @@ export class VaOnThisPage {
 
     return (
       <nav aria-labelledby="on-this-page">
-        <dl>
-          <dt id="on-this-page">{i18next.t('on-this-page')}</dt>
-          <dd role="definition">
-            {h2s.map(heading => (
+        <ul>
+          <li id="on-this-page">{i18next.t('on-this-page')}</li>
+          {h2s.map(heading => (
+            <li>
               <a href={`#${heading.id}`} onClick={handleOnClick}>
                 <va-icon icon="arrow_downward"></va-icon>
-                {heading.innerText}
+                <span>{heading.innerText}</span>
               </a>
-            ))}
-          </dd>
-        </dl>
+            </li>
+          ))}
+        </ul>
       </nav>
     );
   }