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
department-of-veterans-affairs · Oct 3, 2024 · 2a2676c · 2a2676c
1 parent a75dccd
commit 2a2676c
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 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
@@ -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
@@ -7,43 +7,49 @@
   border-radius: 4px;
   padding: 1em;
 }
+
 @media (min-width: 768px) {
   :host {
     border: none;
     padding: 1em 1em 1em 0;
   }
 }
 
-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;
   list-style-type: none;
   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;
 }
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
@@ -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>
     );
   }