From de602f1ddd5a472b9fb2a3f9027fc571a29084f5 Mon Sep 17 00:00:00 2001
From: Tobias <tobias@tujo.no>
Date: Wed, 12 Oct 2022 12:55:41 +0200
Subject: [PATCH] fix(Dl): ensure correct spacing

Using CSS gap did also include our "hidden" wrapper element. So we need to use margin instead.
---
 packages/dnb-eufemia/src/elements/Dl.tsx      | 15 ++++---
 packages/dnb-eufemia/src/elements/Element.tsx |  3 +-
 .../src/elements/__tests__/Lists.test.tsx     | 44 +++++++++++++++++++
 .../src/elements/stories/Lists.stories.tsx    | 12 ++---
 .../__snapshots__/Elements.test.js.snap       | 16 ++++---
 .../dnb-eufemia/src/style/elements/lists.scss | 15 ++++---
 6 files changed, 80 insertions(+), 25 deletions(-)

diff --git a/packages/dnb-eufemia/src/elements/Dl.tsx b/packages/dnb-eufemia/src/elements/Dl.tsx
index 4e0a1082d67..b1f32b3b034 100644
--- a/packages/dnb-eufemia/src/elements/Dl.tsx
+++ b/packages/dnb-eufemia/src/elements/Dl.tsx
@@ -6,6 +6,7 @@
 import React from 'react'
 import classnames from 'classnames'
 import E, { ElementProps } from './Element'
+import { SpacingProps } from '../shared/types'
 
 export type DlProps = {
   /**
@@ -28,18 +29,20 @@ const Dl = ({ direction, ...props }: DlAllProps) => {
   return <E is="dl" {...props} skeleton={false} />
 }
 
-export type DlItemProps = {
-  //
-}
-
 Dl.Item = ({
   className,
   children,
-}: DlItemProps & React.AllHTMLAttributes<HTMLSpanElement>) => {
+  ...props
+}: React.AllHTMLAttributes<HTMLSpanElement> & SpacingProps) => {
   return (
     <>
       {children}
-      <dd aria-hidden className={classnames(className, 'dnb-dl__item')} />
+      <E
+        is="dd"
+        aria-hidden
+        className={classnames(className, 'dnb-dl__item')}
+        {...props}
+      />
     </>
   )
 }
diff --git a/packages/dnb-eufemia/src/elements/Element.tsx b/packages/dnb-eufemia/src/elements/Element.tsx
index cd59a05e4ee..6c679f6fc86 100644
--- a/packages/dnb-eufemia/src/elements/Element.tsx
+++ b/packages/dnb-eufemia/src/elements/Element.tsx
@@ -17,6 +17,7 @@ import {
   SkeletonMethods,
 } from '../components/skeleton/SkeletonHelper'
 import { includeValidProps } from '../components/form-row/FormRowHelpers'
+import { SpacingProps } from '../shared/types'
 
 export type ElementInternalProps = {
   is: React.ReactNode
@@ -36,7 +37,7 @@ export type ElementProps = {
 
   /** @deprecated use skeletonMethod instead */
   skeleton_method?: SkeletonMethods
-}
+} & SpacingProps
 type ElementAllProps = ElementInternalProps & ElementProps
 
 type Attributes = Record<string, unknown>
