fix: layout, visual and API issues with the DescriptionList

nulogy · Dec 13, 2024 · 15296cc · 15296cc
1 parent 39def81
commit 15296cc
Show file tree

Hide file tree

Showing 6 changed files with 124 additions and 49 deletions.
diff --git a/src/DescriptionList/DescriptionDetails.tsx b/src/DescriptionList/DescriptionDetails.tsx
@@ -5,22 +5,39 @@ export const DescriptionDetails = styled.dd(({ theme }) => {
   const { autoLayoutBreakpoint, showDivider, density, layout } = useDescriptionListContext();
 
   return {
+    // narrow stuff + non-auto layout
     margin: 0,
+    color: theme.colors.black,
     paddingLeft: theme.space.none,
     paddingRight: theme.space.none,
-    color: theme.colors.darkGrey,
+
+    ...(layout !== "inline" && {
+      paddingTop: theme.space.none,
+    }),
 
     ...(density === "compact" && {
-      paddingTop: theme.space.x0_25,
       paddingBottom: theme.space.x0_25,
+
+      ...(layout === "inline" && {
+        paddingTop: theme.space.x0_25,
+      }),
     }),
+
     ...(density === "medium" && {
-      paddingTop: theme.space.x0_75,
       paddingBottom: theme.space.x0_75,
+
+      ...(layout === "inline" && {
+        paddingTop: theme.space.x0_75,
+      }),
     }),
+
     ...(density === "relaxed" && {
-      paddingTop: theme.space.x1_5,
       paddingBottom: theme.space.x1_5,
+      paddingTop: theme.space.x0_25,
+
+      ...(layout === "inline" && {
+        paddingTop: theme.space.x1_5,
+      }),
     }),
 
     ...(showDivider &&
@@ -30,17 +47,31 @@ export const DescriptionDetails = styled.dd(({ theme }) => {
         borderTopColor: theme.colors.lightGrey,
       }),
 
+    "&:nth-child(2)": {
+      border: "none",
+    },
+
     [`@container (min-width: ${autoLayoutBreakpoint})`]: {
+      // wide auto layout
       ...(showDivider &&
         layout !== "stacked" && {
           borderTopWidth: "1px",
           borderTopStyle: "solid",
           borderTopColor: theme.colors.lightGrey,
         }),
 
-      "&:nth-child(2)": {
-        border: "none",
-      },
+      ...(density === "compact" &&
+        layout !== "stacked" && {
+          paddingTop: theme.space.x0_25,
+        }),
+      ...(density === "medium" &&
+        layout !== "stacked" && {
+          paddingTop: theme.space.x0_75,
+        }),
+      ...(density === "relaxed" &&
+        layout !== "stacked" && {
+          paddingTop: theme.space.x1_5,
+        }),
     },
   };
 });
diff --git a/src/DescriptionList/DescriptionList.story.tsx b/src/DescriptionList/DescriptionList.story.tsx
@@ -1,6 +1,6 @@
 import React from "react";
 import { Flex } from "../Flex";
-import { Heading4 } from "../Type";
+import { Heading4, Text } from "../Type";
 import { Link } from "../Link";
 import { Icon } from "../Icon";
 import { StatusIndicator } from "../StatusIndicator";
@@ -16,9 +16,10 @@ const SampleContent = () => (
     <DescriptionTerm>Customer</DescriptionTerm>
     <DescriptionDetails>Nulogy</DescriptionDetails>
     <DescriptionTerm>
-      <Flex display="inlineFlex" as="span" alignItems="center" gap="half">
-        Order number <Icon icon="info" size="x2_5" />
-      </Flex>
+      <Text display="inline-flex" alignItems="end">
+        Order number
+        <Icon icon="info" size="x3" paddingLeft="half" />
+      </Text>
     </DescriptionTerm>
     <DescriptionDetails>
       <Link href="/customer-details">P12-90381-2039</Link>
@@ -38,37 +39,34 @@ const SampleContent = () => (
   </>
 );
 
-export function WithDifferentLayouts() {
+export function Layout() {
   return (
-    <Flex flexDirection="column" gap="x4">
+    <Flex flexDirection="column" gap="x8">
       <Flex flexDirection="column" gap="x1" flex="1">
         <Heading4>Auto Layout (Default)</Heading4>
         <DescriptionList layout="auto">
           <SampleContent />
         </DescriptionList>
       </Flex>
-      <Flex>
-        <Flex flexDirection="column" gap="x1" flex="1">
-          <Heading4>Stacked Layout</Heading4>
-          <DescriptionList layout="stacked">
-            <SampleContent />
-          </DescriptionList>
-        </Flex>
-
-        <Flex flexDirection="column" gap="x1" flex="1">
-          <Heading4>Inline Layout</Heading4>
-          <DescriptionList layout="inline">
-            <SampleContent />
-          </DescriptionList>
-        </Flex>
+      <Flex flexDirection="column" gap="x1" flex="1">
+        <Heading4>Stacked Layout</Heading4>
+        <DescriptionList layout="stacked">
+          <SampleContent />
+        </DescriptionList>
+      </Flex>
+      <Flex flexDirection="column" gap="x1" flex="1">
+        <Heading4>Inline Layout</Heading4>
+        <DescriptionList layout="inline">
+          <SampleContent />
+        </DescriptionList>
       </Flex>
     </Flex>
   );
 }
 
-export function WithDifferentDensities() {
+export function Density() {
   return (
-    <Flex gap="x4" flexDirection="column">
+    <Flex gap="x8" flexDirection="column">
       <Flex flexDirection="column" gap="x1">
         <Heading4>Compact Density</Heading4>
         <Flex>
@@ -108,9 +106,9 @@ export function WithDifferentDensities() {
   );
 }
 
-export function WithDifferentFontSizes() {
+export function FontSize() {
   return (
-    <Flex gap="x4">
+    <Flex gap="x8">
       <Flex flexDirection="column" gap="x1" flexBasis="100%">
         <Heading4>Smaller Font Size</Heading4>
         <DescriptionList fontSize="smaller">
@@ -135,9 +133,9 @@ export function WithDifferentFontSizes() {
   );
 }
 
-export function WithDividers() {
+export function Divider() {
   return (
-    <Flex gap="x4">
+    <Flex gap="x8">
       <Flex flexDirection="column" gap="x1" flexBasis="100%">
         <Heading4>With Dividers (Stacked layout)</Heading4>
         <DescriptionList showDivider layout="stacked">
@@ -154,9 +152,9 @@ export function WithDividers() {
   );
 }
 
-export function WithCustomBreakpoints() {
+export function AutoLayoutCustomBreakpoint() {
   return (
-    <Flex gap="x4" flexDirection="column">
+    <Flex gap="x8" flexDirection="column">
       <Flex flexDirection="column" gap="x1">
         <Heading4>Auto layout custom breakpoint (800px)</Heading4>
         <DescriptionList layout="auto" autoLayoutBreakpoint="800px">
@@ -167,9 +165,9 @@ export function WithCustomBreakpoints() {
   );
 }
 
-export function WithCustomDescriptionTermWidth() {
+export function CustomDescriptionTermWidth() {
   return (
-    <Flex gap="x4" flexDirection="column">
+    <Flex gap="x8" flexDirection="column">
       <Flex flexDirection="column" gap="x1">
         <Heading4>Custom description term max-width (33.33%)</Heading4>
         <DescriptionList layout="inline" descriptionTermMaxWidth="30%">
@@ -194,13 +192,12 @@ export function WithCustomDescriptionTermWidth() {
 
 export function CombinedFeatures() {
   return (
-    <Flex gap="x4" flexDirection="column">
+    <Flex gap="x8" flexDirection="column">
       <Flex flexDirection="column" gap="x1">
         <Heading4>
-          Auto Layout + Dividers + Compact Density + Small Font + 40% Term Width + 720px auto layout breakpoint
+          Auto Layout, divider, compact density, small font, 40% term width, 720px auto layout breakpoint
         </Heading4>
         <DescriptionList
-          layout="auto"
           showDivider
           density="compact"
           fontSize="small"
@@ -210,6 +207,12 @@ export function CombinedFeatures() {
           <SampleContent />
         </DescriptionList>
       </Flex>
+      <Flex flexDirection="column" gap="x1">
+        <Heading4>Stacked, no dividers, relaxed, large font</Heading4>
+        <DescriptionList layout="stacked" density="relaxed" fontSize="large">
+          <SampleContent />
+        </DescriptionList>
+      </Flex>
     </Flex>
   );
 }
diff --git a/src/DescriptionList/DescriptionList.tsx b/src/DescriptionList/DescriptionList.tsx
@@ -11,6 +11,7 @@ export function DescriptionList({
   showDivider = false,
   density = "medium",
   fontSize = "medium",
+  lineHeight = "base",
   autoLayoutBreakpoint = "640px",
   children,
 }: Props) {
@@ -21,6 +22,7 @@ export function DescriptionList({
       showDivider={showDivider}
       density={density}
       fontSize={fontSize}
+      lineHeight={lineHeight}
       autoLayoutBreakpoint={autoLayoutBreakpoint}
     >
       <DescriptionListContainer>
@@ -36,13 +38,13 @@ export const DescriptionListContainer = styled.div({
 });
 
 export const StyledDescriptionList = styled.dl(({ theme }) => {
-  const { descriptionTermMaxWidth, layout, fontSize, autoLayoutBreakpoint } = useDescriptionListContext();
+  const { descriptionTermMaxWidth, layout, fontSize, lineHeight, autoLayoutBreakpoint } = useDescriptionListContext();
 
   return {
     margin: 0,
     display: "grid",
     fontSize: theme.fontSizes[fontSize] ?? theme.fontSizes.medium,
-    lineHeight: theme.lineHeights.base,
+    lineHeight: theme.lineHeights[lineHeight] ?? theme.lineHeights.base,
 
     ...(layout === "inline" && {
       gridTemplateColumns: `minmax(0, ${descriptionTermMaxWidth}) auto`,

diff --git a/src/DescriptionList/DescriptionListContext.tsx b/src/DescriptionList/DescriptionListContext.tsx
@@ -7,6 +7,7 @@ export interface DescriptionListConfig {
   showDivider?: boolean;
   density?: "medium" | "compact" | "relaxed";
   fontSize?: keyof DefaultNDSThemeType["fontSizes"];
+  lineHeight?: keyof DefaultNDSThemeType["lineHeights"];
   autoLayoutBreakpoint?: string;
 }
 

diff --git a/src/DescriptionList/DescriptionTerm.tsx b/src/DescriptionList/DescriptionTerm.tsx
@@ -2,27 +2,42 @@ import { styled } from "styled-components";
 import { useDescriptionListContext } from "./DescriptionListContext";
 
 export const DescriptionTerm = styled.dt(({ theme }) => {
-  const { showDivider, density, layout } = useDescriptionListContext();
+  const { showDivider, density, layout, autoLayoutBreakpoint } = useDescriptionListContext();
 
   return {
+    // narrow stuff + non-auto layout
     gridColumnStart: 1,
     color: theme.colors.midGrey,
     paddingLeft: theme.space.none,
     paddingRight: theme.space.none,
 
+    ...(layout !== "inline" && {
+      paddingBottom: theme.space.none,
+    }),
+
     ...(density === "compact" && {
       paddingTop: theme.space.x0_25,
-      paddingBottom: theme.space.x0_25,
+
+      ...(layout === "inline" && {
+        paddingBottom: theme.space.x0_25,
+      }),
     }),
 
     ...(density === "medium" && {
       paddingTop: theme.space.x0_75,
-      paddingBottom: theme.space.x0_75,
+
+      ...(layout === "inline" && {
+        paddingBottom: theme.space.x0_75,
+      }),
     }),
 
     ...(density === "relaxed" && {
       paddingTop: theme.space.x1_5,
-      paddingBottom: theme.space.x1_5,
+      paddingBottom: theme.space.x0_25,
+
+      ...(layout === "inline" && {
+        paddingBottom: theme.space.x1_5,
+      }),
     }),
 
     ...(showDivider && {
@@ -34,5 +49,28 @@ export const DescriptionTerm = styled.dt(({ theme }) => {
     "&:first-child": {
       border: "none",
     },
+
+    [`@container (min-width: ${autoLayoutBreakpoint})`]: {
+      // wide auto layout
+      ...(showDivider &&
+        layout !== "stacked" && {
+          borderTopWidth: "1px",
+          borderTopStyle: "solid",
+          borderTopColor: theme.colors.lightGrey,
+        }),
+
+      ...(density === "compact" &&
+        layout !== "stacked" && {
+          paddingBottom: theme.space.x0_25,
+        }),
+      ...(density === "medium" &&
+        layout !== "stacked" && {
+          paddingBottom: theme.space.x0_75,
+        }),
+      ...(density === "relaxed" &&
+        layout !== "stacked" && {
+          paddingBottom: theme.space.x1_5,
+        }),
+    },
   };
 });
diff --git a/src/DescriptionList/README.md b/src/DescriptionList/README.md
@@ -1,7 +1,6 @@
 # DescriptionList
 
-The DescriptionList component is a structured list that pairs terms with their descriptions, providing a clear way to display key-value information.
-The DescriptionList is ideal for displaying key-value pair information in a structured format organizing and explaining related information.
+The DescriptionList component is a structured list that pairs terms with their descriptions, providing a standardized way to display key-value pair information in a structured format organizing and explaining related information.
 
 ## Basic Usage
 
@@ -27,10 +26,11 @@ function MyComponent() {
 | `layout` | "stacked" \| "inline" \| "auto" | Controls the arrangement of terms and details | `"auto"` |
 | `density` | "compact" \| "medium" \| "relaxed" | Controls the spacing between items | `"medium"` |
 | `fontSize` | `keyof FontSizes` | Controls the text size | `"medium"` |
+| `lineHeight` | `keyof LineHeights` | Controls the text line height | `"base"` |
 | `showDivider` | boolean | Shows divider lines between items | `false` |
 | `descriptionTermMaxWidth` | string | Controls the maximum width of description terms | `"50%"` |
 | `autoLayoutBreakpoint` | string | Defines the container width at which the layout changes when layout is set to `auto` | `"640px"` |
-| `children` | ReactNode | Content to be displayed inside the list | - |
+| `children` | ReactNode | `DescriptionTerm` and `DescriptionDetails` to be placed inside the list | - |
 
 ## Accessibility