diff --git a/packages/dnb-eufemia/src/elements/__tests__/Lists.test.tsx b/packages/dnb-eufemia/src/elements/__tests__/Lists.test.tsx
index 691b4e1959f..a32e7a0eb59 100644
--- a/packages/dnb-eufemia/src/elements/__tests__/Lists.test.tsx
+++ b/packages/dnb-eufemia/src/elements/__tests__/Lists.test.tsx
@@ -11,6 +11,36 @@ import Dt from '../Dt'
 import Dd from '../Dd'
 
 describe('Dl', () => {
+  it('should support spacing props', () => {
+    render(
+      <Dl top="medium" direction="horizontal">
+        <Dl.Item top="medium">
+          <Dt top="medium">Term</Dt>
+          <Dd top="medium">Description</Dd>
+        </Dl.Item>
+      </Dl>
+    )
+
+    const element = document.querySelector('.dnb-dl')
+
+    expect(Array.from(element.classList)).toEqual([
+      'dnb-dl__direction--horizontal',
+      'dnb-space__top--medium',
+      'dnb-dl',
+    ])
+    expect(Array.from(element.querySelector('dt').classList)).toEqual([
+      'dnb-space__top--medium',
+      'dnb-dt',
+    ])
+    expect(Array.from(element.querySelector('dd').classList)).toEqual([
+      'dnb-space__top--medium',
+      'dnb-dd',
+    ])
+    expect(
+      Array.from(element.querySelector('.dnb-dl__item').classList)
+    ).toEqual(['dnb-dl__item', 'dnb-space__top--medium', 'dnb-dd'])
+  })
+
   describe('in horizontal direction', () => {
     it('should validate with ARIA rules', async () => {
       const Component = render(
@@ -27,6 +57,20 @@ describe('Dl', () => {
       )
       expect(await axeComponent(Component)).toHaveNoViolations()
     })
+
+    it('should have aria-hidden on item', async () => {
+      render(
+        <Dl direction="horizontal">
+          <Dl.Item>
+            <Dt>Term</Dt>
+            <Dd>Description</Dd>
+          </Dl.Item>
+        </Dl>
+      )
+      expect(
+        document.querySelector('.dnb-dl__item').getAttribute('aria-hidden')
+      ).toBe('true')
+    })
   })
 
   describe('with nested Dls', () => {
diff --git a/packages/dnb-eufemia/src/elements/stories/Lists.stories.tsx b/packages/dnb-eufemia/src/elements/stories/Lists.stories.tsx
index 66a1095c07a..9a0a030e245 100644
--- a/packages/dnb-eufemia/src/elements/stories/Lists.stories.tsx
+++ b/packages/dnb-eufemia/src/elements/stories/Lists.stories.tsx
@@ -22,7 +22,7 @@ export default {
 }
 
 export const ListSandbox = () => (
-  <Wrapper className="dnb-spacing">
+  <Wrapper>
     <Provider>
       <div>
         <Box>
@@ -127,10 +127,12 @@ export const ListSandbox = () => (
             <Dd>Description 1</Dd>
             <Dd>Description 2</Dd>
             <Dd>Description 3</Dd>
-            <dl className="dnb-dl">
-              <Dt>Sub Term</Dt>
-              <Dd>Sub Description</Dd>
-            </dl>
+            <Dd>
+              <Dl>
+                <Dt>Sub Term</Dt>
+                <Dd>Sub Description</Dd>
+              </Dl>
+            </Dd>
           </Dl>
         </Box>
         <Box>
diff --git a/packages/dnb-eufemia/src/style/elements/__tests__/__snapshots__/Elements.test.js.snap b/packages/dnb-eufemia/src/style/elements/__tests__/__snapshots__/Elements.test.js.snap
index 03a8ef5e4d3..9c9540272d8 100644
--- a/packages/dnb-eufemia/src/style/elements/__tests__/__snapshots__/Elements.test.js.snap
+++ b/packages/dnb-eufemia/src/style/elements/__tests__/__snapshots__/Elements.test.js.snap
@@ -733,22 +733,24 @@ a.dnb-button {
   @media screen and (min-width: 40em) {
     .dnb-dl__direction--horizontal {
       display: flex;
-      flex-wrap: wrap;
-      gap: 0.5rem; }
+      flex-wrap: wrap; }
       .dnb-dl__direction--horizontal dt {
-        margin-right: 0.5rem;
+        margin-right: 1rem;
         max-width: 30%; }
       .dnb-dl__direction--horizontal dd {
-        width: calc(70% - 1.5rem); }
+        width: calc(70% - 1rem); }
       .dnb-dl__direction--horizontal dt,
       .dnb-dl__direction--horizontal dd,
       .dnb-dl__direction--horizontal dd ~ dt {
-        margin-top: 0;
+        margin-top: 0.5rem;
         margin-bottom: 0; }
+        .dnb-dl__direction--horizontal dt:first-of-type,
+        .dnb-dl__direction--horizontal dd:first-of-type,
+        .dnb-dl__direction--horizontal dd ~ dt:first-of-type {
+          margin-top: 0; }
     .dnb-dl dd.dnb-dl__item {
       width: 100%;
-      height: 0;
-      overflow: hidden; } }
+      height: 0; } }
 
 .dnb-spacing .dnb-ul:not([class*='dnb-space__top']),
 .dnb-spacing .dnb-ol:not([class*='dnb-space__top']) {
diff --git a/packages/dnb-eufemia/src/style/elements/lists.scss b/packages/dnb-eufemia/src/style/elements/lists.scss
index 321c0a086a6..39d5a500edd 100644
--- a/packages/dnb-eufemia/src/style/elements/lists.scss
+++ b/packages/dnb-eufemia/src/style/elements/lists.scss
@@ -139,30 +139,33 @@
   // Inline
   @include allAbove(small) {
     &__direction--horizontal {
+      // We can not use gap: 0.5rem because of our wrapper "__item"
       display: flex;
       flex-wrap: wrap;
-      gap: 0.5rem;
 
       & dt {
-        margin-right: 0.5rem;
+        margin-right: 1rem;
         max-width: 30%;
       }
+
       & dd {
-        // width: calc(70% - 1.5rem); // minus margin-right and the gap
-        width: calc(70% - 1.5rem); // minus margin-right and the gap
+        width: calc(70% - 1rem); // minus margin-right and the gap
       }
+
       & dt,
       & dd,
       & dd ~ dt {
-        margin-top: 0;
+        margin-top: 0.5rem;
         margin-bottom: 0;
+        &:first-of-type {
+          margin-top: 0;
+        }
       }
     }
 
     dd#{&}__item {
       width: 100%;
       height: 0;
-      overflow: hidden;
     }
   }
 }